2015-08-14 10:02:12

by Andrew Murray

[permalink] [raw]
Subject: Re: [PATCH v4 7/7] mtd: spi-nor: add read loop

On 14 August 2015 at 10:23, Michal Suchanek <[email protected]> wrote:
> mtdblock and ubi do not handle the situation when read returns less data
> than requested. Loop in spi-nor until buffer is filled or an error is
> returned.
>
> Signed-off-by: Michal Suchanek <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index e0ae9cf..246fac7 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -738,14 +738,22 @@ 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, buf);
> + while (len) {
> + ret = nor->read(nor, from, len, buf);
> + if (ret <= 0)
> + goto read_err;
> +
> + BUG_ON(ret > len);
> + *retlen += ret;

Is *retlen initialized to 0 anywhere?

Andrew Murray

> + buf += ret;
> + from += ret;
> + len -= ret;
> + }
> + ret = 0;
>
> +read_err:
> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_READ);
> - if (ret < 0)
> - return ret;
> -
> - *retlen += ret;
> - return 0;
> + return ret;
> }
>
> static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
> --
> 2.1.4
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/


2015-08-14 10:08:43

by Michal Suchanek

[permalink] [raw]
Subject: Re: [PATCH v4 7/7] mtd: spi-nor: add read loop

On 14 August 2015 at 12:02, Andrew Murray <[email protected]> wrote:
> On 14 August 2015 at 10:23, Michal Suchanek <[email protected]> wrote:
>> mtdblock and ubi do not handle the situation when read returns less data
>> than requested. Loop in spi-nor until buffer is filled or an error is
>> returned.
>>
>> Signed-off-by: Michal Suchanek <[email protected]>
>> ---
>> drivers/mtd/spi-nor/spi-nor.c | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index e0ae9cf..246fac7 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -738,14 +738,22 @@ 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, buf);
>> + while (len) {
>> + ret = nor->read(nor, from, len, buf);
>> + if (ret <= 0)
>> + goto read_err;
>> +
>> + BUG_ON(ret > len);
>> + *retlen += ret;
>
> Is *retlen initialized to 0 anywhere?

It's initialized in mtdcore and passed into mtd->_read.

Yes, the interface is really awkward.

int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
u_char *buf)
{
int ret_code;
*retlen = 0;


Thanks

Michal

2015-11-05 03:55:23

by Hou Zhiqiang

[permalink] [raw]
Subject: RE: [PATCH v4 7/7] mtd: spi-nor: add read loop

Hi Michal,

Does this have any updates?

Thanks,
Zhiqiang

> -----Original Message-----
> From: Michal Suchanek [mailto:[email protected]]
> Sent: 2015年8月14日 18:08
> To: Andrew Murray
> Cc: Hou Zhiqiang-B48286; Huang Shijie; David Woodhouse; Brian Norris; Xu
> Han-B45815; Rafał Miłecki; Huang Shijie; Ben Hutchings; Marek Vasut;
> Gabor Juhos; Bean Huo 霍斌斌,; MTD Maling List; LKML
> Subject: Re: [PATCH v4 7/7] mtd: spi-nor: add read loop
>
> On 14 August 2015 at 12:02, Andrew Murray <[email protected]>
> wrote:
> > On 14 August 2015 at 10:23, Michal Suchanek <[email protected]> wrote:
> >> mtdblock and ubi do not handle the situation when read returns less
> >> data than requested. Loop in spi-nor until buffer is filled or an
> >> error is returned.
> >>
> >> Signed-off-by: Michal Suchanek <[email protected]>
> >> ---
> >> drivers/mtd/spi-nor/spi-nor.c | 20 ++++++++++++++------
> >> 1 file changed, 14 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c
> >> b/drivers/mtd/spi-nor/spi-nor.c index e0ae9cf..246fac7 100644
> >> --- a/drivers/mtd/spi-nor/spi-nor.c
> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> @@ -738,14 +738,22 @@ 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, buf);
> >> + while (len) {
> >> + ret = nor->read(nor, from, len, buf);
> >> + if (ret <= 0)
> >> + goto read_err;
> >> +
> >> + BUG_ON(ret > len);
> >> + *retlen += ret;
> >
> > Is *retlen initialized to 0 anywhere?
>
> It's initialized in mtdcore and passed into mtd->_read.
>
> Yes, the interface is really awkward.
>
> int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t
> *retlen,
> u_char *buf)
> {
> int ret_code;
> *retlen = 0;
>
>
> Thanks
>
> Michal
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-11-20 19:19:16

by Michal Suchanek

[permalink] [raw]
Subject: Re: [PATCH v4 7/7] mtd: spi-nor: add read loop

Hello,

On 5 November 2015 at 04:39, Hou Zhiqiang <[email protected]> wrote:
> Hi Michal,
>
> Does this have any updates?

I will try to update this patchset when it's agreed how the limit on
transfer size is handled. Writing less data than was requested is
acceptable for spi-nor but might disrupt other drivers so perhaps some
preemptive mechanism with the SPI master saying how much data it can
transfer and the drivers looking at that value would be preferred.

Thanks

Michal