Skip to contentSkip to conversation

spi: designware_spi: Fix off-by-one for checking fifo depth

PATCH New Axel Lin 2 participants 3 messages Dec 19, 2014 SPI

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 spi: designware_spi: Fix off-by-one for checking fifo depth

The depth could be from 2 to 256 from HW spec, so fix off-by-one for checking fifo depth.

---
Hi,
I don't have this hardware handy, so please test it.

 drivers/spi/designware_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/spi/designware_spi.c +1 -1
index 98c9f03..14d0c23 100644
@@ -164,7 +164,7 @@ static void spi_hw_init(struct dw_spi_priv *priv)
 	if (!priv->fifo_len) {
 		u32 fifo;
 
-		for (fifo = 2; fifo <= 257; fifo++) {
+		for (fifo = 2; fifo <= 256; fifo++) {
 			dw_writew(priv, DW_SPI_TXFLTR, fifo);
 			if (fifo != dw_readw(priv, DW_SPI_TXFLTR))
 				break;
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 spi: designware_spi: Fix off-by-one for checking fifo depth

Hi Axel,

Quoted 15 lines · Axel Lin

The depth could be from 2 to 256 from HW spec, so fix off-by-one for checking fifo depth.

---
Hi,
I don't have this hardware handy, so please test it.

  drivers/spi/designware_spi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/spi/designware_spi.c +0 -0
index 98c9f03..14d0c23 100644

drivers/spi/designware_spi.c static void spi_hw_init(struct dw_spi_priv *priv) · line 170

Quoted 8 lines
drivers/spi/designware_spi.c +1 -1
@@ -164,7 +164,7 @@ static void spi_hw_init(struct dw_spi_priv *priv)
  	if (!priv->fifo_len) {
  		u32 fifo;
 
-		for (fifo = 2; fifo <= 257; fifo++) {
+		for (fifo = 2; fifo <= 256; fifo++) {
  			dw_writew(priv, DW_SPI_TXFLTR, fifo);
  			if (fifo != dw_readw(priv, DW_SPI_TXFLTR))
  				break;

Please note that this code was not written by myself but is based on the Linux kernel version. And from my current understanding, this code tries to detect the FIFO size. So this 257 being bigger than the max value of 256 is most likely correct. Also another 257 is a few lines below your code:

		dws->fifo_len = (fifo == 257) ? 0 : fifo;

So I don't think that we need to change this here.

BTW: It might be better to send this patch (or a question about this 256 vs 257) to the SPI Linux mailing list. The maintainer there will have deeper knowledge about this issue.

Thanks,
Stefan

author
#2 lore raw

PATCH 1/1 spi: designware_spi: Fix off-by-one for checking fifo depth

2014-12-19 15:35 GMT+08:00 Stefan Roese <[email protected]>:

Quoted 36 lines

Hi Axel,

Quoted 17 lines · Axel Lin

The depth could be from 2 to 256 from HW spec, so fix off-by-one for checking
fifo depth.

---
Hi,
I don't have this hardware handy, so please test it.

  drivers/spi/designware_spi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/spi/designware_spi.c +0 -0
index 98c9f03..14d0c23 100644

drivers/spi/designware_spi.c static void spi_hw_init(struct dw_spi_priv *priv) · line 170

Quoted 8 lines
drivers/spi/designware_spi.c +1 -1
@@ -164,7 +164,7 @@ static void spi_hw_init(struct dw_spi_priv *priv)
        if (!priv->fifo_len) {
                u32 fifo;
 
-               for (fifo = 2; fifo <= 257; fifo++) {
+               for (fifo = 2; fifo <= 256; fifo++) {
                        dw_writew(priv, DW_SPI_TXFLTR, fifo);
                        if (fifo != dw_readw(priv, DW_SPI_TXFLTR))
                                break;

Please note that this code was not written by myself but is based on the Linux kernel version. And from my current understanding, this code tries to detect the FIFO size. So this 257 being bigger than the max value of 256 is most likely correct. Also another 257 is a few lines below your code:

Quoted 1 line
                dws->fifo_len = (fifo == 257) ? 0 : fifo;

hi Stefan,

After the for loop iteration with "no-match" case, the fifo variable will be 258 in original code. So above code won't catch such case.

Quoted 6 lines

So I don't think that we need to change this here.

BTW: It might be better to send this patch (or a question about this 256 vs
257) to the SPI Linux mailing list. The maintainer there will have deeper knowledge about this issue.

Well, I'll send similar patch for linux driver for review (in spi maillist). Thanks.