Add error checking in spi-nor read and write
Subsystems (2)
v3 cover Add error checking in spi-nor read and write
Hello,
this is patchset which implements checking for SPI transfer errors in spi-nor.
With this you get an error rather than bunch of zeroes when a transfer fails and you get correct operation on SPI controllers that truncate long transfers.
Since the only comment to the patchset was that it introduces a warning when all patches are not applied I resend with this issue fixed.
Thanks
Michal
Michal Suchanek (6):
mtd: spi-nor: change return value of read/write
mtd: m25p80: return amount of data transferred or error in read/write
mtd: fsl-quadspi: return amount of data read/written or error
mtd: spi-nor: check return value from read/write
mtd: spi-nor: stop passing around retlen
mtd: spi-nor: simplify write loop
drivers/mtd/devices/m25p80.c | 33 ++++++++++------ drivers/mtd/spi-nor/fsl-quadspi.c | 29 +++++++------- drivers/mtd/spi-nor/spi-nor.c | 79 ++++++++++++++++++++++----------------- include/linux/mtd/spi-nor.h | 8 ++-- 4 files changed, 84 insertions(+), 65 deletions(-)
Signature · 1 line
2.1.4
PATCH v3 1/6 mtd: spi-nor: change return value of read/write
Change the return value of spi-nor device read and write methods to allow returning amount of data transferred and errors as read(2)/write(2) does.
---
- avoid compiler warning
--- drivers/mtd/devices/m25p80.c | 5 +++-- drivers/mtd/spi-nor/fsl-quadspi.c | 5 +++-- include/linux/mtd/spi-nor.h | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-)
drivers/mtd/devices/m25p80.c +3 -2
@@ -75,7 +75,7 @@ static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len,return spi_write (spi, flash->command, len +1 ); } -static void m25p80_write (struct spi_nor *nor,loff_t to,size_t len, +static ssize_t m25p80_write (struct spi_nor *nor,loff_t to,size_t len,size_t *retlen,const u_char *buf) {struct m25p *flash = nor->priv;
@@ -103,6 +103,7 @@ static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,spi_sync (spi, &m); *retlen += m.actual_length - cmd_sz; +return 0 ; }static inline unsigned int m25p80_rx_nbits (struct spi_nor *nor)
@@ -121,7 +122,7 @@ static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)* Read an address range from the nor chip. The address range * may be any size provided it is within the physical boundaries. */ -static int m25p80_read (struct spi_nor *nor,loff_t from,size_t len, +static ssize_t m25p80_read (struct spi_nor *nor,loff_t from,size_t len,size_t *retlen,u_char *buf) {struct m25p *flash = nor->priv;
drivers/mtd/spi-nor/fsl-quadspi.c +3 -2
@@ -712,7 +712,7 @@ static int fsl_qspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len,return ret; } -static void fsl_qspi_write (struct spi_nor *nor,loff_t to, +static ssize_t fsl_qspi_write (struct spi_nor *nor,loff_t to,size_t len,size_t *retlen,const u_char *buf) {struct fsl_qspi *q = nor->priv;
@@ -722,9 +722,10 @@ static void fsl_qspi_write(struct spi_nor *nor, loff_t to,/* invalid the data in the AHB buffer. */ fsl_qspi_invalid (q); +return 0 ; } -static int fsl_qspi_read (struct spi_nor *nor,loff_t from, +static ssize_t fsl_qspi_read (struct spi_nor *nor,loff_t from,size_t len,size_t *retlen,u_char *buf) {struct fsl_qspi *q = nor->priv;
include/linux/mtd/spi-nor.h +2 -2
@@ -185,9 +185,9 @@ struct spi_nor {int (*write_reg)(struct spi_nor *nor,u8 opcode,u8 *buf,int len,int write_enable); -int (*read)(struct spi_nor *nor,loff_t from, +ssize_t (*read)(struct spi_nor *nor,loff_t from,size_t len,size_t *retlen,u_char *read_buf); -void (*write)(struct spi_nor *nor,loff_t to, +ssize_t (*write)(struct spi_nor *nor,loff_t to,size_t len,size_t *retlen,const u_char *write_buf);int (*erase)(struct spi_nor *nor,loff_t offs);
Signature · 1 line
2.1.4
PATCH v3 2/6 mtd: m25p80: return amount of data transferred or error in read/write
In reply to Michal SuchanekAdd checking of SPI transfer errors and return them from read/write functions. Also return the amount of data transferred.
--- drivers/mtd/devices/m25p80.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
drivers/mtd/devices/m25p80.c +19 -9
@@ -83,6 +83,7 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,struct spi_transfer t[2 ] = {};struct spi_message m;int cmd_sz =m25p_cmdsz (nor); +ssize_t ret;spi_message_init (&m);
@@ -100,10 +101,15 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len, t[1 ].len = len;spi_message_add_tail (&t[1 ], &m); -spi_sync (spi, &m); + ret =spi_sync (spi, &m); +if (ret) +return ret; - *retlen += m.actual_length - cmd_sz; -return 0 ; + ret = m.actual_length - cmd_sz; +if (ret <0 ) +return -EIO ; + *retlen += ret; +return ret; }static inline unsigned int m25p80_rx_nbits (struct spi_nor *nor)
@@ -130,6 +136,7 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,struct spi_transfer t[2 ];struct spi_message m;unsigned int dummy = nor->read_dummy; +ssize_t ret;/* convert the dummy cycles to the number of bytes */ dummy /=8 ;
@@ -149,10 +156,15 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len, t[1 ].len = len;spi_message_add_tail (&t[1 ], &m); -spi_sync (spi, &m); + ret =spi_sync (spi, &m); +if (ret) +return ret; - *retlen = m.actual_length -m25p_cmdsz (nor) - dummy; -return 0 ; + ret = m.actual_length -m25p_cmdsz (nor) - dummy; +if (ret <0 ) +return -EIO ; + *retlen += ret; +return ret; }static int m25p80_erase (struct spi_nor *nor,loff_t offset)
@@ -166,9 +178,7 @@ static int m25p80_erase(struct spi_nor *nor, loff_t offset) flash->command[0 ] = nor->erase_opcode;m25p_addr2cmd (nor, offset, flash->command); -spi_write (flash->spi, flash->command,m25p_cmdsz (nor)); - -return 0 ; +return spi_write (flash->spi, flash->command,m25p_cmdsz (nor)); }/*
Signature · 1 line
2.1.4
PATCH v3 3/6 mtd: fsl-quadspi: return amount of data read/written or error
In reply to Michal SuchanekReturn amount of data read/written or error as read(2)/write(2) does.
--- drivers/mtd/spi-nor/fsl-quadspi.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
drivers/mtd/spi-nor/fsl-quadspi.c +11 -7
@@ -525,7 +525,7 @@ static inline void fsl_qspi_invalid(struct fsl_qspi *q)writel (reg, q->iobase +QUADSPI_MCR ); } -static int fsl_qspi_nor_write (struct fsl_qspi *q,struct spi_nor *nor, +static ssize_t fsl_qspi_nor_write (struct fsl_qspi *q,struct spi_nor *nor,u8 opcode,unsigned int to,u32 *txbuf,unsigned count,size_t *retlen) {
@@ -549,8 +549,11 @@ static int fsl_qspi_nor_write(struct fsl_qspi *q, struct spi_nor *nor,/* Trigger it */ ret =fsl_qspi_runcmd (q, opcode, to, count); -if (ret ==0 && retlen) - *retlen += count; +if (ret ==0 ) { +if (retlen) + *retlen += count; +return count; + }return ret; }
@@ -704,6 +707,8 @@ static int fsl_qspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len, }else if (len >0 ) { ret =fsl_qspi_nor_write (q, nor, opcode,0 , (u32 *)buf, len,NULL ); +if (ret >0 ) +return 0 ; }else {dev_err (q->dev,"invalid cmd %d\n" , opcode); ret = -EINVAL ;
@@ -717,12 +722,12 @@ static ssize_t fsl_qspi_write(struct spi_nor *nor, loff_t to, {struct fsl_qspi *q = nor->priv; -fsl_qspi_nor_write (q, nor, nor->program_opcode, to, +ssize_t ret =fsl_qspi_nor_write (q, nor, nor->program_opcode, to, (u32 *)buf, len, retlen);/* invalid the data in the AHB buffer. */ fsl_qspi_invalid (q); -return 0 ; +return ret; } static ssize_t fsl_qspi_read(struct spi_nor *nor, loff_t from,
@@ -737,8 +742,7 @@ static ssize_t fsl_qspi_read(struct spi_nor *nor, loff_t from,/* Read out the data directly from the AHB buffer.*/ memcpy (buf, q->ahb_base + q->chip_base_addr + from, len); - *retlen += len; -return 0 ; +return len; }static int fsl_qspi_erase (struct spi_nor *nor,loff_t offs)
Signature · 1 line
2.1.4
PATCH v3 4/6 mtd: spi-nor: check return value from read/write
In reply to Michal SuchanekSPI NOR hardware drivers now return useful value from their read/write functions so check them.
--- drivers/mtd/spi-nor/spi-nor.c | 50 +++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 14 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +36 -14
@@ -741,7 +741,10 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, ret = nor->read (nor, from, len, retlen, buf);spi_nor_unlock_and_unprep (nor,SPI_NOR_OPS_READ ); -return ret; +if (ret <0 ) +return ret; + +return 0 ; } static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
@@ -767,10 +770,14 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len, nor->program_opcode =SPINOR_OP_BP ;/* write one byte. */ - nor->write (nor, to,1 , retlen, buf); + ret = nor->write (nor, to,1 , retlen, buf); +if (ret <0 ) + goto sst_write_err; +WARN (ret !=1 ,"While writing 1 byte written %i bytes\n" , + (int )ret); ret =spi_nor_wait_till_ready (nor);if (ret) - goto time_out; + goto sst_write_err; } to += actual;
@@ -779,10 +786,14 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len, nor->program_opcode =SPINOR_OP_AAI_WP ;/* write two bytes. */ - nor->write (nor, to,2 , retlen, buf + actual); + ret = nor->write (nor, to,2 , retlen, buf + actual); +if (ret <0 ) + goto sst_write_err; +WARN (ret !=2 ,"While writing 2 bytes written %i bytes\n" , + (int )ret); ret =spi_nor_wait_till_ready (nor);if (ret) - goto time_out; + goto sst_write_err; to +=2 ; nor->sst_write_second = true; }
@@ -791,21 +802,24 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,write_disable (nor); ret =spi_nor_wait_till_ready (nor);if (ret) - goto time_out; + goto sst_write_err;/* Write out trailing byte if it exists. */ if (actual != len) {write_enable (nor); nor->program_opcode =SPINOR_OP_BP ; - nor->write (nor, to,1 , retlen, buf + actual); - + ret = nor->write (nor, to,1 , retlen, buf + actual); +if (ret <0 ) + goto sst_write_err; +WARN (ret !=1 ,"While writing 1 byte written %i bytes\n" , + (int )ret); ret =spi_nor_wait_till_ready (nor);if (ret) - goto time_out; + goto sst_write_err;write_disable (nor); } -time_out: +sst_write_err:spi_nor_unlock_and_unprep (nor,SPI_NOR_OPS_WRITE );return ret; }
@@ -834,14 +848,18 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,/* do all the bytes fit onto one page? */ if (page_offset + len <= nor->page_size) { - nor->write (nor, to, len, retlen, buf); + ret = nor->write (nor, to, len, retlen, buf); +if (ret <0 ) + goto write_err; }else {/* the size of data remaining on the first page */ page_size = nor->page_size - page_offset; - nor->write (nor, to, page_size, retlen, buf); + ret = nor->write (nor, to, page_size, retlen, buf); +if (ret <0 ) + goto write_err;/* write everything in nor->page_size chunks */ -for (i = page_size; i < len; i += page_size) { +for (i = ret; i < len; ) { page_size = len - i;if (page_size > nor->page_size) page_size = nor->page_size;
@@ -852,7 +870,11 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,write_enable (nor); - nor->write (nor, to + i, page_size, retlen, buf + i); + ret = nor->write (nor, to + i, page_size, retlen, + buf + i); +if (ret <0 ) + goto write_err; + i += ret; } }
Signature · 1 line
2.1.4
PATCH v3 5/6 mtd: spi-nor: stop passing around retlen
In reply to Michal SuchanekDo not pass retlen to hardware driver read/write functions. Update it in spi-nor generic driver instead.
--- drivers/mtd/devices/m25p80.c | 6 ++---- drivers/mtd/spi-nor/fsl-quadspi.c | 16 ++++++---------- drivers/mtd/spi-nor/spi-nor.c | 21 +++++++++++++-------- include/linux/mtd/spi-nor.h | 4 ++-- 4 files changed, 23 insertions(+), 24 deletions(-)
drivers/mtd/devices/m25p80.c +2 -4
@@ -76,7 +76,7 @@ static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len, }static ssize_t m25p80_write (struct spi_nor *nor,loff_t to,size_t len, -size_t *retlen,const u_char *buf) +const u_char *buf) {struct m25p *flash = nor->priv;struct spi_device *spi = flash->spi;
@@ -108,7 +108,6 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len, ret = m.actual_length - cmd_sz;if (ret <0 )return -EIO ; - *retlen += ret;return ret; }
@@ -129,7 +128,7 @@ static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)* may be any size provided it is within the physical boundaries. */ static ssize_t m25p80_read (struct spi_nor *nor,loff_t from,size_t len, -size_t *retlen,u_char *buf) +u_char *buf) {struct m25p *flash = nor->priv;struct spi_device *spi = flash->spi;
@@ -163,7 +162,6 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len, ret = m.actual_length -m25p_cmdsz (nor) - dummy;if (ret <0 )return -EIO ; - *retlen += ret;return ret; }
drivers/mtd/spi-nor/fsl-quadspi.c +6 -10
@@ -527,7 +527,7 @@ static inline void fsl_qspi_invalid(struct fsl_qspi *q)static ssize_t fsl_qspi_nor_write (struct fsl_qspi *q,struct spi_nor *nor,u8 opcode,unsigned int to,u32 *txbuf, -unsigned count,size_t *retlen) +unsigned count) {int ret, i, j;u32 tmp;
@@ -549,11 +549,8 @@ static ssize_t fsl_qspi_nor_write(struct fsl_qspi *q, struct spi_nor *nor,/* Trigger it */ ret =fsl_qspi_runcmd (q, opcode, to, count); -if (ret ==0 ) { -if (retlen) - *retlen += count; +if (ret ==0 )return count; - }return ret; }
@@ -706,7 +703,7 @@ static int fsl_qspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len, }else if (len >0 ) { ret =fsl_qspi_nor_write (q, nor, opcode,0 , - (u32 *)buf, len,NULL ); + (u32 *)buf, len);if (ret >0 )return 0 ; }else {
@@ -718,12 +715,11 @@ static int fsl_qspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len, }static ssize_t fsl_qspi_write (struct spi_nor *nor,loff_t to, -size_t len,size_t *retlen,const u_char *buf) +size_t len,const u_char *buf) {struct fsl_qspi *q = nor->priv; -ssize_t ret =fsl_qspi_nor_write (q, nor, nor->program_opcode, to, - (u32 *)buf, len, retlen); + (u32 *)buf, len);/* invalid the data in the AHB buffer. */ fsl_qspi_invalid (q);
@@ -731,7 +727,7 @@ static ssize_t fsl_qspi_write(struct spi_nor *nor, loff_t to, }static ssize_t fsl_qspi_read (struct spi_nor *nor,loff_t from, -size_t len,size_t *retlen,u_char *buf) +size_t len,u_char *buf) {struct fsl_qspi *q = nor->priv;u8 cmd = nor->read_opcode;
drivers/mtd/spi-nor/spi-nor.c +13 -8
@@ -738,12 +738,13 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,if (ret)return ret; - ret = nor->read (nor, from, len, retlen, buf); + ret = nor->read (nor, from, len, buf);spi_nor_unlock_and_unprep (nor,SPI_NOR_OPS_READ );if (ret <0 )return ret; + *retlen += ret;return 0 ; }
@@ -770,7 +771,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len, nor->program_opcode =SPINOR_OP_BP ;/* write one byte. */ - ret = nor->write (nor, to,1 , retlen, buf); + ret = nor->write (nor, to,1 , buf);if (ret <0 ) goto sst_write_err; WARN(ret != 1, "While writing 1 byte written %i bytes\n",
@@ -786,7 +787,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len, nor->program_opcode =SPINOR_OP_AAI_WP ;/* write two bytes. */ - ret = nor->write (nor, to,2 , retlen, buf + actual); + ret = nor->write (nor, to,2 , buf + actual);if (ret <0 ) goto sst_write_err; WARN(ret != 2, "While writing 2 bytes written %i bytes\n",
@@ -809,7 +810,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,write_enable (nor); nor->program_opcode =SPINOR_OP_BP ; - ret = nor->write (nor, to,1 , retlen, buf + actual); + ret = nor->write (nor, to,1 , buf + actual);if (ret <0 ) goto sst_write_err; WARN(ret != 1, "While writing 1 byte written %i bytes\n",
@@ -818,8 +819,10 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,if (ret) goto sst_write_err;write_disable (nor); + actual +=1 ; } sst_write_err: + *retlen += actual;spi_nor_unlock_and_unprep (nor,SPI_NOR_OPS_WRITE );return ret; }
@@ -848,15 +851,17 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,/* do all the bytes fit onto one page? */ if (page_offset + len <= nor->page_size) { - ret = nor->write (nor, to, len, retlen, buf); + ret = nor->write (nor, to, len, buf);if (ret <0 ) goto write_err; + *retlen += ret; }else {/* the size of data remaining on the first page */ page_size = nor->page_size - page_offset; - ret = nor->write (nor, to, page_size, retlen, buf); + ret = nor->write (nor, to, page_size, buf);if (ret <0 ) goto write_err; + *retlen += ret;/* write everything in nor->page_size chunks */ for (i = ret; i < len; ) {
@@ -870,10 +875,10 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,write_enable (nor); - ret = nor->write (nor, to + i, page_size, retlen, - buf + i); + ret = nor->write (nor, to + i, page_size, buf + i);if (ret <0 ) goto write_err; + *retlen += ret; i += ret; } }
include/linux/mtd/spi-nor.h +2 -2
@@ -186,9 +186,9 @@ struct spi_nor { int write_enable);ssize_t (*read)(struct spi_nor *nor,loff_t from, -size_t len,size_t *retlen,u_char *read_buf); +size_t len,u_char *read_buf);ssize_t (*write)(struct spi_nor *nor,loff_t to, -size_t len,size_t *retlen,const u_char *write_buf); +size_t len,const u_char *write_buf);int (*erase)(struct spi_nor *nor,loff_t offs);int (*flash_lock)(struct spi_nor *nor,loff_t ofs,uint64_t len);
Signature · 1 line
2.1.4
PATCH v3 6/6 mtd: spi-nor: simplify write loop
In reply to Michal SuchanekThe spi-nor write loop assumes that what is passed to the hardware driver write() is what gets written.
When write() writes less than page size at once data is dropped on the floor. Check the amount of data writen.
This also means that write can start mid-page any time so there is no special case for first page.
--- drivers/mtd/spi-nor/spi-nor.c | 48 ++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 33 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +15 -33
@@ -836,7 +836,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, const u_char *buf) { struct spi_nor *nor = mtd_to_spi_nor(mtd); -u32 page_offset, page_size, i; +size_t page_offset, page_remain, i;int ret;dev_dbg (nor->dev,"to 0x%08x, len %zd\n" , (u32 )to, len);
@@ -845,45 +845,27 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,if (ret)return ret; -write_enable (nor); - - page_offset = to & (nor->page_size -1 ); +for (i =0 ; i < len; ) { +int written; -/* do all the bytes fit onto one page? */ -if (page_offset + len <= nor->page_size) { - ret = nor->write (nor, to, len, buf); -if (ret <0 ) - goto write_err; - *retlen += ret; - }else { + page_offset = to & (nor->page_size -1 );/* the size of data remaining on the first page */ - page_size = nor->page_size - page_offset; - ret = nor->write (nor, to, page_size, buf); + page_remain =min_t (size_t, + nor->page_size - page_offset, len - i); + +write_enable (nor); + ret = nor->write (nor, to + i, page_remain, buf + i);if (ret <0 ) goto write_err; - *retlen += ret; - -/* write everything in nor->page_size chunks */ -for (i = ret; i < len; ) { - page_size = len - i; -if (page_size > nor->page_size) - page_size = nor->page_size; - - ret =spi_nor_wait_till_ready (nor); -if (ret) - goto write_err; + written = ret; -write_enable (nor); - - ret = nor->write (nor, to + i, page_size, buf + i); -if (ret <0 ) - goto write_err; - *retlen += ret; - i += ret; - } + ret =spi_nor_wait_till_ready (nor); +if (ret) + goto write_err; + *retlen += written; + i += written; } - ret =spi_nor_wait_till_ready (nor); write_err:spi_nor_unlock_and_unprep (nor,SPI_NOR_OPS_WRITE );return ret;
Signature · 1 line
2.1.4