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

[Johan Jonker via U-Boot <u-boot@lists.u-boot-project.org>]

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
>