In function spinand_mtd_read, if the last page to read occurs bitflip,
this function will return error value because veriable ret not equal to 0.
Signed-off-by: liaoweixiong <[email protected]>
---
drivers/mtd/nand/spi/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 556bfdb..6b9388d 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -511,12 +511,12 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
if (ret == -EBADMSG) {
ecc_failed = true;
mtd->ecc_stats.failed++;
- ret = 0;
} else {
mtd->ecc_stats.corrected += ret;
max_bitflips = max_t(unsigned int, max_bitflips, ret);
}
+ ret = 0;
ops->retlen += iter.req.datalen;
ops->oobretlen += iter.req.ooblen;
}
--
1.9.1
Hi liaoweixiong,
On Wed, 19 Jun 2019 21:13:24 +0800
liaoweixiong <[email protected]> wrote:
> In function spinand_mtd_read, if the last page to read occurs bitflip,
> this function will return error value because veriable ret not equal to 0.
Actually, that's exactly what the MTD core expects (see [1]), so you're
the one introducing a regression here.
Regards,
Boris
>
> Signed-off-by: liaoweixiong <[email protected]>
> ---
> drivers/mtd/nand/spi/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 556bfdb..6b9388d 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -511,12 +511,12 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
> if (ret == -EBADMSG) {
> ecc_failed = true;
> mtd->ecc_stats.failed++;
> - ret = 0;
> } else {
> mtd->ecc_stats.corrected += ret;
> max_bitflips = max_t(unsigned int, max_bitflips, ret);
> }
>
> + ret = 0;
> ops->retlen += iter.req.datalen;
> ops->oobretlen += iter.req.ooblen;
> }
[1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/mtdcore.c#L1209
On 19.06.19 15:46, Boris Brezillon wrote:
> Hi liaoweixiong,
>
> On Wed, 19 Jun 2019 21:13:24 +0800
> liaoweixiong <[email protected]> wrote:
>
>> In function spinand_mtd_read, if the last page to read occurs bitflip,
>> this function will return error value because veriable ret not equal to 0.
>
> Actually, that's exactly what the MTD core expects (see [1]), so you're
> the one introducing a regression here.
To me it looks like the patch description is somewhat incorrect, but the
fix itself looks okay, unless I'm getting it wrong.
In case of the last page containing bitflips (ret > 0),
spinand_mtd_read() will return that number of bitflips for the last
page. But to me it looks like it should instead return max_bitflips like
it does when the last page read returns with 0.
>>
>> Signed-off-by: liaoweixiong <[email protected]>
>> ---
>> drivers/mtd/nand/spi/core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
>> index 556bfdb..6b9388d 100644
>> --- a/drivers/mtd/nand/spi/core.c
>> +++ b/drivers/mtd/nand/spi/core.c
>> @@ -511,12 +511,12 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
>> if (ret == -EBADMSG) {
>> ecc_failed = true;
>> mtd->ecc_stats.failed++;
>> - ret = 0;
>> } else {
>> mtd->ecc_stats.corrected += ret;
>> max_bitflips = max_t(unsigned int, max_bitflips, ret);
>> }
>>
>> + ret = 0;
>> ops->retlen += iter.req.datalen;
>> ops->oobretlen += iter.req.ooblen;
>> }
>
> [1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/mtdcore.c#L1209
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
On Wed, 19 Jun 2019 14:02:14 +0000
Schrempf Frieder <[email protected]> wrote:
> On 19.06.19 15:46, Boris Brezillon wrote:
> > Hi liaoweixiong,
> >
> > On Wed, 19 Jun 2019 21:13:24 +0800
> > liaoweixiong <[email protected]> wrote:
> >
> >> In function spinand_mtd_read, if the last page to read occurs bitflip,
> >> this function will return error value because veriable ret not equal to 0.
> >
> > Actually, that's exactly what the MTD core expects (see [1]), so you're
> > the one introducing a regression here.
>
> To me it looks like the patch description is somewhat incorrect, but the
> fix itself looks okay, unless I'm getting it wrong.
>
> In case of the last page containing bitflips (ret > 0),
> spinand_mtd_read() will return that number of bitflips for the last
> page. But to me it looks like it should instead return max_bitflips like
> it does when the last page read returns with 0.
Oh, you're right. liaoweixiong, can you adjust the commit message
accordingly?
>
> >>
> >> Signed-off-by: liaoweixiong <[email protected]>
> >> ---
> >> drivers/mtd/nand/spi/core.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> >> index 556bfdb..6b9388d 100644
> >> --- a/drivers/mtd/nand/spi/core.c
> >> +++ b/drivers/mtd/nand/spi/core.c
> >> @@ -511,12 +511,12 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
> >> if (ret == -EBADMSG) {
> >> ecc_failed = true;
> >> mtd->ecc_stats.failed++;
> >> - ret = 0;
> >> } else {
> >> mtd->ecc_stats.corrected += ret;
> >> max_bitflips = max_t(unsigned int, max_bitflips, ret);
> >> }
> >>
> >> + ret = 0;
> >> ops->retlen += iter.req.datalen;
> >> ops->oobretlen += iter.req.ooblen;
> >> }
> >
> > [1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/mtdcore.c#L1209
> >
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> >
hi Boris Brezillon,
Sure, i will adjust the commit message and send again right now.
On 2019/6/20 AM12:18, Boris Brezillon wrote:
> On Wed, 19 Jun 2019 14:02:14 +0000
> Schrempf Frieder <[email protected]> wrote:
>
>> On 19.06.19 15:46, Boris Brezillon wrote:
>>> Hi liaoweixiong,
>>>
>>> On Wed, 19 Jun 2019 21:13:24 +0800
>>> liaoweixiong <[email protected]> wrote:
>>>
>>>> In function spinand_mtd_read, if the last page to read occurs bitflip,
>>>> this function will return error value because veriable ret not equal to 0.
>>>
>>> Actually, that's exactly what the MTD core expects (see [1]), so you're
>>> the one introducing a regression here.
>>
>> To me it looks like the patch description is somewhat incorrect, but the
>> fix itself looks okay, unless I'm getting it wrong.
>>
>> In case of the last page containing bitflips (ret > 0),
>> spinand_mtd_read() will return that number of bitflips for the last
>> page. But to me it looks like it should instead return max_bitflips like
>> it does when the last page read returns with 0.
>
> Oh, you're right. liaoweixiong, can you adjust the commit message
> accordingly?
>
>>
>>>>
>>>> Signed-off-by: liaoweixiong <[email protected]>
>>>> ---
>>>> drivers/mtd/nand/spi/core.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
>>>> index 556bfdb..6b9388d 100644
>>>> --- a/drivers/mtd/nand/spi/core.c
>>>> +++ b/drivers/mtd/nand/spi/core.c
>>>> @@ -511,12 +511,12 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
>>>> if (ret == -EBADMSG) {
>>>> ecc_failed = true;
>>>> mtd->ecc_stats.failed++;
>>>> - ret = 0;
>>>> } else {
>>>> mtd->ecc_stats.corrected += ret;
>>>> max_bitflips = max_t(unsigned int, max_bitflips, ret);
>>>> }
>>>>
>>>> + ret = 0;
>>>> ops->retlen += iter.req.datalen;
>>>> ops->oobretlen += iter.req.ooblen;
>>>> }
>>>
>>> [1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/mtdcore.c#L1209
>>>
>>> ______________________________________________________
>>> Linux MTD discussion mailing list
>>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>>
--
liaoweixiong