Re: [PATCH] mtd: devices: fix mchp23k256 read and write

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Angelo Dureghello <angelo.dureghello@timesys.com>
Cc: richard@nod.at, linux-mtd@lists.infradead.org, vigneshr@ti.com,
	andrew@lunn.ch
Subject: Re: [PATCH] mtd: devices: fix mchp23k256 read and write
Date: Tue, 5 Nov 2019 19:41:22 +0100	[thread overview]
Message-ID: <20191105194122.4dcee126@xps13> (raw)
In-Reply-To: <20191030113957.1625342-1-angelo.dureghello@timesys.com>

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/