All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Fainelli <florian.fainelli@broadcom.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>,
	David Regan <dregan@broadcom.com>
Cc: "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	linux-mtd@lists.infradead.org,
	bcm-kernel-feedback-list@broadcom.com,
	"William Zhang" <william.zhang@broadcom.com>,
	"Anand Gore" <anand.gore@broadcom.com>,
	"Kamal Dasu" <kamal.dasu@broadcom.com>,
	"Dan Beygelman" <dan.beygelman@broadcom.com>,
	"Álvaro Fernández Rojas" <noltari@gmail.com>,
	rafal@milecki.pl, computersforpeace@gmail.com,
	frieder.schrempf@kontron.de,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Boris Brezillon" <bbrezillon@kernel.org>,
	kdasu.kdev@gmail.com, JaimeLiao <jaimeliao.tw@gmail.com>,
	"Adam Borowski" <kilobyte@angband.pl>,
	"Jonas Gorski" <jonas.gorski@gmail.com>,
	dgcbueu@gmail.com, dregan@mail.com
Subject: Re: [PATCH v2] mtd: nand: brcmnand: fix mtd corrected bits stat
Date: Thu, 19 Jun 2025 11:11:20 -0700	[thread overview]
Message-ID: <44bc4834-da10-49fd-90bb-0ca63746aae8@broadcom.com> (raw)
In-Reply-To: <87h60dmn86.fsf@bootlin.com>

