Re: [PATCH 13/39] mtd: nand: denali: increment ecc_stats->corrected
Hi Boris, 2016-11-28 0:31 GMT+09:00 Boris Brezillon: > On Sun, 27 Nov 2016 03:05:59 +0900 > Masahiro Yamada wrote: > >> Update the number of corrected bit flips when read_page() succeeds. >> >> Signed-off-by: Masahiro Yamada >> --- >> >> drivers/mtd/nand/denali.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c >> index a6445d9..4cc8945 100644 >> --- a/drivers/mtd/nand/denali.c >> +++ b/drivers/mtd/nand/denali.c >> @@ -1162,6 +1162,9 @@ static int denali_read_page(struct mtd_info *mtd, >> struct nand_chip *chip, >> mtd->ecc_stats.failed++; >> return 0; >> } >> + >> + mtd->ecc_stats.corrected += max_bitflips; > > First of all, ecc_stats.corrected should contain the total number of > bitflips detected on the MTD device, here you're just adding the > maximum number of bitflips per ECC chunk for the current page, which is > slightly different. > > Then, ecc_stats.corrected seems to be properly updated in handle_ecc() > [1], so I see no reason to do it here. > You are right. This patch is completely wrong, so I will drop it in v2. -- Best Regards Masahiro Yamada
Re: [PATCH 13/39] mtd: nand: denali: increment ecc_stats->corrected
Hi Boris, 2016-11-28 0:31 GMT+09:00 Boris Brezillon : > On Sun, 27 Nov 2016 03:05:59 +0900 > Masahiro Yamada wrote: > >> Update the number of corrected bit flips when read_page() succeeds. >> >> Signed-off-by: Masahiro Yamada >> --- >> >> drivers/mtd/nand/denali.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c >> index a6445d9..4cc8945 100644 >> --- a/drivers/mtd/nand/denali.c >> +++ b/drivers/mtd/nand/denali.c >> @@ -1162,6 +1162,9 @@ static int denali_read_page(struct mtd_info *mtd, >> struct nand_chip *chip, >> mtd->ecc_stats.failed++; >> return 0; >> } >> + >> + mtd->ecc_stats.corrected += max_bitflips; > > First of all, ecc_stats.corrected should contain the total number of > bitflips detected on the MTD device, here you're just adding the > maximum number of bitflips per ECC chunk for the current page, which is > slightly different. > > Then, ecc_stats.corrected seems to be properly updated in handle_ecc() > [1], so I see no reason to do it here. > You are right. This patch is completely wrong, so I will drop it in v2. -- Best Regards Masahiro Yamada
Re: [PATCH 13/39] mtd: nand: denali: increment ecc_stats->corrected
On Sun, 27 Nov 2016 03:05:59 +0900 Masahiro Yamadawrote: > Update the number of corrected bit flips when read_page() succeeds. > > Signed-off-by: Masahiro Yamada > --- > > drivers/mtd/nand/denali.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c > index a6445d9..4cc8945 100644 > --- a/drivers/mtd/nand/denali.c > +++ b/drivers/mtd/nand/denali.c > @@ -1162,6 +1162,9 @@ static int denali_read_page(struct mtd_info *mtd, > struct nand_chip *chip, > mtd->ecc_stats.failed++; > return 0; > } > + > + mtd->ecc_stats.corrected += max_bitflips; First of all, ecc_stats.corrected should contain the total number of bitflips detected on the MTD device, here you're just adding the maximum number of bitflips per ECC chunk for the current page, which is slightly different. Then, ecc_stats.corrected seems to be properly updated in handle_ecc() [1], so I see no reason to do it here. [1]http://lxr.free-electrons.com/source/drivers/mtd/nand/denali.c#L1003 > + > return max_bitflips; > } >
Re: [PATCH 13/39] mtd: nand: denali: increment ecc_stats->corrected
On Sun, 27 Nov 2016 03:05:59 +0900 Masahiro Yamada wrote: > Update the number of corrected bit flips when read_page() succeeds. > > Signed-off-by: Masahiro Yamada > --- > > drivers/mtd/nand/denali.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c > index a6445d9..4cc8945 100644 > --- a/drivers/mtd/nand/denali.c > +++ b/drivers/mtd/nand/denali.c > @@ -1162,6 +1162,9 @@ static int denali_read_page(struct mtd_info *mtd, > struct nand_chip *chip, > mtd->ecc_stats.failed++; > return 0; > } > + > + mtd->ecc_stats.corrected += max_bitflips; First of all, ecc_stats.corrected should contain the total number of bitflips detected on the MTD device, here you're just adding the maximum number of bitflips per ECC chunk for the current page, which is slightly different. Then, ecc_stats.corrected seems to be properly updated in handle_ecc() [1], so I see no reason to do it here. [1]http://lxr.free-electrons.com/source/drivers/mtd/nand/denali.c#L1003 > + > return max_bitflips; > } >