Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mtd: spinand: fix error read return value
@ 2019-06-19 13:13 liaoweixiong
  2019-06-19 13:46 ` Boris Brezillon
  0 siblings, 1 reply; 5+ messages in thread
From: liaoweixiong @ 2019-06-19 13:13 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, David Woodhouse, Brian Norris,
	Marek Vasut, Vignesh Raghavendra, Boris Brezillon,
	Frieder Schrempf, Peter Pan, Clément Péron,
	Chuanhong Guo
  Cc: liaoweixiong, linux-mtd, linux-kernel

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 <liaoweixiong@allwinnertech.com>
---
 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


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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: spinand: fix error read return value
  2019-06-19 13:13 [PATCH] mtd: spinand: fix error read return value liaoweixiong
@ 2019-06-19 13:46 ` Boris Brezillon
  2019-06-19 14:02   ` Schrempf Frieder
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Brezillon @ 2019-06-19 13:46 UTC (permalink / raw)
  To: liaoweixiong
  Cc: Vignesh Raghavendra, Boris Brezillon, Richard Weinberger,
	linux-kernel, Chuanhong Guo, Marek Vasut, Frieder Schrempf,
	Clément Péron, Miquel Raynal, linux-mtd, Brian Norris,
	David Woodhouse, Peter Pan

Hi liaoweixiong,

On Wed, 19 Jun 2019 21:13:24 +0800
liaoweixiong <liaoweixiong@allwinnertech.com> 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 <liaoweixiong@allwinnertech.com>
> ---
>  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/

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: spinand: fix error read return value
  2019-06-19 13:46 ` Boris Brezillon
@ 2019-06-19 14:02   ` Schrempf Frieder
  2019-06-19 16:18     ` Boris Brezillon
  0 siblings, 1 reply; 5+ messages in thread
From: Schrempf Frieder @ 2019-06-19 14:02 UTC (permalink / raw)
  To: Boris Brezillon, liaoweixiong
  Cc: Vignesh Raghavendra, Boris Brezillon, Richard Weinberger,
	linux-kernel@vger.kernel.org, Marek Vasut, Frieder Schrempf,
	Clément Péron, Miquel Raynal,
	linux-mtd@lists.infradead.org, Brian Norris, Chuanhong Guo,
	David Woodhouse, Peter Pan

On 19.06.19 15:46, Boris Brezillon wrote:
> Hi liaoweixiong,
> 
> On Wed, 19 Jun 2019 21:13:24 +0800
> liaoweixiong <liaoweixiong@allwinnertech.com> 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 <liaoweixiong@allwinnertech.com>
>> ---
>>   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/
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: spinand: fix error read return value
  2019-06-19 14:02   ` Schrempf Frieder
@ 2019-06-19 16:18     ` Boris Brezillon
  2019-06-20  0:40       ` liaoweixiong
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Brezillon @ 2019-06-19 16:18 UTC (permalink / raw)
  To: Schrempf Frieder
  Cc: Vignesh Raghavendra, Boris Brezillon, Richard Weinberger,
	linux-kernel@vger.kernel.org, liaoweixiong, Marek Vasut,
	Frieder Schrempf, Clément Péron, Miquel Raynal,
	linux-mtd@lists.infradead.org, Brian Norris, Chuanhong Guo,
	David Woodhouse, Peter Pan

On Wed, 19 Jun 2019 14:02:14 +0000
Schrempf Frieder <frieder.schrempf@kontron.de> wrote:

> On 19.06.19 15:46, Boris Brezillon wrote:
> > Hi liaoweixiong,
> > 
> > On Wed, 19 Jun 2019 21:13:24 +0800
> > liaoweixiong <liaoweixiong@allwinnertech.com> 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 <liaoweixiong@allwinnertech.com>
> >> ---
> >>   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/
> >  


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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: spinand: fix error read return value
  2019-06-19 16:18     ` Boris Brezillon
@ 2019-06-20  0:40       ` liaoweixiong
  0 siblings, 0 replies; 5+ messages in thread
From: liaoweixiong @ 2019-06-20  0:40 UTC (permalink / raw)
  To: Boris Brezillon, Schrempf Frieder
  Cc: Vignesh Raghavendra, Boris Brezillon, Richard Weinberger,
	linux-kernel@vger.kernel.org, Marek Vasut, Frieder Schrempf,
	Clément Péron, Miquel Raynal,
	linux-mtd@lists.infradead.org, Brian Norris, Chuanhong Guo,
	David Woodhouse, Peter Pan

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 <frieder.schrempf@kontron.de> wrote:
> 
>> On 19.06.19 15:46, Boris Brezillon wrote:
>>> Hi liaoweixiong,
>>>
>>> On Wed, 19 Jun 2019 21:13:24 +0800
>>> liaoweixiong <liaoweixiong@allwinnertech.com> 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 <liaoweixiong@allwinnertech.com>
>>>> ---
>>>>   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

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-20  0:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-19 13:13 [PATCH] mtd: spinand: fix error read return value liaoweixiong
2019-06-19 13:46 ` Boris Brezillon
2019-06-19 14:02   ` Schrempf Frieder
2019-06-19 16:18     ` Boris Brezillon
2019-06-20  0:40       ` liaoweixiong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox