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

From: [email-masked] (jason)
Date: Mon, 28 Jun 2010 20:59:55 +0800

roman.tereshonkov at nokia.com wrote:
>  
>
>   
>> -----Original Message-----
>> From: ext jason [mailto:jason77.wang at gmail.com] 
>> Sent: 25 June, 2010 15:31
>> To: Grant Likely
>> Cc: Tereshonkov Roman (Nokia-D/Helsinki); david-b at pacbell.net; 
>>     
>>     
[snip]
>> the RXS bit will be set to status register immediately,
>> write a dummy data to TX register to trigger this session(in 
>> fact this  
>> trigger  doesn't take  effect),
>> check the RXS bit and read the first data(the first data is the last 
>> random data received in TX_ONLY transfer).
>>
>>     
>
> For me it is very strange that you have RXS bit set after TX_ONLY transmission.
> The problem might be elsewhere and your solution will just mask the real problem.
> Can you enable the spi debug and error logs and show them for your broken case.
>
> Regards
> Roman Tereshonkov
>
>   
Hi Roman,
I don't know how to enable the spi debug and generate an error log, but 
i design
3 testcases to investigate this issue. These 3 testcases are based off 
the latest upstream.
test1: After TX_ONLY transfer, we will change this channel to RX_ONLY 
mode. And then we
will write a dummy data to trigger this RX transaction, here i add a 
polling RXS bit
before write triggering data, from the result, we can see we 
successfully to get a RXS
bit and read an unmeaningful data.(this prove the write triggering data 
is not necessary
to start a rx transaction).
test2: This time i move the polling RXS bit to the location after the 
write trigger data, we can get
RXS bit as well and read an unmeaningful data too. The data read out is 
identical to the one
in test1.(This prove the write trigger data don't take effect in fact).
test3: Except i revert the commit a330ce2 "omap2_mcspi: Flush posted 
writes", other is same as the test2.
But this time, we read out a correct data ads7846 sending out. (I don't 
know why this commit can
affect the result, maybe it is time issue. If we can disable this 
channel after TX_ONLY transfer and
reenable it before RX_ONLY transfer, we will not get RXS bit in test1, 
and will read out a correct data
both in test2 and test3).

Test1 result output & patch:
# =====SPI MESSAGE BEGIN=====
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = f0
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = f0
*****SPI MESSAGE END*****
=====SPI MESSAGE BEGIN=====
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = f0
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = f0
*****SPI MESSAGE END*****

Subject: [PATCH] SPI/test1: get rx_data in RX_ONLY transfer before 
trigger writing

get rx_data in RX_ONLY transfer before trigger writing dummy data to
TX register.

Signed-off-by: Jason Wang <[email-masked]>
---
drivers/spi/omap2_mcspi.c | 19 ++++++++++++++-----
1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
index b3a94ca..ddfe4b0 100644
--- a/drivers/spi/omap2_mcspi.c
+++ b/drivers/spi/omap2_mcspi.c
@@ -893,7 +893,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) {
@@ -928,17 +928,26 @@ static void omap2_mcspi_work(struct work_struct 
*work)
if (t->len > ((cs->word_len + 7) >> 3))
chconf |= OMAP2_MCSPI_CHCONF_TURBO;
}
-
+ printk("tansfer%s: conf=%x, data=%x\n", t->tx_buf ? "TX" : "RX",
+ chconf, t->tx_buf ? *(u8 *)(t->tx_buf) : 0);
mcspi_write_chconf0(spi, chconf);

if (t->len) {
unsigned count;

/* RX_ONLY mode needs dummy data in TX reg */
- if (t->tx_buf == NULL)
+ if (t->tx_buf == NULL) {
+ u8 rx_data;
+ if (mcspi_wait_for_reg_bit(cs->base + OMAP2_MCSPI_CHSTAT0,
+ OMAP2_MCSPI_CHSTAT_RXS) < 0)
+ dev_err(&spi->dev, "RXS timed out\n");
+
+ rx_data = __raw_readl(cs->base + OMAP2_MCSPI_RX0);
+ printk("rx_data = %x\n", rx_data);
+
__raw_writel(0, cs->base
+ OMAP2_MCSPI_TX0);
-
+ }
if (m->is_dma_mapped || t->len >= DMA_MIN_BYTES)
count = omap2_mcspi_txrx_dma(spi, t);
else
@@ -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

Test2 result output & patch:
# =====SPI MESSAGE BEGIN=====
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = f0
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = f0
*****SPI MESSAGE END*****
=====SPI MESSAGE BEGIN=====
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = f0
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = f0
*****SPI MESSAGE END*****

Subject: [PATCH] SPI/test2: get rx_data in RX_ONLY transfer after 
trigger writing

get rx_data in RX_ONLY transfer after trigger writing dummy data to
TX register.

Signed-off-by: Jason Wang <[email-masked]>
---
drivers/spi/omap2_mcspi.c | 18 ++++++++++++++----
1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
index b3a94ca..96c508a 100644
--- a/drivers/spi/omap2_mcspi.c
+++ b/drivers/spi/omap2_mcspi.c
@@ -893,7 +893,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) {
@@ -928,17 +928,27 @@ static void omap2_mcspi_work(struct work_struct 
*work)
if (t->len > ((cs->word_len + 7) >> 3))
chconf |= OMAP2_MCSPI_CHCONF_TURBO;
}
-
+ printk("tansfer%s: conf=%x, data=%x\n", t->tx_buf ? "TX" : "RX",
+ chconf, t->tx_buf ? *(u8 *)(t->tx_buf) : 0);
mcspi_write_chconf0(spi, chconf);

if (t->len) {
unsigned count;

/* RX_ONLY mode needs dummy data in TX reg */
- if (t->tx_buf == NULL)
+ if (t->tx_buf == NULL) {
+ u8 rx_data;
__raw_writel(0, cs->base
+ OMAP2_MCSPI_TX0);

+ if (mcspi_wait_for_reg_bit(cs->base + OMAP2_MCSPI_CHSTAT0,
+ OMAP2_MCSPI_CHSTAT_RXS) < 0)
+ dev_err(&spi->dev, "RXS timed out\n");
+
+ rx_data = __raw_readl(cs->base + OMAP2_MCSPI_RX0);
+ printk("rx_data = %x\n", rx_data);
+
+ }
if (m->is_dma_mapped || t->len >= DMA_MIN_BYTES)
count = omap2_mcspi_txrx_dma(spi, t);
else
@@ -971,7 +981,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 result output & patch:
# =====SPI MESSAGE BEGIN=====
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = 77
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = 76
*****SPI MESSAGE END*****
=====SPI MESSAGE BEGIN=====
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = 75
tansferTX: conf=1123d4, data=93
tansferRX: conf=1113d4, data=0
rx_data = 74
*****SPI MESSAGE END*****

Subject: [PATCH] SPI/test3: get rx_data in RX_ONLY transfer after 
trigger writing

At frist revert the commit a330ce2 "omap2_mcspi: Flush posted writes",
then get rx_data in RX_ONLY transfer after trigger writing dummy data
to TX register.

Signed-off-by: Jason Wang <[email-masked]>
---
drivers/spi/omap2_mcspi.c | 19 ++++++++++++++-----
1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
index b3a94ca..5d71cad 100644
--- a/drivers/spi/omap2_mcspi.c
+++ b/drivers/spi/omap2_mcspi.c
@@ -204,7 +204,6 @@ 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);
}

