diff mbox series

mtd: devices: fix mchp23k256 read and write

Message ID 20191030113957.1625342-1-angelo.dureghello@timesys.com
State Accepted
Headers show
Series mtd: devices: fix mchp23k256 read and write | expand

Commit Message

Angelo Dureghello Oct. 30, 2019, 11:39 a.m. UTC 
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(-)

Comments

Miquel Raynal Nov. 5, 2019, 6:41 p.m. UTC | #1 
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;
>  }
Andrew Lunn Nov. 5, 2019, 11:17 p.m. UTC | #2 
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
Miquel Raynal Nov. 5, 2019, 11:39 p.m. UTC | #3 
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
Angelo Dureghello Nov. 11, 2019, 4:56 p.m. UTC | #4 
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
Miquel Raynal Nov. 12, 2019, 2:19 p.m. UTC | #5 
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 mbox series

Patch

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;
 }