[PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Johan Jonker]

The Rockchip boot ROM only checks for NAND chip 0 and with
reduced ECC strength. Currently only the read page functions
have this condition check added.

Fix by adding the same condition to all read and write page
functions by dropping the existing 'selected_bank == 0' check
and use the NAND_IS_BOOT_MEDIUM option that was introduced to
U-Boot more recently than this driver to behave
identically to the Linux driver.

It is now the users responsibility to apply the device tree
property "nand-is-boot-medium" to only NAND chip 0.

Fixes: b12dc5d6fa76 ("mtd: nand: NFC drivers for RK3308, RK2928 and others")
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Tested-by: Hüseyin BIYIK <boogiepop@gmx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changed V2:
fix typos
reword
---
 drivers/mtd/nand/raw/rockchip_nfc.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/rockchip_nfc.c b/drivers/mtd/nand/raw/rockchip_nfc.c
index ea8e67d1a23e..e788fda22a95 100644
--- a/drivers/mtd/nand/raw/rockchip_nfc.c
+++ b/drivers/mtd/nand/raw/rockchip_nfc.c
@@ -440,7 +440,8 @@ static int rk_nfc_write_page_raw(struct mtd_info *mtd,
 	int i, pages_per_blk;
 
 	pages_per_blk = mtd->erasesize / mtd->writesize;
-	if ((page < (pages_per_blk * rknand->boot_blks)) &&
+	if ((chip->options & NAND_IS_BOOT_MEDIUM) &&
+	    (page < (pages_per_blk * rknand->boot_blks)) &&
 	    rknand->boot_ecc != ecc->strength) {
 		/*
 		 * There's currently no method to notify the MTD framework that
@@ -560,7 +561,8 @@ static int rk_nfc_write_page_hwecc(struct mtd_info *mtd,
 	 *
 	 * Configure the ECC algorithm supported by the boot ROM.
 	 */
-	if (page < (pages_per_blk * rknand->boot_blks)) {
+	if ((chip->options & NAND_IS_BOOT_MEDIUM) &&
+	    (page < (pages_per_blk * rknand->boot_blks))) {
 		boot_rom_mode = 1;
 		if (rknand->boot_ecc != ecc->strength)
 			rk_nfc_hw_ecc_setup(chip, rknand->boot_ecc);
@@ -624,8 +626,8 @@ static int rk_nfc_read_page_raw(struct mtd_info *mtd,
 	int i, pages_per_blk;
 
 	pages_per_blk = mtd->erasesize / mtd->writesize;
-	if ((page < (pages_per_blk * rknand->boot_blks)) &&
-	    nfc->selected_bank == 0 &&
+	if ((chip->options & NAND_IS_BOOT_MEDIUM) &&
+	    (page < (pages_per_blk * rknand->boot_blks)) &&
 	    rknand->boot_ecc != ecc->strength) {
 		/*
 		 * There's currently no method to notify the MTD framework that
@@ -700,8 +702,8 @@ static int rk_nfc_read_page_hwecc(struct mtd_info *mtd,
 	 * are used by the boot ROM.
 	 * Configure the ECC algorithm supported by the boot ROM.
 	 */
-	if (page < (pages_per_blk * rknand->boot_blks) &&
-	    nfc->selected_bank == 0) {
+	if ((chip->options & NAND_IS_BOOT_MEDIUM) &&
+	    (page < (pages_per_blk * rknand->boot_blks))) {
 		boot_rom_mode = 1;
 		if (rknand->boot_ecc != ecc->strength)
 			rk_nfc_hw_ecc_setup(chip, rknand->boot_ecc);
-- 
2.39.5

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Quentin Schulz via U-Boot]

Hi Johan,

Resending because the ML rejected my mail sent from my other address... 
I have to figure out what I set up wrong to trigger the spam filter :)

On 7/14/26 8:39 AM, Johan Jonker wrote:
 > The Rockchip boot ROM only checks for NAND chip 0 and with
 > reduced ECC strength. Currently only the read page functions
 > have this condition check added.
 >
 > Fix by adding the same condition to all read and write page
 > functions by dropping the existing 'selected_bank == 0' check
 > and use the NAND_IS_BOOT_MEDIUM option that was introduced to
 > U-Boot more recently than this driver to behave
 > identically to the Linux driver.
 >
 > It is now the users responsibility to apply the device tree
 > property "nand-is-boot-medium" to only NAND chip 0.
 >
 > Fixes: b12dc5d6fa76 ("mtd: nand: NFC drivers for RK3308, RK2928 and 
others")
 > Signed-off-by: Johan Jonker <jbx6244@gmail.com>
 > Tested-by: Hüseyin BIYIK <boogiepop@gmx.com>
 > Reviewed-by: Simon Glass <sjg@chromium.org>

You don't explain how the bug can be triggered. It'd be nice to provide 
the usecase when this is an issue so that other people looking on the 
Internet for bug reports could somehow stumble upon this patch.

I'm thinking the issue is that we currently verify all NAND chips use 
the boot_blks and boot_ecc from the boot medium whereas they might not 
be used as a boot medium (they are missing the nand-is-boot-medium 
property) so we cannot actually make use of them. Is that correct?

Considering boot_blks is 0 if rockchip,boot-blks property isn't set, 
we'll never be able to meet the page < pages_per_blk * 0) condition 
anyway so we would never enter the if block... or can page actually be 
negative????

To be clear, I don't disagree with the fix, I just am missing a lot of 
information that should be in the commit log.

Cheers,
Quentin

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Boogie via U-Boot]

Hello Quentin

I had reported this bug to Johan so i can give detailed explanation.

The bug was really the lock of "&& nfc->selected_bank == 0" not existing 
for write_page_* variants.

When rockchip nfc was reading, it required bootblks to be a part of the 
first nand chip only with the condition "nfc->selected_bank == 0". But 
when writing it interpreted 'all' nand chips first boot_blks as boot blocks.

The practical problem. I had mentioned this in V1 and give more explicit 
details here.

I have 2 nand chips, and a partition (linux below) starts in nand chip 1 
(CS=0 in below) and ends in chip2 (CS=2 below). And i am using UBI on 
top of that mtd.

nand@0 {
     reg = <0>, <2>;
     label = "rk-nand-0";
     nand-bus-width = <8>;
     nand-ecc-mode = "hw";
     nand-ecc-step-size = <1024>;
     nand-ecc-strength = <40>;
     nand-is-boot-medium;
     rockchip,boot-blks = <8>;
     rockchip,boot-ecc-strength = <24>;

     // block 14 - end
     linux@1C00000 {
         label = "linux";
         reg = <0x0 0x1C00000 0x3 0xFE400000>;
     };
};

When i create the UBI volume the middle of this partition where the 
blocks are at 2nd chip block0-7, will be written in boot block strength 
[ECC:1024/24] but will be read with normal strength [ECC:1024/40].

This causes first UBI creation to be successful, and next scan to fail 
due to written strength is different that read.

You dont actually need to use UBI at all, any time you write those 
sectors, you wont be able to read them. A workaround it to mark them 
bad, but this is not nice, since the blocks are completely fine.

Rockchip NFC technically as minimum needs to know rockchip,boot-blks, & 
rockchip,boot-ecc-strength props only. Currently the checking condition 
is rockchip,boot-blks & applied affect is rockchip,boot-ecc-strength.

The bug is this is only applicable to first chip not all chips.

NAND_IS_BOOT_MEDIUM is actually coming from mainline linux. Similar 
bootrom tricks are also available in other socs. And detection of boot 
rom blocks is not always straight forward as rockchip's 
rockchip,boot-blks, they have to do some "if" case acrobatics to detect 
those. For simplicity reasons mainline linux introduced a global flag 
NAND_IS_BOOT_MEDIUM and it is applied to other socs as well.

Since this "&& nfc->selected_bank == 0" fix is exactly at the same line 
with mainline changes of NAND_IS_BOOT_MEDIUM check, i think Johan also 
integrated both at the same line.

In u-boot only mk808 is using nfc with boot blocks and it is already 
marking the nand device as boot medium, so the code change should not 
break existing devices.

Additional note: Linux mainline also is lacking the "nfc->selected_bank 
== 0" check, so i think similar patch should got to linux as well.

@johan if anything i am missing feel free to correct me.

hüseyin

On 7/20/26 20:25, Quentin Schulz via U-Boot wrote:
> Hi Johan,
> 
> Resending because the ML rejected my mail sent from my other address...
> I have to figure out what I set up wrong to trigger the spam filter :)
> 
> On 7/14/26 8:39 AM, Johan Jonker wrote:
>   > The Rockchip boot ROM only checks for NAND chip 0 and with
>   > reduced ECC strength. Currently only the read page functions
>   > have this condition check added.
>   >
>   > Fix by adding the same condition to all read and write page
>   > functions by dropping the existing 'selected_bank == 0' check
>   > and use the NAND_IS_BOOT_MEDIUM option that was introduced to
>   > U-Boot more recently than this driver to behave
>   > identically to the Linux driver.
>   >
>   > It is now the users responsibility to apply the device tree
>   > property "nand-is-boot-medium" to only NAND chip 0.
>   >
>   > Fixes: b12dc5d6fa76 ("mtd: nand: NFC drivers for RK3308, RK2928 and
> others")
>   > Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>   > Tested-by: Hüseyin BIYIK <boogiepop@gmx.com>
>   > Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> You don't explain how the bug can be triggered. It'd be nice to provide
> the usecase when this is an issue so that other people looking on the
> Internet for bug reports could somehow stumble upon this patch.
> 
> I'm thinking the issue is that we currently verify all NAND chips use
> the boot_blks and boot_ecc from the boot medium whereas they might not
> be used as a boot medium (they are missing the nand-is-boot-medium
> property) so we cannot actually make use of them. Is that correct?
> 
> Considering boot_blks is 0 if rockchip,boot-blks property isn't set,
> we'll never be able to meet the page < pages_per_blk * 0) condition
> anyway so we would never enter the if block... or can page actually be
> negative????
> 
> To be clear, I don't disagree with the fix, I just am missing a lot of
> information that should be in the commit log.
> 
> Cheers,
> Quentin

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Johan Jonker via U-Boot]

Changed References:
Added Linux MTD maintainers. Question below.

Hi,

On 7/22/26 00:56, Boogie wrote:
> Hello Quentin
> 
> I had reported this bug to Johan so i can give detailed explanation.
> 
> The bug was really the lock of "&& nfc->selected_bank == 0" not existing for write_page_* variants.
> 
> When rockchip nfc was reading, it required bootblks to be a part of the first nand chip only with the condition "nfc->selected_bank == 0". But when writing it interpreted 'all' nand chips first boot_blks as boot blocks.
> 
> The practical problem. I had mentioned this in V1 and give more explicit details here.
> 
> I have 2 nand chips, and a partition (linux below) starts in nand chip 1 (CS=0 in below) and ends in chip2 (CS=2 below). And i am using UBI on top of that mtd.
> 
> nand@0 {
>     reg = <0>, <2>;
>     label = "rk-nand-0";
>     nand-bus-width = <8>;
>     nand-ecc-mode = "hw";
>     nand-ecc-step-size = <1024>;
>     nand-ecc-strength = <40>;

>     nand-is-boot-medium;

This property was introduced during review, but can't find the reason.
https://lore.kernel.org/linux-rockchip/20200426100250.14678-1-yifeng.zhao@rock-chips.com/

>     rockchip,boot-blks = <8>;
>     rockchip,boot-ecc-strength = <24>;

Describing 2 nands with 1 node also exposes properties to a second nand that result in reduced ecc strength in both.  

> 

>     // block 14 - end
>     linux@1C00000 {

The binding puts the partitions under 1 nand node, but says nothing about a partition across 2 or more nands.
Could the MTD maintainers inform us the support status of this 'feature' in Linux and U-Boot.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/mtd/mtd.yaml#n39

>         label = "linux";
>         reg = <0x0 0x1C00000 0x3 0xFE400000>;
>     };
> };

I would like to produce a patch with "&& nfc->selected_bank == 0" for Linux.
Not sure if they are willing to merge.

Depending on the output/feedback put this patch on hold till I know more.

Johan

> 
> When i create the UBI volume the middle of this partition where the blocks are at 2nd chip block0-7, will be written in boot block strength [ECC:1024/24] but will be read with normal strength [ECC:1024/40].
> 
> This causes first UBI creation to be successful, and next scan to fail due to written strength is different that read.
> 
> You dont actually need to use UBI at all, any time you write those sectors, you wont be able to read them. A workaround it to mark them bad, but this is not nice, since the blocks are completely fine.
> 
> Rockchip NFC technically as minimum needs to know rockchip,boot-blks, & rockchip,boot-ecc-strength props only. Currently the checking condition is rockchip,boot-blks & applied affect is rockchip,boot-ecc-strength.
> 
> The bug is this is only applicable to first chip not all chips.
> 
> NAND_IS_BOOT_MEDIUM is actually coming from mainline linux. Similar bootrom tricks are also available in other socs. And detection of boot rom blocks is not always straight forward as rockchip's rockchip,boot-blks, they have to do some "if" case acrobatics to detect those. For simplicity reasons mainline linux introduced a global flag NAND_IS_BOOT_MEDIUM and it is applied to other socs as well.
> 
> Since this "&& nfc->selected_bank == 0" fix is exactly at the same line with mainline changes of NAND_IS_BOOT_MEDIUM check, i think Johan also integrated both at the same line.
> 
> In u-boot only mk808 is using nfc with boot blocks and it is already marking the nand device as boot medium, so the code change should not break existing devices.
> 
> Additional note: Linux mainline also is lacking the "nfc->selected_bank == 0" check, so i think similar patch should got to linux as well.
> 
> @johan if anything i am missing feel free to correct me.
> 
> hüseyin
> 
> On 7/20/26 20:25, Quentin Schulz via U-Boot wrote:
>> Hi Johan,
>>
>> Resending because the ML rejected my mail sent from my other address...
>> I have to figure out what I set up wrong to trigger the spam filter :)
>>
>> On 7/14/26 8:39 AM, Johan Jonker wrote:
>>   > The Rockchip boot ROM only checks for NAND chip 0 and with
>>   > reduced ECC strength. Currently only the read page functions
>>   > have this condition check added.
>>   >
>>   > Fix by adding the same condition to all read and write page
>>   > functions by dropping the existing 'selected_bank == 0' check
>>   > and use the NAND_IS_BOOT_MEDIUM option that was introduced to
>>   > U-Boot more recently than this driver to behave
>>   > identically to the Linux driver.
>>   >
>>   > It is now the users responsibility to apply the device tree
>>   > property "nand-is-boot-medium" to only NAND chip 0.
>>   >
>>   > Fixes: b12dc5d6fa76 ("mtd: nand: NFC drivers for RK3308, RK2928 and
>> others")
>>   > Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>>   > Tested-by: Hüseyin BIYIK <boogiepop@gmx.com>
>>   > Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> You don't explain how the bug can be triggered. It'd be nice to provide
>> the usecase when this is an issue so that other people looking on the
>> Internet for bug reports could somehow stumble upon this patch.
>>
>> I'm thinking the issue is that we currently verify all NAND chips use
>> the boot_blks and boot_ecc from the boot medium whereas they might not
>> be used as a boot medium (they are missing the nand-is-boot-medium
>> property) so we cannot actually make use of them. Is that correct?
>>
>> Considering boot_blks is 0 if rockchip,boot-blks property isn't set,
>> we'll never be able to meet the page < pages_per_blk * 0) condition
>> anyway so we would never enter the if block... or can page actually be
>> negative????
>>
>> To be clear, I don't disagree with the fix, I just am missing a lot of
>> information that should be in the commit log.
>>
>> Cheers,
>> Quentin
> 

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Quentin Schulz via U-Boot]

Hi Hüseyin, Johan,

On 7/22/26 12:56 AM, Boogie wrote:
> [You don't often get email from boogiepop@gmx.com. Learn why this is 
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hello Quentin
> 
> I had reported this bug to Johan so i can give detailed explanation.
> 
> The bug was really the lock of "&& nfc->selected_bank == 0" not existing
> for write_page_* variants.
> 
> When rockchip nfc was reading, it required bootblks to be a part of the
> first nand chip only with the condition "nfc->selected_bank == 0". But
> when writing it interpreted 'all' nand chips first boot_blks as boot 
> blocks.
> 

OK ECC strength mismatch between what's written and read.

> The practical problem. I had mentioned this in V1 and give more explicit
> details here.
> 
> I have 2 nand chips, and a partition (linux below) starts in nand chip 1
> (CS=0 in below) and ends in chip2 (CS=2 below). And i am using UBI on
> top of that mtd.
> 
> nand@0 {
>      reg = <0>, <2>;
>      label = "rk-nand-0";
>      nand-bus-width = <8>;
>      nand-ecc-mode = "hw";
>      nand-ecc-step-size = <1024>;
>      nand-ecc-strength = <40>;
>      nand-is-boot-medium;
>      rockchip,boot-blks = <8>;
>      rockchip,boot-ecc-strength = <24>;
> 
>      // block 14 - end
>      linux@1C00000 {
>          label = "linux";
>          reg = <0x0 0x1C00000 0x3 0xFE400000>;
>      };
> };
> 
> When i create the UBI volume the middle of this partition where the
> blocks are at 2nd chip block0-7, will be written in boot block strength
> [ECC:1024/24] but will be read with normal strength [ECC:1024/40].
> 
> This causes first UBI creation to be successful, and next scan to fail
> due to written strength is different that read.
> 
> You dont actually need to use UBI at all, any time you write those
> sectors, you wont be able to read them. A workaround it to mark them
> bad, but this is not nice, since the blocks are completely fine.
> 
> Rockchip NFC technically as minimum needs to know rockchip,boot-blks, &
> rockchip,boot-ecc-strength props only. Currently the checking condition
> is rockchip,boot-blks & applied affect is rockchip,boot-ecc-strength.
> 
> The bug is this is only applicable to first chip not all chips.
> 

"is only applied" I think you meant here instead? Or are you implying 
that you want chip 0 to have rockchip,boot-blks and 
rockchip,boot-ecc-strength applied to it, but NOT chip 2 even though 
they are listed in the same node?

> NAND_IS_BOOT_MEDIUM is actually coming from mainline linux. Similar
> bootrom tricks are also available in other socs. And detection of boot
> rom blocks is not always straight forward as rockchip's
> rockchip,boot-blks, they have to do some "if" case acrobatics to detect
> those. For simplicity reasons mainline linux introduced a global flag
> NAND_IS_BOOT_MEDIUM and it is applied to other socs as well.
> 

rockchip,boot-blks and rockchip,boot-ecc-strength properties explicitly 
require nand-is-boot-medium property to be set in order to be used 
(though they don't enforce it via YAML/JSON schema; although it is 
enforced by the kernel driver). So a device tree that doesn't follow 
those restrictions is deemed invalid and we are not aiming at supporting 
them. This means that the last sentence in Johan's commit log:

It is now the users responsibility to apply the device tree
property "nand-is-boot-medium" to only NAND chip 0.

is redundant, as we already expect the users to set it for the 
appropriate NAND chip(s). (just adding this piece of information here, 
not disputing anything either of you said)

> Since this "&& nfc->selected_bank == 0" fix is exactly at the same line
> with mainline changes of NAND_IS_BOOT_MEDIUM check, i think Johan also
> integrated both at the same line.
> 

@Johan, can you please reorder the condition in rk_nfc_write_page_hwecc 
and rk_nfc_read_page_hwecc to match the order in the Linux kernel 
driver? (the NAND_IS_BOOT_MEDIUM is last).

> In u-boot only mk808 is using nfc with boot blocks and it is already
> marking the nand device as boot medium, so the code change should not
> break existing devices.
> 

Correct.

> Additional note: Linux mainline also is lacking the "nfc->selected_bank
> == 0" check, so i think similar patch should got to linux as well.
> 

Not sure what you mean here as Johan removed both checks in the patch.

[...]

As far as I could tell, Johan's patch makes U-Boot behave the same as 
the Linux kernel for a multi-chip node, so with the reorder I asked 
above and a much more precise commit log providing the usecase and 
misbehavior, and also pointing at the Linux kernel doing the exact same 
thing as this patch, I think we're going in the right direction.

Cheers,
Quentin

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Boogie via U-Boot]

Hi Quentin

On 7/22/26 11:28, Quentin Schulz wrote:
> Hi Hüseyin, Johan,
> 
> On 7/22/26 12:56 AM, Boogie wrote:
>> [You don't often get email from boogiepop@gmx.com. Learn why this is 
>> important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Hello Quentin
>>
>> I had reported this bug to Johan so i can give detailed explanation.
>>
>> The bug was really the lock of "&& nfc->selected_bank == 0" not existing
>> for write_page_* variants.
>>
>> When rockchip nfc was reading, it required bootblks to be a part of the
>> first nand chip only with the condition "nfc->selected_bank == 0". But
>> when writing it interpreted 'all' nand chips first boot_blks as boot 
>> blocks.
>>
> 
> OK ECC strength mismatch between what's written and read.
> 
>> The practical problem. I had mentioned this in V1 and give more explicit
>> details here.
>>
>> I have 2 nand chips, and a partition (linux below) starts in nand chip 1
>> (CS=0 in below) and ends in chip2 (CS=2 below). And i am using UBI on
>> top of that mtd.
>>
>> nand@0 {
>>      reg = <0>, <2>;
>>      label = "rk-nand-0";
>>      nand-bus-width = <8>;
>>      nand-ecc-mode = "hw";
>>      nand-ecc-step-size = <1024>;
>>      nand-ecc-strength = <40>;
>>      nand-is-boot-medium;
>>      rockchip,boot-blks = <8>;
>>      rockchip,boot-ecc-strength = <24>;
>>
>>      // block 14 - end
>>      linux@1C00000 {
>>          label = "linux";
>>          reg = <0x0 0x1C00000 0x3 0xFE400000>;
>>      };
>> };
>>
>> When i create the UBI volume the middle of this partition where the
>> blocks are at 2nd chip block0-7, will be written in boot block strength
>> [ECC:1024/24] but will be read with normal strength [ECC:1024/40].
>>
>> This causes first UBI creation to be successful, and next scan to fail
>> due to written strength is different that read.
>>
>> You dont actually need to use UBI at all, any time you write those
>> sectors, you wont be able to read them. A workaround it to mark them
>> bad, but this is not nice, since the blocks are completely fine.
>>
>> Rockchip NFC technically as minimum needs to know rockchip,boot-blks, &
>> rockchip,boot-ecc-strength props only. Currently the checking condition
>> is rockchip,boot-blks & applied affect is rockchip,boot-ecc-strength.
>>
>> The bug is this is only applicable to first chip not all chips.
>>
> 
> "is only applied" I think you meant here instead? Or are you implying 
> that you want chip 0 to have rockchip,boot-blks and rockchip,boot-ecc- 
> strength applied to it, but NOT chip 2 even though they are listed in 
> the same node?
> 

Yes boot-blks and rockchip,boot-ecc-strength must be applied to only to 
the chip CS=0. This is how the bootrom works, bootrom does not look for 
an idb image [tpl/spl] in a chip which is not CS == 0.

>> NAND_IS_BOOT_MEDIUM is actually coming from mainline linux. Similar
>> bootrom tricks are also available in other socs. And detection of boot
>> rom blocks is not always straight forward as rockchip's
>> rockchip,boot-blks, they have to do some "if" case acrobatics to detect
>> those. For simplicity reasons mainline linux introduced a global flag
>> NAND_IS_BOOT_MEDIUM and it is applied to other socs as well.
>>
> 
> rockchip,boot-blks and rockchip,boot-ecc-strength properties explicitly 
> require nand-is-boot-medium property to be set in order to be used 
> (though they don't enforce it via YAML/JSON schema; although it is 
> enforced by the kernel driver). So a device tree that doesn't follow 
> those restrictions is deemed invalid and we are not aiming at supporting 
> them. This means that the last sentence in Johan's commit log:
> 
> It is now the users responsibility to apply the device tree
> property "nand-is-boot-medium" to only NAND chip 0.
> 
> is redundant, as we already expect the users to set it for the 
> appropriate NAND chip(s). (just adding this piece of information here, 
> not disputing anything either of you said)
> 
>> Since this "&& nfc->selected_bank == 0" fix is exactly at the same line
>> with mainline changes of NAND_IS_BOOT_MEDIUM check, i think Johan also
>> integrated both at the same line.
>>
> 
> @Johan, can you please reorder the condition in rk_nfc_write_page_hwecc 
> and rk_nfc_read_page_hwecc to match the order in the Linux kernel 
> driver? (the NAND_IS_BOOT_MEDIUM is last).
> 
>> In u-boot only mk808 is using nfc with boot blocks and it is already
>> marking the nand device as boot medium, so the code change should not
>> break existing devices.
>>
> 
> Correct.
> 
>> Additional note: Linux mainline also is lacking the "nfc->selected_bank
>> == 0" check, so i think similar patch should got to linux as well.
>>
> 
> Not sure what you mean here as Johan removed both checks in the patch.

You are right, sorry for misleading, we had discussed to put a lock to 
with "&& nfc->selected_bank == 0" for both read and write to uniform. 
But now i noticed that this patch only syncs with mainline and 
downgrades the ECC to bootrom strength for all chips first boot blocks. 
This also works, but of course the side affect is reduced ecc strength 
(if your current one is higher, and most likely is).

> 
> [...]
> 
> As far as I could tell, Johan's patch makes U-Boot behave the same as 
> the Linux kernel for a multi-chip node, so with the reorder I asked 
> above and a much more precise commit log providing the usecase and 
> misbehavior, and also pointing at the Linux kernel doing the exact same 
> thing as this patch, I think we're going in the right direction.
> 
> Cheers,
> Quentin

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Boogie via U-Boot]

Hi Johan

On 7/22/26 11:16, Johan Jonker wrote:
> Changed References:
> Added Linux MTD maintainers. Question below.
> 
> Hi,
> 
> On 7/22/26 00:56, Boogie wrote:
>> Hello Quentin
>>
>> I had reported this bug to Johan so i can give detailed explanation.
>>
>> The bug was really the lock of "&& nfc->selected_bank == 0" not existing for write_page_* variants.
>>
>> When rockchip nfc was reading, it required bootblks to be a part of the first nand chip only with the condition "nfc->selected_bank == 0". But when writing it interpreted 'all' nand chips first boot_blks as boot blocks.
>>
>> The practical problem. I had mentioned this in V1 and give more explicit details here.
>>
>> I have 2 nand chips, and a partition (linux below) starts in nand chip 1 (CS=0 in below) and ends in chip2 (CS=2 below). And i am using UBI on top of that mtd.
>>
>> nand@0 {
>>      reg = <0>, <2>;
>>      label = "rk-nand-0";
>>      nand-bus-width = <8>;
>>      nand-ecc-mode = "hw";
>>      nand-ecc-step-size = <1024>;
>>      nand-ecc-strength = <40>;
> 
>>      nand-is-boot-medium;
> 
> This property was introduced during review, but can't find the reason.
> https://lore.kernel.org/linux-rockchip/20200426100250.14678-1-yifeng.zhao@rock-chips.com/
> 
>>      rockchip,boot-blks = <8>;
>>      rockchip,boot-ecc-strength = <24>;
> 
> Describing 2 nands with 1 node also exposes properties to a second nand that result in reduced ecc strength in both.
> 
>>
> 
>>      // block 14 - end
>>      linux@1C00000 {
> 
> The binding puts the partitions under 1 nand node, but says nothing about a partition across 2 or more nands.
> Could the MTD maintainers inform us the support status of this 'feature' in Linux and U-Boot.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/mtd/mtd.yaml#n39
> 
>>          label = "linux";
>>          reg = <0x0 0x1C00000 0x3 0xFE400000>;
>>      };
>> };
> 
> I would like to produce a patch with "&& nfc->selected_bank == 0" for Linux.
> Not sure if they are willing to merge.
> 
> Depending on the output/feedback put this patch on hold till I know more.

This current version in mainline linux (and your patch) also works, may 
be it is better to leave it so (at least for me), because adding "&& 
nfc->selected_bank == 0" will break existing devices, which is not 
always worth it specially for legacy devices like ones using this driver.

> 
> Johan
> 
>>
>> When i create the UBI volume the middle of this partition where the blocks are at 2nd chip block0-7, will be written in boot block strength [ECC:1024/24] but will be read with normal strength [ECC:1024/40].
>>
>> This causes first UBI creation to be successful, and next scan to fail due to written strength is different that read.
>>
>> You dont actually need to use UBI at all, any time you write those sectors, you wont be able to read them. A workaround it to mark them bad, but this is not nice, since the blocks are completely fine.
>>
>> Rockchip NFC technically as minimum needs to know rockchip,boot-blks, & rockchip,boot-ecc-strength props only. Currently the checking condition is rockchip,boot-blks & applied affect is rockchip,boot-ecc-strength.
>>
>> The bug is this is only applicable to first chip not all chips.
>>
>> NAND_IS_BOOT_MEDIUM is actually coming from mainline linux. Similar bootrom tricks are also available in other socs. And detection of boot rom blocks is not always straight forward as rockchip's rockchip,boot-blks, they have to do some "if" case acrobatics to detect those. For simplicity reasons mainline linux introduced a global flag NAND_IS_BOOT_MEDIUM and it is applied to other socs as well.
>>
>> Since this "&& nfc->selected_bank == 0" fix is exactly at the same line with mainline changes of NAND_IS_BOOT_MEDIUM check, i think Johan also integrated both at the same line.
>>
>> In u-boot only mk808 is using nfc with boot blocks and it is already marking the nand device as boot medium, so the code change should not break existing devices.
>>
>> Additional note: Linux mainline also is lacking the "nfc->selected_bank == 0" check, so i think similar patch should got to linux as well.
>>
>> @johan if anything i am missing feel free to correct me.
>>
>> hüseyin
>>
>> On 7/20/26 20:25, Quentin Schulz via U-Boot wrote:
>>> Hi Johan,
>>>
>>> Resending because the ML rejected my mail sent from my other address...
>>> I have to figure out what I set up wrong to trigger the spam filter :)
>>>
>>> On 7/14/26 8:39 AM, Johan Jonker wrote:
>>>    > The Rockchip boot ROM only checks for NAND chip 0 and with
>>>    > reduced ECC strength. Currently only the read page functions
>>>    > have this condition check added.
>>>    >
>>>    > Fix by adding the same condition to all read and write page
>>>    > functions by dropping the existing 'selected_bank == 0' check
>>>    > and use the NAND_IS_BOOT_MEDIUM option that was introduced to
>>>    > U-Boot more recently than this driver to behave
>>>    > identically to the Linux driver.
>>>    >
>>>    > It is now the users responsibility to apply the device tree
>>>    > property "nand-is-boot-medium" to only NAND chip 0.
>>>    >
>>>    > Fixes: b12dc5d6fa76 ("mtd: nand: NFC drivers for RK3308, RK2928 and
>>> others")
>>>    > Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>>>    > Tested-by: Hüseyin BIYIK <boogiepop@gmx.com>
>>>    > Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>> You don't explain how the bug can be triggered. It'd be nice to provide
>>> the usecase when this is an issue so that other people looking on the
>>> Internet for bug reports could somehow stumble upon this patch.
>>>
>>> I'm thinking the issue is that we currently verify all NAND chips use
>>> the boot_blks and boot_ecc from the boot medium whereas they might not
>>> be used as a boot medium (they are missing the nand-is-boot-medium
>>> property) so we cannot actually make use of them. Is that correct?
>>>
>>> Considering boot_blks is 0 if rockchip,boot-blks property isn't set,
>>> we'll never be able to meet the page < pages_per_blk * 0) condition
>>> anyway so we would never enter the if block... or can page actually be
>>> negative????
>>>
>>> To be clear, I don't disagree with the fix, I just am missing a lot of
>>> information that should be in the commit log.
>>>
>>> Cheers,
>>> Quentin
>>
> 

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Quentin Schulz via U-Boot]

On 7/22/26 1:39 PM, Boogie wrote:
> [You don't often get email from boogiepop@gmx.com. Learn why this is 
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hi Quentin
> 
> On 7/22/26 11:28, Quentin Schulz wrote:
>> Hi Hüseyin, Johan,
>>
>> On 7/22/26 12:56 AM, Boogie wrote:
[...]
>>> Rockchip NFC technically as minimum needs to know rockchip,boot-blks, &
>>> rockchip,boot-ecc-strength props only. Currently the checking condition
>>> is rockchip,boot-blks & applied affect is rockchip,boot-ecc-strength.
>>>
>>> The bug is this is only applicable to first chip not all chips.
>>>
>>
>> "is only applied" I think you meant here instead? Or are you implying
>> that you want chip 0 to have rockchip,boot-blks and rockchip,boot-ecc-
>> strength applied to it, but NOT chip 2 even though they are listed in
>> the same node?
>>
> 
> Yes boot-blks and rockchip,boot-ecc-strength must be applied to only to
> the chip CS=0. This is how the bootrom works, bootrom does not look for
> an idb image [tpl/spl] in a chip which is not CS == 0.
> 

The BootROM can only read from chip 0, and with a specific (lower I 
guess) ECC strength. That I understood and is a limitation we cannot 
work-around.

I was trying to figure out if you wanted to have chip 2 have a different 
ECC strength than chip 0 while being in the same node (and thus, 
necessarily with the same properties). Considering the properties are in 
a node with multiple chips (via the reg property), I would think the 
properties should apply to *all* chips in that node. Considering the 
kernel doesn't support this (from a cursory glance, maybe I misread the 
code), if you want something different, you need to discuss this with 
the Linux kernel MTD and Device Tree maintainers to have this validated 
first. I think they won't agree simply because we cannot know for sure 
that all (past and future) SoCs will forever only read from chip 0 and 
would result in different behaviors with the exact same node for 
different SoCs, but maybe I'm wrong :)

[...]

>>> Additional note: Linux mainline also is lacking the "nfc->selected_bank
>>> == 0" check, so i think similar patch should got to linux as well.
>>>
>>
>> Not sure what you mean here as Johan removed both checks in the patch.
> 
> You are right, sorry for misleading, we had discussed to put a lock to
> with "&& nfc->selected_bank == 0" for both read and write to uniform.
> But now i noticed that this patch only syncs with mainline and
> downgrades the ECC to bootrom strength for all chips first boot blocks.
> This also works, but of course the side affect is reduced ecc strength
> (if your current one is higher, and most likely is).
> 

Indeed. Thanks for the confirmation!

Cheers,
Quentin

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Miquel Raynal via U-Boot]

On 22/07/2026 at 11:16:20 +02, Johan Jonker <jbx6244@gmail.com> wrote:

> Changed References:
> Added Linux MTD maintainers. Question below.
>
> Hi,
>
> On 7/22/26 00:56, Boogie wrote:
>> Hello Quentin
>> 
>> I had reported this bug to Johan so i can give detailed explanation.
>> 
>> The bug was really the lock of "&& nfc->selected_bank == 0" not existing for write_page_* variants.
>> 
>> When rockchip nfc was reading, it required bootblks to be a part of
>> the first nand chip only with the condition "nfc->selected_bank ==
>> 0". But when writing it interpreted 'all' nand chips first boot_blks
>> as boot blocks.
>> 
>> The practical problem. I had mentioned this in V1 and give more explicit details here.
>> 
>> I have 2 nand chips, and a partition (linux below) starts in nand
>> chip 1 (CS=0 in below) and ends in chip2 (CS=2 below). And i am using
>> UBI on top of that mtd.
>> nand@0 {
>>     reg = <0>, <2>;
>>     label = "rk-nand-0";
>>     nand-bus-width = <8>;
>>     nand-ecc-mode = "hw";
>>     nand-ecc-step-size = <1024>;
>>     nand-ecc-strength = <40>;
>
>>     nand-is-boot-medium;

What you may want is to declare the NAND using two different nodes, then
use mtd-virt-concat to generate one bigger MTD device spanning across
the two devices.

The driver should not blindly check the 'cs ==0' condition but instead
also take nand-is-boot-medium into account (which would then only be
required in the NAND chip with the said boot partition) to make sure the
special handling is only applied on the boot partition.

Hope that helps.

Thanks,
Miquèl

Re: [PATCH v2] mtd: nand: raw: rockchip_nfc: fix ecc setup

[Boogie]

Hello Miquel

On 7/24/26 11:26, Miquel Raynal wrote:
> On 22/07/2026 at 11:16:20 +02, Johan Jonker <jbx6244@gmail.com> wrote:
> 
>> Changed References:
>> Added Linux MTD maintainers. Question below.
>>
>> Hi,
>>
>> On 7/22/26 00:56, Boogie wrote:
>>> Hello Quentin
>>>
>>> I had reported this bug to Johan so i can give detailed explanation.
>>>
>>> The bug was really the lock of "&& nfc->selected_bank == 0" not existing for write_page_* variants.
>>>
>>> When rockchip nfc was reading, it required bootblks to be a part of
>>> the first nand chip only with the condition "nfc->selected_bank ==
>>> 0". But when writing it interpreted 'all' nand chips first boot_blks
>>> as boot blocks.
>>>
>>> The practical problem. I had mentioned this in V1 and give more explicit details here.
>>>
>>> I have 2 nand chips, and a partition (linux below) starts in nand
>>> chip 1 (CS=0 in below) and ends in chip2 (CS=2 below). And i am using
>>> UBI on top of that mtd.
>>> nand@0 {
>>>      reg = <0>, <2>;
>>>      label = "rk-nand-0";
>>>      nand-bus-width = <8>;
>>>      nand-ecc-mode = "hw";
>>>      nand-ecc-step-size = <1024>;
>>>      nand-ecc-strength = <40>;
>>
>>>      nand-is-boot-medium;
> 
> What you may want is to declare the NAND using two different nodes, then
> use mtd-virt-concat to generate one bigger MTD device spanning across
> the two devices.

Thats also an option but i prefer this way to be more straight forward 
since entire partition layout can be kept in dts.

> 
> The driver should not blindly check the 'cs ==0' condition but instead

I think my statement was also not accurate, i think the brom at least 
for 3066 should check only the first chip which has the least cs so 
nfc->selected_bank == 0. This could have been validated easily by 
erasing boot blocks in both chips and writing an idb to 2nd chip 
deliberately and check if the 3066 would boot or not. But i think i 
burned gpios in my board while messing with the shorting nand chips, 2nd 
chip is RIP. so i cant test this anymore unfortunately. In any case 
current working way of rockchip_nfc in mainline linux is also fine. If 
somehow those low eccs create problem, they can be marked as bad anyways.

But thanks for your input.

> also take nand-is-boot-medium into account (which would then only be
> required in the NAND chip with the said boot partition) to make sure the
> special handling is only applied on the boot partition.
> 
> Hope that helps.
> 
> Thanks,
> Miquèl