On 6/18/25 02:07, Miquel Raynal wrote:
> Hello David,
> 
>> I'm not familiar with nandbiterrs but here's the results from
>> mtd_nandbiterrs.ko on my NAND set to BCH8:
>>
>> # insmod mtd_nandbiterrs.ko dev=0
>> [  676.097190]
>> [  676.098760] ==================================================
>> [  676.104609] mtd_nandbiterrs: MTD device: 0
>> [  676.108732] mtd_nandbiterrs: MTD device size 2097152,
>> eraseblock=262144, page=4096, oob=216
>> [  676.117089] mtd_nandbiterrs: Device uses 1 subpages of 4096 bytes
>> [  676.123188] mtd_nandbiterrs: Using page=0, offset=0, eraseblock=0
>> [  676.130863] mtd_nandbiterrs: incremental biterrors test
>> [  676.136154] mtd_nandbiterrs: write_page
>> [  676.140761] mtd_nandbiterrs: rewrite page
>> [  676.145473] mtd_nandbiterrs: read_page
>> [  676.149621] mtd_nandbiterrs: verify_page
>> [  676.153625] mtd_nandbiterrs: Successfully corrected 0 bit errors per subpage
>> [  676.160678] mtd_nandbiterrs: Inserted biterror @ 0/5
>> [  676.165647] mtd_nandbiterrs: rewrite page
>> [  676.170363] mtd_nandbiterrs: read_page
>> [  676.174508] mtd_nandbiterrs: Read reported 1 corrected bit errors
>> [  676.180606] mtd_nandbiterrs: verify_page
>> [  676.184609] mtd_nandbiterrs: Successfully corrected 1 bit errors per subpage
>> [  676.191662] mtd_nandbiterrs: Inserted biterror @ 0/2
>> [  676.196631] mtd_nandbiterrs: rewrite page
>> [  676.201342] mtd_nandbiterrs: read_page
>> [  676.205487] mtd_nandbiterrs: Read reported 2 corrected bit errors
>> [  676.211586] mtd_nandbiterrs: verify_page
>> [  676.215588] mtd_nandbiterrs: Successfully corrected 2 bit errors per subpage
>> [  676.222641] mtd_nandbiterrs: Inserted biterror @ 0/0
>> [  676.227608] mtd_nandbiterrs: rewrite page
>> [  676.228356] mtd_nandbiterrs: read_page
>> [  676.228749] mtd_nandbiterrs: Read reported 3 corrected bit errors
>> [  676.228751] mtd_nandbiterrs: verify_page
>> [  676.228829] mtd_nandbiterrs: Successfully corrected 3 bit errors per subpage
>> [  676.228831] mtd_nandbiterrs: Inserted biterror @ 1/7
>> [  676.228833] mtd_nandbiterrs: rewrite page
>> [  676.229530] mtd_nandbiterrs: read_page
>> [  676.229922] mtd_nandbiterrs: Read reported 4 corrected bit errors
>> [  676.229924] mtd_nandbiterrs: verify_page
>> [  676.230001] mtd_nandbiterrs: Successfully corrected 4 bit errors per subpage
>> [  676.230003] mtd_nandbiterrs: Inserted biterror @ 1/5
>> [  676.230005] mtd_nandbiterrs: rewrite page
>> [  676.294177] mtd_nandbiterrs: read_page
>> [  676.298337] mtd_nandbiterrs: Read reported 5 corrected bit errors
>> [  676.304436] mtd_nandbiterrs: verify_page
>> [  676.308441] mtd_nandbiterrs: Successfully corrected 5 bit errors per subpage
>> [  676.315494] mtd_nandbiterrs: Inserted biterror @ 1/2
>> [  676.320464] mtd_nandbiterrs: rewrite page
>> [  676.325174] mtd_nandbiterrs: read_page
>> [  676.329327] mtd_nandbiterrs: Read reported 6 corrected bit errors
>> [  676.335426] mtd_nandbiterrs: verify_page
>> [  676.339429] mtd_nandbiterrs: Successfully corrected 6 bit errors per subpage
>> [  676.346483] mtd_nandbiterrs: Inserted biterror @ 1/0
>> [  676.351452] mtd_nandbiterrs: rewrite page
>> [  676.356162] mtd_nandbiterrs: read_page
>> [  676.360308] mtd_nandbiterrs: Read reported 7 corrected bit errors
>> [  676.366407] mtd_nandbiterrs: verify_page
>> [  676.370409] mtd_nandbiterrs: Successfully corrected 7 bit errors per subpage
>> [  676.377462] mtd_nandbiterrs: Inserted biterror @ 2/6
>> [  676.382432] mtd_nandbiterrs: rewrite page
>> [  676.387142] mtd_nandbiterrs: read_page
>> [  676.391287] mtd_nandbiterrs: Read reported 8 corrected bit errors
>> [  676.397385] mtd_nandbiterrs: verify_page
>> [  676.401388] mtd_nandbiterrs: Successfully corrected 8 bit errors
>> per subpage
> 
> So far the reporting looks good (and the nandflipbits output looks
> correct as well).
> 
>> [  676.408441] mtd_nandbiterrs: Inserted biterror @ 2/5
>> [  676.413411] mtd_nandbiterrs: rewrite page
>> [  676.418122] mtd_nandbiterrs: read_page
>> [  676.422267] mtd_nandbiterrs: verify_page
>> [  676.426194] mtd_nandbiterrs: Error: page offset 0, expected 25, got 00
>> [  676.432727] mtd_nandbiterrs: Error: page offset 1, expected a5, got 00
>> [  676.439260] mtd_nandbiterrs: Error: page offset 2, expected 65, got 05
>> [  676.445868] mtd_nandbiterrs: ECC failure, read data is incorrect
>> despite read success
> 
> Here however there is something wrong. We do expect a read failure,
> instead of returning wrong data. There is still a problem, I do not know
> if this problem was there before though, but this must be fixed.
> 
> Hello Florian, if you have time, I'd welcome you opinion on this patch :)

I don't think I am entitled any opinion on this patch other than 
confirming the register offset locations. Both William and David have 
far more experience with this NAND controller and devices that I do.
-- 
Florian

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

WARNING: multiple messages have this Message-ID (diff)
From: Florian Fainelli <florian.fainelli@broadcom.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>,
	David Regan <dregan@broadcom.com>
Cc: "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	linux-mtd@lists.infradead.org,
	bcm-kernel-feedback-list@broadcom.com,
	"William Zhang" <william.zhang@broadcom.com>,
	"Anand Gore" <anand.gore@broadcom.com>,
	"Kamal Dasu" <kamal.dasu@broadcom.com>,
	"Dan Beygelman" <dan.beygelman@broadcom.com>,
	"Álvaro Fernández Rojas" <noltari@gmail.com>,
	rafal@milecki.pl, computersforpeace@gmail.com,
	frieder.schrempf@kontron.de,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Boris Brezillon" <bbrezillon@kernel.org>,
	kdasu.kdev@gmail.com, JaimeLiao <jaimeliao.tw@gmail.com>,
	"Adam Borowski" <kilobyte@angband.pl>,
	"Jonas Gorski" <jonas.gorski@gmail.com>,
	dgcbueu@gmail.com, dregan@mail.com
