[PATCH] spi/omap2_mcspi: disable and enable chan between each SPI transfer

Thu Jul 1 09:57:20 EDT 2010

Hi Jason,

Your logs do not show what I wanted to see.
But what I can see now at least is the case when TX is full and RX is full at the same time.

1.  Put 
	dev_dbg(&spi->dev, "status reg: %08x\n", __raw_readl(chstat_reg));
	after "do" and before "while (c)" in omap2_mcspi_txrx_pio function.
	I want to see how status is changed before and after TX or RX transaction.
2. Also try to make fake reading 
	__raw_readl(tx_reg)
	 after TX write in omap2_mcspi_txrx_pio.
	and 
	__raw_readl(cs->base + OMAP2_MCSPI_TX0);
	in mcspi_work function.
	This should exclude the posted write effect if such present.

If you put more logging info from other spi registers it might be also usefull in problem analyzing.
And it is better to concentrate on your test case 1. 
So as it is the test which gives the bug with unknown yet nature.

Regards
Roman Tereshonkov

>-----Original Message-----
>From: ext jason [mailto:jason77.wang at gmail.com] 
>Sent: 01 July, 2010 14:59
>To: Tereshonkov Roman (Nokia-MS/Helsinki)
>Cc: grant.likely at secretlab.ca; david-b at pacbell.net; 
>spi-devel-general at lists.sourceforge.net; 
>linux-arm-kernel at lists.infradead.org
>Subject: Re: [PATCH] spi/omap2_mcspi: disable and enable chan 
>between each SPI transfer
>
>roman.tereshonkov at nokia.com wrote:
>> Hi Jason,
>>
>> It is a little bit hard to analyze your logs.
>> 1. You showed the bytes read in your own way but there is 
>the data reading in omap2_mcspi_txrx_pio function also.
>> 2. For your third test case. You try to read data after 
>TX_ONLY, before triggering RX_ONLY, and get the right word.
>> Looks like there is something wrong.
>>
>> Try to log MCSPI_CHxSTAT register to follow when RXS bit 
>(register RX is full) is set.
>> And do not use the RX register reading in you own way. If 
>you need RX logging do it from omap2_mcspi_txrx_pio function.
>>
>>
>> Regards
>> Roman Tereshonkov
>>
>>
>>   
>Hi roman,
>
>This time i enable SPI_DEBUG first then design 3 test cases to
>investigate spi issue.
>test 1: print tx data, status reg(when RXS bit set) and rx data in 
>txrx_pio func.
>test 2: revert the commit a330ce2 "omap2_mcspi: Flush posted writes",
>other is same as test 1.
>test 3: disable and enable channel between each SPI transfer, other is
>same as test 1.
>
>The output of test2 and test3 is same and is right, while 
>test1's output is
>wrong.
>
>The ads7846 driver works like that, when i touch the top-left 
>corner of the
>touchscreen, it will send a READ-Y command first(8 bit word 0x93,
>TX_ONLY transfer), then it will receive y coordinate(two 8 bit 
>words, MSB
>12bits are meaningful, correct data should be around 0x77nn, RX_ONLY 
>transfer).
>
>the output of test1 is:
><4>======SPI MESSAGE BEGIN======
><7>ads7846 spi1.0: write-8 93
><7>ads7846 spi1.0: status reg: 00000005
><7>ads7846 spi1.0: read-8 f0
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 77
><7>ads7846 spi1.0: write-8 93
><7>ads7846 spi1.0: status reg: 00000005
><7>ads7846 spi1.0: read-8 f0
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 77
><4>.....SPI MESSAGE END.....
>
>the output of test2 is:
><4>======SPI MESSAGE BEGIN======
><7>ads7846 spi1.0: write-8 93
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 77
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 b0
><7>ads7846 spi1.0: write-8 93
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 77
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 88
><4>.....SPI MESSAGE END.....
>
>the output of test3 is:
><4>======SPI MESSAGE BEGIN======
><7>ads7846 spi1.0: write-8 93
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 77
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 68
><7>ads7846 spi1.0: write-8 93
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 77
><7>ads7846 spi1.0: status reg: 00000007
><7>ads7846 spi1.0: read-8 98
><4>.....SPI MESSAGE END.....
>
>
>test1 patch:
>Subject: [PATCH] SPI/test1: print tx data & rx data when ads7846 works
>
>when we touch the top-left corner of the touchscreen, the ads7846
>driver will send a read-y command(one 8-bit word, 0x93) and receive
>y coordinate(two 8-bit words, MSB 12bits are meaningful). now print
>all tx word, staus reg and rx word when RXS bit is set. Test1 only
>add debug output and have no other modification.
>
>Signed-off-by: Jason Wang <jason77.wang at gmail.com>
>---
>drivers/spi/omap2_mcspi.c | 11 +++++++++--
>1 files changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
>index b3a94ca..893a124 100644
>--- a/drivers/spi/omap2_mcspi.c
>+++ b/drivers/spi/omap2_mcspi.c
>@@ -40,6 +40,8 @@
>#include <plat/clock.h>
>#include <plat/mcspi.h>
>
>+#define VERBOSE
>+
>#define OMAP2_MCSPI_MAX_FREQ 48000000
>
>/* OMAP2 has 3 SPI controllers, while OMAP3 has 4 */
>@@ -502,6 +504,11 @@ omap2_mcspi_txrx_pio(struct spi_device 
>*spi, struct 
>spi_transfer *xfer)
>goto out;
>}
>
>+#ifdef VERBOSE
>+ dev_dbg(&spi->dev, "status reg: %08x\n",
>+ __raw_readl(chstat_reg));
>+#endif
>+
>if (c == 1 && tx == NULL &&
>(l & OMAP2_MCSPI_CHCONF_TURBO)) {
>omap2_mcspi_set_enable(spi, 0);
>@@ -893,7 +900,7 @@ static void omap2_mcspi_work(struct 
>work_struct *work)
>spi = m->spi;
>cs = spi->controller_state;
>cd = spi->controller_data;
>-
>+ printk("======SPI MESSAGE BEGIN======\n");
>omap2_mcspi_set_enable(spi, 1);
>list_for_each_entry(t, &m->transfers, transfer_list) {
>if (t->tx_buf == NULL && t->rx_buf == NULL && t->len) {
>@@ -971,7 +978,7 @@ static void omap2_mcspi_work(struct 
>work_struct *work)
>omap2_mcspi_force_cs(spi, 0);
>
>omap2_mcspi_set_enable(spi, 0);
>-
>+ printk(".....SPI MESSAGE END.....\n");
>m->status = status;
>m->complete(m->context);
>
>-- 
>1.5.6.5
>
>
>test2 patch:
>Subject: [PATCH] SPI/test2: print tx data & rx data when ads7846 works
>
>when we touch the top-left corner of the touchscreen, the ads7846
>driver will send a read-y command(one 8-bit word, 0x93) and receive
>y coordinate(two 8-bit words, MSB 12bits are meaningful). now print
>all tx word and rx word. In test2, i revert the commit a330ce2
>"omap2_mcspi: Flush posted writes".
>
>Signed-off-by: Jason Wang <jason77.wang at gmail.com>
>---
>drivers/spi/omap2_mcspi.c | 13 ++++++++++---
>1 files changed, 10 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
>index b3a94ca..cf8066e 100644
>--- a/drivers/spi/omap2_mcspi.c
>+++ b/drivers/spi/omap2_mcspi.c
>@@ -40,6 +40,8 @@
>#include <plat/clock.h>
>#include <plat/mcspi.h>
>
>+#define VERBOSE
>+
>#define OMAP2_MCSPI_MAX_FREQ 48000000
>
>/* OMAP2 has 3 SPI controllers, while OMAP3 has 4 */
>@@ -204,7 +206,7 @@ static inline void 
>mcspi_write_chconf0(const struct 
>spi_device *spi, u32 val)
>
>cs->chconf0 = val;
>mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, val);
>- mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>+ /* mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0); */
>}
>
>static void omap2_mcspi_set_dma_req(const struct spi_device *spi,
>@@ -502,6 +504,11 @@ omap2_mcspi_txrx_pio(struct spi_device 
>*spi, struct 
>spi_transfer *xfer)
>goto out;
>}
>
>+#ifdef VERBOSE
>+ dev_dbg(&spi->dev, "status reg: %08x\n",
>+ __raw_readl(chstat_reg));
>+#endif
>+
>if (c == 1 && tx == NULL &&
>(l & OMAP2_MCSPI_CHCONF_TURBO)) {
>omap2_mcspi_set_enable(spi, 0);
>@@ -893,7 +900,7 @@ static void omap2_mcspi_work(struct 
>work_struct *work)
>spi = m->spi;
>cs = spi->controller_state;
>cd = spi->controller_data;
>-
>+ printk("======SPI MESSAGE BEGIN======\n");
>omap2_mcspi_set_enable(spi, 1);
>list_for_each_entry(t, &m->transfers, transfer_list) {
>if (t->tx_buf == NULL && t->rx_buf == NULL && t->len) {
>@@ -971,7 +978,7 @@ static void omap2_mcspi_work(struct 
>work_struct *work)
>omap2_mcspi_force_cs(spi, 0);
>
>omap2_mcspi_set_enable(spi, 0);
>-
>+ printk(".....SPI MESSAGE END.....\n");
>m->status = status;
>m->complete(m->context);
>
>-- 
>1.5.6.5
>
>
>test3 patch:
>Subject: [PATCH] SPI/test3: print tx data & rx data when ads7846 works
>
>when we touch the top-left corner of the touchscreen, the ads7846
>driver will send a read-y command(one 8-bit word, 0x93) and receive
>y coordinate(two 8-bit words, MSB 12bits are meaningful). now print
>all tx word and rx word. In test3, i add disable channel and reenable
>channel between each SPI transfer.
>
>Signed-off-by: Jason Wang <jason77.wang at gmail.com>
>---
>drivers/spi/omap2_mcspi.c | 21 +++++++++++++++------
>1 files changed, 15 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
>index b3a94ca..7449365 100644
>--- a/drivers/spi/omap2_mcspi.c
>+++ b/drivers/spi/omap2_mcspi.c
>@@ -40,6 +40,8 @@
>#include <plat/clock.h>
>#include <plat/mcspi.h>
>
>+#define VERBOSE
>+
>#define OMAP2_MCSPI_MAX_FREQ 48000000
>
>/* OMAP2 has 3 SPI controllers, while OMAP3 has 4 */
>@@ -408,7 +410,6 @@ omap2_mcspi_txrx_dma(struct spi_device 
>*spi, struct 
>spi_transfer *xfer)
>count -= (word_len <= 8) ? 2 :
>(word_len <= 16) ? 4 :
>/* word_len <= 32 */ 8;
>- omap2_mcspi_set_enable(spi, 1);
>return count;
>}
>}
>@@ -430,8 +431,10 @@ omap2_mcspi_txrx_dma(struct spi_device 
>*spi, struct 
>spi_transfer *xfer)
>(word_len <= 16) ? 2 :
>/* word_len <= 32 */ 4;
>}
>- omap2_mcspi_set_enable(spi, 1);
>}
>+
>+ omap2_mcspi_set_enable(spi, 0);
>+
>return count;
>}
>
>@@ -502,6 +505,11 @@ omap2_mcspi_txrx_pio(struct spi_device 
>*spi, struct 
>spi_transfer *xfer)
>goto out;
>}
>
>+#ifdef VERBOSE
>+ dev_dbg(&spi->dev, "status reg: %08x\n",
>+ __raw_readl(chstat_reg));
>+#endif
>+
>if (c == 1 && tx == NULL &&
>(l & OMAP2_MCSPI_CHCONF_TURBO)) {
>omap2_mcspi_set_enable(spi, 0);
>@@ -646,7 +654,7 @@ omap2_mcspi_txrx_pio(struct spi_device 
>*spi, struct 
>spi_transfer *xfer)
>dev_err(&spi->dev, "EOT timed out\n");
>}
>out:
>- omap2_mcspi_set_enable(spi, 1);
>+ omap2_mcspi_set_enable(spi, 0);
>return count - c;
>}
>
>@@ -893,8 +901,7 @@ static void omap2_mcspi_work(struct 
>work_struct *work)
>spi = m->spi;
>cs = spi->controller_state;
>cd = spi->controller_data;
>-
>- omap2_mcspi_set_enable(spi, 1);
>+ printk("======SPI MESSAGE BEGIN======\n");
>list_for_each_entry(t, &m->transfers, transfer_list) {
>if (t->tx_buf == NULL && t->rx_buf == NULL && t->len) {
>status = -EINVAL;
>@@ -931,6 +938,8 @@ static void omap2_mcspi_work(struct 
>work_struct *work)
>
>mcspi_write_chconf0(spi, chconf);
>
>+ omap2_mcspi_set_enable(spi, 1);
>+
>if (t->len) {
>unsigned count;
>
>@@ -971,7 +980,7 @@ static void omap2_mcspi_work(struct 
>work_struct *work)
>omap2_mcspi_force_cs(spi, 0);
>
>omap2_mcspi_set_enable(spi, 0);
>-
>+ printk(".....SPI MESSAGE END.....\n");
>m->status = status;
>m->complete(m->context);
>
>-- 
>1.5.6.5
>
>
>