| 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
| ||
|
Message-ID: <PH8PR12MB66754DDE4373E3667B882C46E11EA@PH8PR12MB6675.namprd12.prod.outlook.com>
Date: Mon, 21 Aug 2023 07:26:57 +0000
From: "Goud, Srinivas" <srinivas.goud@....com>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>
CC: "broonie@...nel.org" <broonie@...nel.org>,
"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"patches@...nsource.cirrus.com" <patches@...nsource.cirrus.com>
Subject: RE: [PATCH v2 1/2] spi: spi-cadence: Interleave write of TX and read
of RX FIFO
Hi Charles,
>-----Original Message-----
>From: Charles Keepax <ckeepax@...nsource.cirrus.com>
>Sent: Thursday, August 10, 2023 3:40 PM
>To: Goud, Srinivas <srinivas.goud@....com>
>Cc: broonie@...nel.org; linux-spi@...r.kernel.org; linux-
>kernel@...r.kernel.org; patches@...nsource.cirrus.com
>Subject: Re: [PATCH v2 1/2] spi: spi-cadence: Interleave write of TX and read of
>RX FIFO
>
>On Wed, Aug 09, 2023 at 11:31:24AM +0000, Goud, Srinivas wrote:
>> >+ while (ntx || nrx) {
>> > /* When xspi in busy condition, bytes may send failed,
>> > * then spi control did't work thoroughly, add one byte delay
>> > */
>> >- if (cdns_spi_read(xspi, CDNS_SPI_ISR) &
>> >- CDNS_SPI_IXR_TXFULL)
>> >+ if (cdns_spi_read(xspi, CDNS_SPI_ISR) &
>> >CDNS_SPI_IXR_TXFULL)
>> > udelay(10);
>> Linux driver configured as Slave, due to this above delay we see data
>corruption issue on Master side.
>> when Master is continuously reading the data, Slave is failed to prepare the
>date on time due to above delay.
>>
>> >@@ -391,11 +388,10 @@ static irqreturn_t cdns_spi_irq(int irq, void
>*dev_id)
>> > if (xspi->tx_bytes < xspi->tx_fifo_depth >> 1)
>> > cdns_spi_write(xspi, CDNS_SPI_THLD, 1);
>> >
>> >- cdns_spi_read_rx_fifo(xspi, trans_cnt);
>> >-
>> > if (xspi->tx_bytes) {
>> >- cdns_spi_fill_tx_fifo(xspi, trans_cnt);
>> >+ cdns_spi_process_fifo(xspi, trans_cnt, trans_cnt);
>> > } else {
>> >+ cdns_spi_process_fifo(xspi, 0, trans_cnt);
>> When Linux driver configured as Slave, we observed data corruption issue
>with Slave mode read when data length is 400 bytes.
>> As TX empty doesn't guaranties valid data in RX FIFO, hence we added one
>byte delay(10us) before RX FIFO read to overcome above issue.
>> Created patch with above changes, find patch as attachment.
>> Can you please test and let me know your observations.
>>
>
>Yeah I can test the patch, I am on holiday this week so don't have access to the
>hardware, but will do so next week.
>
>> From 40154932ac7486c99e339bbc0b85b3cfe382286c Mon Sep 17 00:00:00
>2001
>> From: Srinivas Goud <srinivas.goud@....com>
>> Date: Tue, 1 Aug 2023 21:21:09 +0530
>> Subject: [PATCH] spi: spi-cadence: Fix data corruption issues in slave
>> mode
>>
>> Remove 10us delay in cdns_spi_process_fifo() (called from
>> cdns_spi_irq()) to fix data corruption issue on Master side when this
>> driver configured in Slave mode, as Slave is failed to prepare the
>> date on time due to above delay.
>>
>> Add 10us delay before processing the RX FIFO as TX empty doesn't
>> guaranty valid data in RX FIFO.
>
>guarantee
>
>>
>> Signed-off-by: Srinivas Goud <srinivas.goud@....com>
>> ---
>> drivers/spi/spi-cadence.c | 19 ++++++++++++-------
>> 1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
>> index 42f101d..07a593c 100644
>> --- a/drivers/spi/spi-cadence.c
>> +++ b/drivers/spi/spi-cadence.c
>> @@ -317,12 +317,6 @@ static void cdns_spi_process_fifo(struct cdns_spi
>*xspi, int ntx, int nrx)
>> xspi->rx_bytes -= nrx;
>>
>> while (ntx || nrx) {
>> - /* When xspi in busy condition, bytes may send failed,
>> - * then spi control did't work thoroughly, add one byte delay
>> - */
>> - if (cdns_spi_read(xspi, CDNS_SPI_ISR) &
>CDNS_SPI_IXR_TXFULL)
>> - udelay(10);
>> -
>> if (ntx) {
>> if (xspi->txbuf)
>> cdns_spi_write(xspi, CDNS_SPI_TXD, *xspi-
>>txbuf++); @@ -392,6
>> +386,11 @@ static irqreturn_t cdns_spi_irq(int irq, void *dev_id)
>> if (xspi->tx_bytes) {
>> cdns_spi_process_fifo(xspi, trans_cnt, trans_cnt);
>> } else {
>> + /* Fixed delay due to controller limitation with
>> + * RX_NEMPTY incorrect status
>> + * Xilinx AR:65885 contains more details
>
>Do you have a web link to this ticket? Would be good to get some more
>background.
Here AR link which contains the issue description
https://support.xilinx.com/s/article/65885?language=en_US
Thanks,
Srinivas
>
>> + */
>> + udelay(10);
>> cdns_spi_process_fifo(xspi, 0, trans_cnt);
>> cdns_spi_write(xspi, CDNS_SPI_IDR,
>> CDNS_SPI_IXR_DEFAULT);
>> @@ -439,12 +438,18 @@ static int cdns_transfer_one(struct spi_controller
>*ctlr,
>> cdns_spi_setup_transfer(spi, transfer);
>> } else {
>> /* Set TX empty threshold to half of FIFO depth
>> - * only if TX bytes are more than half FIFO depth.
>> + * only if TX bytes are more than FIFO depth.
>> */
>> if (xspi->tx_bytes > xspi->tx_fifo_depth)
>> cdns_spi_write(xspi, CDNS_SPI_THLD, xspi-
>>tx_fifo_depth >> 1);
>> }
>>
>> + /* When xspi in busy condition, bytes may send failed,
>> + * then spi control did't work thoroughly, add one byte delay
>
>Just noticing there is an n missing in didn't might as well add that whilst moving
>the comment.
>
>> + */
>> + if (cdns_spi_read(xspi, CDNS_SPI_ISR) & CDNS_SPI_IXR_TXFULL)
>> + udelay(10);
>> +
>> cdns_spi_process_fifo(xspi, xspi->tx_fifo_depth, 0);
>> spi_transfer_delay_exec(transfer);
>>
>> --
>> 2.1.1
>
>Thanks,
>Charles
Powered by blists - more mailing lists