* [PATCH v1] spi: dw: add check for Rx FIFO overflow
@ 2023-10-17 7:05 Maksim Kiselev
2023-12-18 11:28 ` Jagan Teki
0 siblings, 1 reply; 4+ messages in thread
From: Maksim Kiselev @ 2023-10-17 7:05 UTC (permalink / raw)
To: u-boot; +Cc: Maksim Kiselev, Jagan Teki
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 <bigunclemax@gmail.com>
---
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_DCOL BIT(6)
+/* Bit fields in ISR, IMR, RISR, 7 bits */
+#define DW_SPI_INT_MASK GENMASK(5, 0)
+#define DW_SPI_INT_TXEI BIT(0)
+#define DW_SPI_INT_TXOI BIT(1)
+#define DW_SPI_INT_RXUI BIT(2)
+#define DW_SPI_INT_RXOI BIT(3)
+#define DW_SPI_INT_RXFI BIT(4)
+#define DW_SPI_INT_MSTI BIT(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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v1] spi: dw: add check for Rx FIFO overflow
2023-10-17 7:05 [PATCH v1] spi: dw: add check for Rx FIFO overflow Maksim Kiselev
@ 2023-12-18 11:28 ` Jagan Teki
2023-12-18 17:31 ` Maxim Kiselev
0 siblings, 1 reply; 4+ messages in thread
From: Jagan Teki @ 2023-12-18 11:28 UTC (permalink / raw)
To: Maksim Kiselev; +Cc: u-boot
On Tue, Oct 17, 2023 at 12:35 PM Maksim Kiselev <bigunclemax@gmail.com> 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 <bigunclemax@gmail.com>
> ---
> 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_DCOL BIT(6)
>
> +/* Bit fields in ISR, IMR, RISR, 7 bits */
> +#define DW_SPI_INT_MASK GENMASK(5, 0)
> +#define DW_SPI_INT_TXEI BIT(0)
> +#define DW_SPI_INT_TXOI BIT(1)
> +#define DW_SPI_INT_RXUI BIT(2)
> +#define DW_SPI_INT_RXOI BIT(3)
> +#define DW_SPI_INT_RXFI BIT(4)
> +#define DW_SPI_INT_MSTI BIT(5)
Why do we need unused macros?
Jagan.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] spi: dw: add check for Rx FIFO overflow
2023-12-18 11:28 ` Jagan Teki
@ 2023-12-18 17:31 ` Maxim Kiselev
2023-12-20 7:23 ` Jagan Teki
0 siblings, 1 reply; 4+ messages in thread
From: Maxim Kiselev @ 2023-12-18 17:31 UTC (permalink / raw)
To: Jagan Teki; +Cc: u-boot
Hello Jagan,
пн, 18 дек. 2023 г. в 14:28, Jagan Teki <jagan@amarulasolutions.com>:
>
> On Tue, Oct 17, 2023 at 12:35 PM Maksim Kiselev <bigunclemax@gmail.com> 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 <bigunclemax@gmail.com>
> > ---
> > 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_DCOL BIT(6)
> >
> > +/* Bit fields in ISR, IMR, RISR, 7 bits */
> > +#define DW_SPI_INT_MASK GENMASK(5, 0)
> > +#define DW_SPI_INT_TXEI BIT(0)
> > +#define DW_SPI_INT_TXOI BIT(1)
> > +#define DW_SPI_INT_RXUI BIT(2)
> > +#define DW_SPI_INT_RXOI BIT(3)
> > +#define DW_SPI_INT_RXFI BIT(4)
> > +#define DW_SPI_INT_MSTI BIT(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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] spi: dw: add check for Rx FIFO overflow
2023-12-18 17:31 ` Maxim Kiselev
@ 2023-12-20 7:23 ` Jagan Teki
0 siblings, 0 replies; 4+ messages in thread
From: Jagan Teki @ 2023-12-20 7:23 UTC (permalink / raw)
To: Maxim Kiselev; +Cc: u-boot
On Mon, Dec 18, 2023 at 11:01 PM Maxim Kiselev <bigunclemax@gmail.com> wrote:
>
> Hello Jagan,
>
> пн, 18 дек. 2023 г. в 14:28, Jagan Teki <jagan@amarulasolutions.com>:
> >
> > On Tue, Oct 17, 2023 at 12:35 PM Maksim Kiselev <bigunclemax@gmail.com> 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 <bigunclemax@gmail.com>
> > > ---
> > > 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_DCOL BIT(6)
> > >
> > > +/* Bit fields in ISR, IMR, RISR, 7 bits */
> > > +#define DW_SPI_INT_MASK GENMASK(5, 0)
> > > +#define DW_SPI_INT_TXEI BIT(0)
> > > +#define DW_SPI_INT_TXOI BIT(1)
> > > +#define DW_SPI_INT_RXUI BIT(2)
> > > +#define DW_SPI_INT_RXOI BIT(3)
> > > +#define DW_SPI_INT_RXFI BIT(4)
> > > +#define DW_SPI_INT_MSTI BIT(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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-12-20 7:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-17 7:05 [PATCH v1] spi: dw: add check for Rx FIFO overflow Maksim Kiselev
2023-12-18 11:28 ` Jagan Teki
2023-12-18 17:31 ` Maxim Kiselev
2023-12-20 7:23 ` Jagan Teki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox