linux-kernel - Re: [PATCH] spi-pxa2xx.c: modify the chip selection timing when spi transfer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 7 Mar 2019 14:47:31 +0800
From:   "Xiao, Jin" <jin.xiao@...el.com>
To:     Jarkko Nikula <jarkko.nikula@...ux.intel.com>, daniel@...que.org,
        haojian.zhuang@...il.com, robert.jarzmik@...e.fr,
        broonie@...nel.org, linux-arm-kernel@...ts.infradead.org,
        linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
        yanmin.zhang@...el.com
Cc:     "he, bo" <bo.he@...el.com>, jin.xiao@...el.com
Subject: Re: [PATCH] spi-pxa2xx.c: modify the chip selection timing when spi
 transfer

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@...el.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.