Re: [PATCH v1] spi: dw: add check for Rx FIFO overflow
On Mon, Dec 18, 2023 at 11:01 PM Maxim Kiselev wrote: > > Hello Jagan, > > пн, 18 дек. 2023 г. в 14:28, Jagan Teki : > > > > On Tue, Oct 17, 2023 at 12:35 PM Maksim Kiselev > > wrote: > > > > > > If even one byte is lost due to Rx FIFO overflow then we will never > > > exit the read loop. Because the (priv->rx != priv->rx_end) condition will > > > be always true. > > > > > > Let's check if Rx FIFO overflow occurred and exit the read loop > > > in this case. > > > > > > Signed-off-by: Maksim Kiselev > > > --- > > > drivers/spi/designware_spi.c | 24 +--- > > > 1 file changed, 21 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c > > > index 1c7d0ca310..0f443bff8e 100644 > > > --- a/drivers/spi/designware_spi.c > > > +++ b/drivers/spi/designware_spi.c > > > @@ -111,6 +111,15 @@ > > > #define SR_TX_ERR BIT(5) > > > #define SR_DCOLBIT(6) > > > > > > +/* Bit fields in ISR, IMR, RISR, 7 bits */ > > > +#define DW_SPI_INT_MASKGENMASK(5, 0) > > > +#define DW_SPI_INT_TXEIBIT(0) > > > +#define DW_SPI_INT_TXOIBIT(1) > > > +#define DW_SPI_INT_RXUIBIT(2) > > > +#define DW_SPI_INT_RXOIBIT(3) > > > +#define DW_SPI_INT_RXFIBIT(4) > > > +#define DW_SPI_INT_MSTIBIT(5) > > > > Why do we need unused macros? > > Actually DW_SPI_INT_RXOI is used. As for the other bits in DW_SPI_RISR, > I decided to add them all to match a Linux driver. > > We already have a lot of unused macro in this driver (ex. definitions > for bits in CTRLR0_FRF, > CTRLR0_MODE regs and so on). > > So, what's a problem with adding the DW_SPI_RISR bits? I'm not fond of unused change in the particular patch, better to send the updated one with used bits. Jagan.
Re: [PATCH v1] spi: dw: add check for Rx FIFO overflow
Hello Jagan, пн, 18 дек. 2023 г. в 14:28, Jagan Teki : > > On Tue, Oct 17, 2023 at 12:35 PM Maksim Kiselev wrote: > > > > If even one byte is lost due to Rx FIFO overflow then we will never > > exit the read loop. Because the (priv->rx != priv->rx_end) condition will > > be always true. > > > > Let's check if Rx FIFO overflow occurred and exit the read loop > > in this case. > > > > Signed-off-by: Maksim Kiselev > > --- > > drivers/spi/designware_spi.c | 24 +--- > > 1 file changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c > > index 1c7d0ca310..0f443bff8e 100644 > > --- a/drivers/spi/designware_spi.c > > +++ b/drivers/spi/designware_spi.c > > @@ -111,6 +111,15 @@ > > #define SR_TX_ERR BIT(5) > > #define SR_DCOLBIT(6) > > > > +/* Bit fields in ISR, IMR, RISR, 7 bits */ > > +#define DW_SPI_INT_MASKGENMASK(5, 0) > > +#define DW_SPI_INT_TXEIBIT(0) > > +#define DW_SPI_INT_TXOIBIT(1) > > +#define DW_SPI_INT_RXUIBIT(2) > > +#define DW_SPI_INT_RXOIBIT(3) > > +#define DW_SPI_INT_RXFIBIT(4) > > +#define DW_SPI_INT_MSTIBIT(5) > > Why do we need unused macros? Actually DW_SPI_INT_RXOI is used. As for the other bits in DW_SPI_RISR, I decided to add them all to match a Linux driver. We already have a lot of unused macro in this driver (ex. definitions for bits in CTRLR0_FRF, CTRLR0_MODE regs and so on). So, what's a problem with adding the DW_SPI_RISR bits? Cheers, Maksim > > Jagan.
Re: [PATCH v1] spi: dw: add check for Rx FIFO overflow
On Tue, Oct 17, 2023 at 12:35 PM Maksim Kiselev wrote: > > If even one byte is lost due to Rx FIFO overflow then we will never > exit the read loop. Because the (priv->rx != priv->rx_end) condition will > be always true. > > Let's check if Rx FIFO overflow occurred and exit the read loop > in this case. > > Signed-off-by: Maksim Kiselev > --- > drivers/spi/designware_spi.c | 24 +--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c > index 1c7d0ca310..0f443bff8e 100644 > --- a/drivers/spi/designware_spi.c > +++ b/drivers/spi/designware_spi.c > @@ -111,6 +111,15 @@ > #define SR_TX_ERR BIT(5) > #define SR_DCOLBIT(6) > > +/* Bit fields in ISR, IMR, RISR, 7 bits */ > +#define DW_SPI_INT_MASKGENMASK(5, 0) > +#define DW_SPI_INT_TXEIBIT(0) > +#define DW_SPI_INT_TXOIBIT(1) > +#define DW_SPI_INT_RXUIBIT(2) > +#define DW_SPI_INT_RXOIBIT(3) > +#define DW_SPI_INT_RXFIBIT(4) > +#define DW_SPI_INT_MSTIBIT(5) Why do we need unused macros? Jagan.
[PATCH v1] spi: dw: add check for Rx FIFO overflow
If even one byte is lost due to Rx FIFO overflow then we will never exit the read loop. Because the (priv->rx != priv->rx_end) condition will be always true. Let's check if Rx FIFO overflow occurred and exit the read loop in this case. Signed-off-by: Maksim Kiselev --- drivers/spi/designware_spi.c | 24 +--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 1c7d0ca310..0f443bff8e 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -111,6 +111,15 @@ #define SR_TX_ERR BIT(5) #define SR_DCOLBIT(6) +/* Bit fields in ISR, IMR, RISR, 7 bits */ +#define DW_SPI_INT_MASKGENMASK(5, 0) +#define DW_SPI_INT_TXEIBIT(0) +#define DW_SPI_INT_TXOIBIT(1) +#define DW_SPI_INT_RXUIBIT(2) +#define DW_SPI_INT_RXOIBIT(3) +#define DW_SPI_INT_RXFIBIT(4) +#define DW_SPI_INT_MSTIBIT(5) + #define RX_TIMEOUT 1000/* timeout in ms */ struct dw_spi_plat { @@ -588,7 +597,7 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) struct dw_spi_priv *priv = dev_get_priv(bus); u8 op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes; u8 op_buf[op_len]; - u32 cr0; + u32 cr0, sts; if (read) priv->tmode = CTRLR0_TMOD_EPROMREAD; @@ -632,12 +641,21 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) * them to fail because we are not reading/writing the fifo fast enough. */ if (read) { - priv->rx = op->data.buf.in; + void *prev_rx = priv->rx = op->data.buf.in; priv->rx_end = priv->rx + op->data.nbytes; dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev)); - while (priv->rx != priv->rx_end) + while (priv->rx != priv->rx_end) { dw_reader(priv); + if (prev_rx == priv->rx) { + sts = dw_read(priv, DW_SPI_RISR); + if (sts & DW_SPI_INT_RXOI) { + dev_err(bus, "FIFO overflow on Rx\n"); + return -EIO; + } + } + prev_rx = priv->rx; + } } else { u32 val; -- 2.40.1