static void omap2_mcspi_set_dma_req(const struct spi_device *spi,
@@ -893,7 +892,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) {
@@ -928,17 +927,27 @@ static void omap2_mcspi_work(struct work_struct 
*work)
if (t->len > ((cs->word_len + 7) >> 3))
chconf |= OMAP2_MCSPI_CHCONF_TURBO;
}
-
+ printk("tansfer%s: conf=%x, data=%x\n", t->tx_buf ? "TX" : "RX",
+ chconf, t->tx_buf ? *(u8 *)(t->tx_buf) : 0);
mcspi_write_chconf0(spi, chconf);

if (t->len) {
unsigned count;

/* RX_ONLY mode needs dummy data in TX reg */
- if (t->tx_buf == NULL)
+ if (t->tx_buf == NULL) {
+ u8 rx_data;
__raw_writel(0, cs->base
+ OMAP2_MCSPI_TX0);

+ if (mcspi_wait_for_reg_bit(cs->base + OMAP2_MCSPI_CHSTAT0,
+ OMAP2_MCSPI_CHSTAT_RXS) < 0)
+ dev_err(&spi->dev, "RXS timed out\n");
+
+ rx_data = __raw_readl(cs->base + OMAP2_MCSPI_RX0);
+ printk("rx_data = %x\n", rx_data);
+
+ }
if (m->is_dma_mapped || t->len >= DMA_MIN_BYTES)
count = omap2_mcspi_txrx_dma(spi, t);
else
@@ -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

>> But if we disable this channel after the TX_ONLY  transfer and 
>> reenable 
>> it  before  the RX_ONLY  transfer,
>> the trigger action will take effect and a meaningful data  will be  
>> transfered to RX register.
>>
>> Thanks,
>> Jason.
>>
>>     
>>>> Signed-off-by: Jason Wang <[email-masked]>
>>>> ---
>>>>  drivers/spi/omap2_mcspi.c |   16 ++++++----------
>>>>  1 files changed, 6 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
>>>>     
>>>>         
>> [snip]
>>
>>