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

[roman.tereshonkov at nokia.com]

Hi,

I tried to find the board with the same touchscreen but failed.

Your patch solves the problem but does not fix the bug.
It would be nice if you find the roots of the bug to exclude future headackes.
 

I am leaving to vacation and will be silent some time.


Regards
Roman Tereshonkov


>-----Original Message-----
>From: ext jason [mailto:jason77.wang at gmail.com] 
>Sent: 13 July, 2010 16:26
>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
>
>Hi roman,
>
>How about my test case, does it expose the real problem?
>
>I know my original patch was a big change both for PIO mode and for
>DMA mode, because there is no DMA mode device on my platform and
>i can't validate whether DMA mode works fine with that patch, 
>it is a bit
>risky to apply that patch to upstream.
>
>This time i limit my patch to PIO mode, In theory, it is safe for all 
>PIO transfers.
>The side effect is to add a little bit overhead for a sequence of 
>TX_ONLY transfers.
>But it really can solve my touch panel issue.
>
>Is the following patch acceptable?
>
> From 7b310e690fcccff400233a4c3340f42168016c92 Mon Sep 17 00:00:00 2001
>From: Jason Wang <jason77.wang at gmail.com>
>Date: Tue, 13 Jul 2010 18:05:59 +0800
>Subject: [PATCH] spi/omap2_mcspi: disable channel after 
>TX_ONLY transfer 
>in PIO mode
>
>In TX_ONLY transfer, the spi controller will receive datas
>simultaneously and hold them in the rx register, these datas will
>affect the direct following RX_ONLY transfer. So add disable channel
>to purge those rx datas.
>
>Signed-off-by: Jason Wang <jason77.wang at gmail.com>
>---
>drivers/spi/omap2_mcspi.c | 6 ++++++
>1 files changed, 6 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
>index b3a94ca..43fab41 100644
>--- a/drivers/spi/omap2_mcspi.c
>+++ b/drivers/spi/omap2_mcspi.c
>@@ -644,6 +644,12 @@ omap2_mcspi_txrx_pio(struct spi_device 
>*spi, struct 
>spi_transfer *xfer)
>} else if (mcspi_wait_for_reg_bit(chstat_reg,
>OMAP2_MCSPI_CHSTAT_EOT) < 0)
>dev_err(&spi->dev, "EOT timed out\n");
>+
>+ /* disable chan to purge rx datas received in TX_ONLY transfer,
>+ * otherwise these rx datas will affect the direct following
>+ * RX_ONLY transfer.
>+ */
>+ omap2_mcspi_set_enable(spi, 0);
>}
>out:
>omap2_mcspi_set_enable(spi, 1);
>-- 
>1.5.6.5
>
>
>
>roman.tereshonkov at nokia.com wrote:
>> 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.
>>>     
>>>>       
><snip>
>
>