Subject: Re: [PATCH v2] mtd: nand: brcmnand: fix mtd corrected bits stat
Date: Thu, 19 Jun 2025 11:11:20 -0700	[thread overview]
Message-ID: <44bc4834-da10-49fd-90bb-0ca63746aae8@broadcom.com> (raw)
In-Reply-To: <87h60dmn86.fsf@bootlin.com>

On 6/18/25 02:07, Miquel Raynal wrote:
> Hello David,
> 
>> I'm not familiar with nandbiterrs but here's the results from
>> mtd_nandbiterrs.ko on my NAND set to BCH8:
>>
>> # insmod mtd_nandbiterrs.ko dev=0
>> [  676.097190]
>> [  676.098760] ==================================================
>> [  676.104609] mtd_nandbiterrs: MTD device: 0
>> [  676.108732] mtd_nandbiterrs: MTD device size 2097152,
>> eraseblock=262144, page=4096, oob=216
>> [  676.117089] mtd_nandbiterrs: Device uses 1 subpages of 4096 bytes
>> [  676.123188] mtd_nandbiterrs: Using page=0, offset=0, eraseblock=0
>> [  676.130863] mtd_nandbiterrs: incremental biterrors test
>> [  676.136154] mtd_nandbiterrs: write_page
>> [  676.140761] mtd_nandbiterrs: rewrite page
>> [  676.145473] mtd_nandbiterrs: read_page
>> [  676.149621] mtd_nandbiterrs: verify_page
>> [  676.153625] mtd_nandbiterrs: Successfully corrected 0 bit errors per subpage
>> [  676.160678] mtd_nandbiterrs: Inserted biterror @ 0/5
>> [  676.165647] mtd_nandbiterrs: rewrite page
>> [  676.170363] mtd_nandbiterrs: read_page
>> [  676.174508] mtd_nandbiterrs: Read reported 1 corrected bit errors
>> [  676.180606] mtd_nandbiterrs: verify_page
>> [  676.184609] mtd_nandbiterrs: Successfully corrected 1 bit errors per subpage
>> [  676.191662] mtd_nandbiterrs: Inserted biterror @ 0/2
>> [  676.196631] mtd_nandbiterrs: rewrite page
>> [  676.201342] mtd_nandbiterrs: read_page
>> [  676.205487] mtd_nandbiterrs: Read reported 2 corrected bit errors
>> [  676.211586] mtd_nandbiterrs: verify_page
>> [  676.215588] mtd_nandbiterrs: Successfully corrected 2 bit errors per subpage
>> [  676.222641] mtd_nandbiterrs: Inserted biterror @ 0/0
>> [  676.227608] mtd_nandbiterrs: rewrite page
>> [  676.228356] mtd_nandbiterrs: read_page
>> [  676.228749] mtd_nandbiterrs: Read reported 3 corrected bit errors
>> [  676.228751] mtd_nandbiterrs: verify_page
>> [  676.228829] mtd_nandbiterrs: Successfully corrected 3 bit errors per subpage
>> [  676.228831] mtd_nandbiterrs: Inserted biterror @ 1/7
>> [  676.228833] mtd_nandbiterrs: rewrite page
>> [  676.229530] mtd_nandbiterrs: read_page
>> [  676.229922] mtd_nandbiterrs: Read reported 4 corrected bit errors
>> [  676.229924] mtd_nandbiterrs: verify_page
>> [  676.230001] mtd_nandbiterrs: Successfully corrected 4 bit errors per subpage
>> [  676.230003] mtd_nandbiterrs: Inserted biterror @ 1/5
>> [  676.230005] mtd_nandbiterrs: rewrite page
>> [  676.294177] mtd_nandbiterrs: read_page
>> [  676.298337] mtd_nandbiterrs: Read reported 5 corrected bit errors
>> [  676.304436] mtd_nandbiterrs: verify_page
>> [  676.308441] mtd_nandbiterrs: Successfully corrected 5 bit errors per subpage
>> [  676.315494] mtd_nandbiterrs: Inserted biterror @ 1/2
>> [  676.320464] mtd_nandbiterrs: rewrite page
>> [  676.325174] mtd_nandbiterrs: read_page
>> [  676.329327] mtd_nandbiterrs: Read reported 6 corrected bit errors
>> [  676.335426] mtd_nandbiterrs: verify_page
>> [  676.339429] mtd_nandbiterrs: Successfully corrected 6 bit errors per subpage
>> [  676.346483] mtd_nandbiterrs: Inserted biterror @ 1/0
>> [  676.351452] mtd_nandbiterrs: rewrite page
>> [  676.356162] mtd_nandbiterrs: read_page
>> [  676.360308] mtd_nandbiterrs: Read reported 7 corrected bit errors
>> [  676.366407] mtd_nandbiterrs: verify_page
>> [  676.370409] mtd_nandbiterrs: Successfully corrected 7 bit errors per subpage
>> [  676.377462] mtd_nandbiterrs: Inserted biterror @ 2/6
>> [  676.382432] mtd_nandbiterrs: rewrite page
>> [  676.387142] mtd_nandbiterrs: read_page
>> [  676.391287] mtd_nandbiterrs: Read reported 8 corrected bit errors
>> [  676.397385] mtd_nandbiterrs: verify_page
>> [  676.401388] mtd_nandbiterrs: Successfully corrected 8 bit errors
>> per subpage
> 
> So far the reporting looks good (and the nandflipbits output looks
> correct as well).
> 
>> [  676.408441] mtd_nandbiterrs: Inserted biterror @ 2/5
>> [  676.413411] mtd_nandbiterrs: rewrite page
>> [  676.418122] mtd_nandbiterrs: read_page
>> [  676.422267] mtd_nandbiterrs: verify_page
>> [  676.426194] mtd_nandbiterrs: Error: page offset 0, expected 25, got 00
>> [  676.432727] mtd_nandbiterrs: Error: page offset 1, expected a5, got 00
>> [  676.439260] mtd_nandbiterrs: Error: page offset 2, expected 65, got 05
>> [  676.445868] mtd_nandbiterrs: ECC failure, read data is incorrect
>> despite read success
> 
> Here however there is something wrong. We do expect a read failure,
> instead of returning wrong data. There is still a problem, I do not know
> if this problem was there before though, but this must be fixed.
> 
> Hello Florian, if you have time, I'd welcome you opinion on this patch :)

