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 <[email protected]>
> Tested-by: Hüseyin BIYIK <[email protected]>
> Reviewed-by: Simon Glass <[email protected]>
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
