linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] spi-pxa2xx.c: modify the chip selection timing when spi transfer
       [not found] <20190306030519.10746-1-jin.xiao@intel.com>
@ 2019-03-06  7:52 ` Jarkko Nikula
  2019-03-07  6:47   ` Xiao, Jin
  0 siblings, 1 reply; 2+ messages in thread
From: Jarkko Nikula @ 2019-03-06  7:52 UTC (permalink / raw)
  To: xiao jin, daniel, haojian.zhuang, robert.jarzmik, broonie,
	linux-arm-kernel, linux-spi, linux-kernel, yanmin.zhang
  Cc: he, bo

Hi

On 3/6/19 5:05 AM, xiao jin wrote:
> From: "he, bo" <bo.he@intel.com>
> 
> We find spi can't work on board. More debug shows it's related
> to the following patch that changed the chip selection assert and
> deassert timing.
> 
^^ timing caught my attention. More below.

> @@ -610,6 +596,7 @@ static void int_transfer_complete(struct driver_data *drv_data)
>   	if (!pxa25x_ssp_comp(drv_data))
>   		pxa2xx_spi_write(drv_data, SSTO, 0);
>   
> +	cs_deassert(drv_data);
>   	spi_finalize_current_transfer(drv_data->master);

This

> @@ -1070,6 +1057,7 @@ static int pxa2xx_spi_transfer_one(struct spi_controller *master,
>   			pxa2xx_spi_write(drv_data, SSTO, chip->timeout);
>   	}
>   
> +	cs_assert(drv_data);

and this is not correct with core message loop. It will cause the chip 
select is toggled with each transfer in PIO mode. If there is no 
cs_change flag set then there shouldn't be CS toggling between the 
transfers if SPI message consists of multiple transfers.

More over this patch also will regress with DMA mode since there won't 
be CS deassert at all.

Timing reminded me I've seen two cases where there was a timing related 
glitch in CS output:

d0283eb2dbc1 ("spi: pxa2xx: Add output control for multiple Intel LPSS 
chip selects")
7a8d44bc89e5 ("spi: pxa2xx: Fix too early chipselect deassert")

Do you have a possibility to measure with an oscilloscope what goes 
wrong with the CS after d5898e19c0d7 ("spi: pxa2xx: Use core message 
processing loop")?

Can you share your setup if I can reproduce it here? E.g. SPI clock 
frequency, single or multiple CS, frequency of occurrence, etc

-- 
Jarkko

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] spi-pxa2xx.c: modify the chip selection timing when spi transfer
  2019-03-06  7:52 ` [PATCH] spi-pxa2xx.c: modify the chip selection timing when spi transfer Jarkko Nikula
@ 2019-03-07  6:47   ` Xiao, Jin
  0 siblings, 0 replies; 2+ messages in thread
From: Xiao, Jin @ 2019-03-07  6:47 UTC (permalink / raw)
  To: Jarkko Nikula, daniel, haojian.zhuang, robert.jarzmik, broonie,
	linux-arm-kernel, linux-spi, linux-kernel, yanmin.zhang
  Cc: jin.xiao, he, bo

Hi Jarkko,

On 3/6/2019 3:52 PM, Jarkko Nikula wrote:
> Hi
>
> On 3/6/19 5:05 AM, xiao jin wrote:
>> From: "he, bo" <bo.he@intel.com>
>>
>> We find spi can't work on board. More debug shows it's related
>> to the following patch that changed the chip selection assert and
>> deassert timing.
>>
> ^^ timing caught my attention. More below.
>
>> @@ -610,6 +596,7 @@ static void int_transfer_complete(struct 
>> driver_data *drv_data)
>>       if (!pxa25x_ssp_comp(drv_data))
>>           pxa2xx_spi_write(drv_data, SSTO, 0);
>>   +    cs_deassert(drv_data);
>>       spi_finalize_current_transfer(drv_data->master);
>
> This
>
>> @@ -1070,6 +1057,7 @@ static int pxa2xx_spi_transfer_one(struct 
>> spi_controller *master,
>>               pxa2xx_spi_write(drv_data, SSTO, chip->timeout);
>>       }
>>   +    cs_assert(drv_data);
>
> and this is not correct with core message loop. It will cause the chip 
> select is toggled with each transfer in PIO mode. If there is no 
> cs_change flag set then there shouldn't be CS toggling between the 
> transfers if SPI message consists of multiple transfers.
>
> More over this patch also will regress with DMA mode since there won't 
> be CS deassert at all.
>
> Timing reminded me I've seen two cases where there was a timing 
> related glitch in CS output:
>
> d0283eb2dbc1 ("spi: pxa2xx: Add output control for multiple Intel LPSS 
> chip selects")
> 7a8d44bc89e5 ("spi: pxa2xx: Fix too early chipselect deassert")
>
> Do you have a possibility to measure with an oscilloscope what goes 
> wrong with the CS after d5898e19c0d7 ("spi: pxa2xx: Use core message 
> processing loop")?
>
> Can you share your setup if I can reproduce it here? E.g. SPI clock 
> frequency, single or multiple CS, frequency of occurrence, etc
>
In our setup the pxa_ssp_type is LPSS_BXT_SSP.  With more debug we find 
that the problem is caused in the case of "restart the SSP" when 
pxa2xx_spi_transfer_one.

We will send another RFC patch.

Thanks.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-03-07  6:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20190306030519.10746-1-jin.xiao@intel.com>
2019-03-06  7:52 ` [PATCH] spi-pxa2xx.c: modify the chip selection timing when spi transfer Jarkko Nikula
2019-03-07  6:47   ` Xiao, Jin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).