Message ID | 20191030113957.1625342-1-angelo.dureghello@timesys.com |
---|---|
State | Accepted |
Headers | show |
Series | mtd: devices: fix mchp23k256 read and write | expand |
Hi Angelo, + Andrew Angelo Dureghello <angelo.dureghello@timesys.com> wrote on Wed, 30 Oct 2019 12:39:57 +0100: > Due to the use of sizeof(), command size set for the spi transfer > was wrong. Driver was sending and receiving always 1 byte less > and especially on write, it was hanging. > > echo -n -e "\\x1\\x2\\x3\\x4" > /dev/mtd1 > > And read part too now works as expected. > > hexdump -C -n16 /dev/mtd1 > 00000000 01 02 03 04 ab f3 ad c2 ab e3 f4 36 dd 38 04 15 > 00000010 > Looks fine by me, just added Andrew in case he wants to review the patch as he was the original committer. Anyway you're missing a Cc: stable and Fixes tag on 5dc17fa6fb70 ("mtd: mchp23k256: Add driver for this SPI SRAM device") Thanks, Miquèl > Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> > --- > drivers/mtd/devices/mchp23k256.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c > index b20d02b4f830..77c872fd3d83 100644 > --- a/drivers/mtd/devices/mchp23k256.c > +++ b/drivers/mtd/devices/mchp23k256.c > @@ -64,15 +64,17 @@ static int mchp23k256_write(struct mtd_info *mtd, loff_t to, size_t len, > struct spi_transfer transfer[2] = {}; > struct spi_message message; > unsigned char command[MAX_CMD_SIZE]; > - int ret; > + int ret, cmd_len; > > spi_message_init(&message); > > + cmd_len = mchp23k256_cmdsz(flash); > + > command[0] = MCHP23K256_CMD_WRITE; > mchp23k256_addr2cmd(flash, to, command); > > transfer[0].tx_buf = command; > - transfer[0].len = mchp23k256_cmdsz(flash); > + transfer[0].len = cmd_len; > spi_message_add_tail(&transfer[0], &message); > > transfer[1].tx_buf = buf; > @@ -88,8 +90,8 @@ static int mchp23k256_write(struct mtd_info *mtd, loff_t to, size_t len, > if (ret) > return ret; > > - if (retlen && message.actual_length > sizeof(command)) > - *retlen += message.actual_length - sizeof(command); > + if (retlen && message.actual_length > cmd_len) > + *retlen += message.actual_length - cmd_len; > > return 0; > } > @@ -101,16 +103,18 @@ static int mchp23k256_read(struct mtd_info *mtd, loff_t from, size_t len, > struct spi_transfer transfer[2] = {}; > struct spi_message message; > unsigned char command[MAX_CMD_SIZE]; > - int ret; > + int ret, cmd_len; > > spi_message_init(&message); > > + cmd_len = mchp23k256_cmdsz(flash); > + > memset(&transfer, 0, sizeof(transfer)); > command[0] = MCHP23K256_CMD_READ; > mchp23k256_addr2cmd(flash, from, command); > > transfer[0].tx_buf = command; > - transfer[0].len = mchp23k256_cmdsz(flash); > + transfer[0].len = cmd_len; > spi_message_add_tail(&transfer[0], &message); > > transfer[1].rx_buf = buf; > @@ -126,8 +130,8 @@ static int mchp23k256_read(struct mtd_info *mtd, loff_t from, size_t len, > if (ret) > return ret; > > - if (retlen && message.actual_length > sizeof(command)) > - *retlen += message.actual_length - sizeof(command); > + if (retlen && message.actual_length > cmd_len) > + *retlen += message.actual_length - cmd_len; > > return 0; > }
On Tue, Nov 05, 2019 at 07:41:22PM +0100, Miquel Raynal wrote: > Hi Angelo, > > + Andrew > > Angelo Dureghello <angelo.dureghello@timesys.com> wrote on Wed, 30 Oct > 2019 12:39:57 +0100: > > > Due to the use of sizeof(), command size set for the spi transfer > > was wrong. Driver was sending and receiving always 1 byte less > > and especially on write, it was hanging. > > > > echo -n -e "\\x1\\x2\\x3\\x4" > /dev/mtd1 > > > > And read part too now works as expected. > > > > hexdump -C -n16 /dev/mtd1 > > 00000000 01 02 03 04 ab f3 ad c2 ab e3 f4 36 dd 38 04 15 > > 00000010 > > > > Looks fine by me, just added Andrew in case he wants to review the > patch as he was the original committer. > > Anyway you're missing a Cc: stable and Fixes tag on > 5dc17fa6fb70 ("mtd: mchp23k256: Add driver for this SPI SRAM device") Hi Miquèl This is the wrong fixes tag. The correct one is Fixes: 4379075a870b ("mtd: mchp23k256: Add support for mchp23lcv1024") Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
Hi Andrew, Andrew Lunn <andrew@lunn.ch> wrote on Wed, 6 Nov 2019 00:17:09 +0100: > On Tue, Nov 05, 2019 at 07:41:22PM +0100, Miquel Raynal wrote: > > Hi Angelo, > > > > + Andrew > > > > Angelo Dureghello <angelo.dureghello@timesys.com> wrote on Wed, 30 Oct > > 2019 12:39:57 +0100: > > > > > Due to the use of sizeof(), command size set for the spi transfer > > > was wrong. Driver was sending and receiving always 1 byte less > > > and especially on write, it was hanging. > > > > > > echo -n -e "\\x1\\x2\\x3\\x4" > /dev/mtd1 > > > > > > And read part too now works as expected. > > > > > > hexdump -C -n16 /dev/mtd1 > > > 00000000 01 02 03 04 ab f3 ad c2 ab e3 f4 36 dd 38 04 15 > > > 00000010 > > > > > > > Looks fine by me, just added Andrew in case he wants to review the > > patch as he was the original committer. > > > > Anyway you're missing a Cc: stable and Fixes tag on > > 5dc17fa6fb70 ("mtd: mchp23k256: Add driver for this SPI SRAM device") > > Hi Miquèl > > This is the wrong fixes tag. The correct one is > > Fixes: 4379075a870b ("mtd: mchp23k256: Add support for mchp23lcv1024") Oh crap, you're right, thanks for the correction! > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > Andrew Thanks, Miquèl
Hi all, many thanks. Let me know if i need to prepare a v1 with proper tags in case. Regards, Angelo On Wed, Nov 6, 2019 at 12:17 AM Andrew Lunn <andrew@lunn.ch> wrote: > > On Tue, Nov 05, 2019 at 07:41:22PM +0100, Miquel Raynal wrote: > > Hi Angelo, > > > > + Andrew > > > > Angelo Dureghello <angelo.dureghello@timesys.com> wrote on Wed, 30 Oct > > 2019 12:39:57 +0100: > > > > > Due to the use of sizeof(), command size set for the spi transfer > > > was wrong. Driver was sending and receiving always 1 byte less > > > and especially on write, it was hanging. > > > > > > echo -n -e "\\x1\\x2\\x3\\x4" > /dev/mtd1 > > > > > > And read part too now works as expected. > > > > > > hexdump -C -n16 /dev/mtd1 > > > 00000000 01 02 03 04 ab f3 ad c2 ab e3 f4 36 dd 38 04 15 > > > 00000010 > > > > > > > Looks fine by me, just added Andrew in case he wants to review the > > patch as he was the original committer. > > > > Anyway you're missing a Cc: stable and Fixes tag on > > 5dc17fa6fb70 ("mtd: mchp23k256: Add driver for this SPI SRAM device") > > Hi Miqučl > > This is the wrong fixes tag. The correct one is > > Fixes: 4379075a870b ("mtd: mchp23k256: Add support for mchp23lcv1024") > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > Andrew
On Wed, 2019-10-30 at 11:39:57 UTC, Angelo Dureghello wrote: > Due to the use of sizeof(), command size set for the spi transfer > was wrong. Driver was sending and receiving always 1 byte less > and especially on write, it was hanging. > > echo -n -e "\\x1\\x2\\x3\\x4" > /dev/mtd1 > > And read part too now works as expected. > > hexdump -C -n16 /dev/mtd1 > 00000000 01 02 03 04 ab f3 ad c2 ab e3 f4 36 dd 38 04 15 > 00000010 > > Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> > Reviewed-by: Andrew Lunn <andrew@lunn.ch> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks. Miquel
diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c index b20d02b4f830..77c872fd3d83 100644 --- a/drivers/mtd/devices/mchp23k256.c +++ b/drivers/mtd/devices/mchp23k256.c @@ -64,15 +64,17 @@ static int mchp23k256_write(struct mtd_info *mtd, loff_t to, size_t len, struct spi_transfer transfer[2] = {}; struct spi_message message; unsigned char command[MAX_CMD_SIZE]; - int ret; + int ret, cmd_len; spi_message_init(&message); + cmd_len = mchp23k256_cmdsz(flash); + command[0] = MCHP23K256_CMD_WRITE; mchp23k256_addr2cmd(flash, to, command); transfer[0].tx_buf = command; - transfer[0].len = mchp23k256_cmdsz(flash); + transfer[0].len = cmd_len; spi_message_add_tail(&transfer[0], &message); transfer[1].tx_buf = buf; @@ -88,8 +90,8 @@ static int mchp23k256_write(struct mtd_info *mtd, loff_t to, size_t len, if (ret) return ret; - if (retlen && message.actual_length > sizeof(command)) - *retlen += message.actual_length - sizeof(command); + if (retlen && message.actual_length > cmd_len) + *retlen += message.actual_length - cmd_len; return 0; } @@ -101,16 +103,18 @@ static int mchp23k256_read(struct mtd_info *mtd, loff_t from, size_t len, struct spi_transfer transfer[2] = {}; struct spi_message message; unsigned char command[MAX_CMD_SIZE]; - int ret; + int ret, cmd_len; spi_message_init(&message); + cmd_len = mchp23k256_cmdsz(flash); + memset(&transfer, 0, sizeof(transfer)); command[0] = MCHP23K256_CMD_READ; mchp23k256_addr2cmd(flash, from, command); transfer[0].tx_buf = command; - transfer[0].len = mchp23k256_cmdsz(flash); + transfer[0].len = cmd_len; spi_message_add_tail(&transfer[0], &message); transfer[1].rx_buf = buf; @@ -126,8 +130,8 @@ static int mchp23k256_read(struct mtd_info *mtd, loff_t from, size_t len, if (ret) return ret; - if (retlen && message.actual_length > sizeof(command)) - *retlen += message.actual_length - sizeof(command); + if (retlen && message.actual_length > cmd_len) + *retlen += message.actual_length - cmd_len; return 0; }
Due to the use of sizeof(), command size set for the spi transfer was wrong. Driver was sending and receiving always 1 byte less and especially on write, it was hanging. echo -n -e "\\x1\\x2\\x3\\x4" > /dev/mtd1 And read part too now works as expected. hexdump -C -n16 /dev/mtd1 00000000 01 02 03 04 ab f3 ad c2 ab e3 f4 36 dd 38 04 15 00000010 Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> --- drivers/mtd/devices/mchp23k256.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)