Hi Hüseyin, Johan,
On 7/22/26 12:56 AM, Boogie wrote:
[You don't often get email from [email protected]. 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
