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/[email protected]/
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 <[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