I don't think I am entitled any opinion on this patch other than 
confirming the register offset locations. Both William and David have 
far more experience with this NAND controller and devices that I do.
-- 
Florian

  reply	other threads:[~2025-06-19 18:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-06 16:57 [PATCH v2] mtd: nand: brcmnand: fix mtd corrected bits stat David Regan
2025-06-06 16:57 ` David Regan
2025-06-09  9:20 ` Miquel Raynal
2025-06-09  9:20   ` Miquel Raynal
2025-06-11  0:44   ` David Regan
2025-06-11  0:44     ` David Regan
2025-06-18  9:07     ` Miquel Raynal
2025-06-18  9:07       ` Miquel Raynal
2025-06-19 18:11       ` Florian Fainelli [this message]
2025-06-19 18:11         ` Florian Fainelli
2025-06-19 18:23       ` David Regan
2025-06-19 18:23         ` David Regan
2025-06-26 16:15       ` David Regan
2025-06-26 16:15         ` David Regan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44bc4834-da10-49fd-90bb-0ca63746aae8@broadcom.com \
    --to=florian.fainelli@broadcom.com \
    --cc=anand.gore@broadcom.com \
    --cc=bbrezillon@kernel.org \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=computersforpeace@gmail.com \
    --cc=dan.beygelman@broadcom.com \
    --cc=dgcbueu@gmail.com \
    --cc=dregan@broadcom.com \
    --cc=dregan@mail.com \
    --cc=frieder.schrempf@kontron.de \
    --cc=jaimeliao.tw@gmail.com \
    --cc=jonas.gorski@gmail.com \
    --cc=kamal.dasu@broadcom.com \
    --cc=kdasu.kdev@gmail.com \
    --cc=kilobyte@angband.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=noltari@gmail.com \
    --cc=rafal@milecki.pl \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.com \
    --cc=william.zhang@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.