Hi Johan,
On 7/22/26 11:45 PM, Johan Jonker via U-Boot wrote:
A Rockchip NAND Flash Controller (NFC) supports multiple
flash devices. The SoC boot ROM only checks for NAND chip 0
and with reduced ECC strength. Currently only the read page
functions have this condition check added in the U-Boot
driver.
The rk_nfc_write_page_hwecc function together with a wrongly
placed rockchip,boot-blks property writes these blocks at
any NAND with reduced strength. The rk_nfc_read_page_hwecc
function however does this only for chip 0. Resulting in a
a ECC strength mismatch between what's written and read for
U-Boot applications.
rockchip,boot-blks and rockchip,boot-ecc-strength properties
require the nand-is-boot-medium property to be set in the Linux
driver. When the nand-is-boot-medium property is omitted or accidently
placed in an other NAND node with the required properties than for
chip 0 there is a ECC strength mismatch between what's written
and read between Linux and U-Boot.
Yeah, we don't care, this is simply an invalid device tree. We could
however implement the same logic in U-Boot by simply adding an
if (chip->options & NAND_IS_BOOT_MEDIUM)
around the two ofnode_read_u32 in U-Boot. A follow-up patch would be
nice but not required.
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.
I don't like the wording in the commit log, it still isn't clear to me
what the issues are from reading it. My understanding is that there are
two cases that aren't working in U-Boot currently:
1. multi-chip node with rockchip,boot-blks and
rockchip,boot-ecc-strength. They will be applied correctly for chip 0,
but for other chips in the node, the read will use a different ECC than
the write.
2. single-chip node with rockchip,boot-blks and
rockchip,boot-ecc-strength for a chip that is NOT 0 (not sure this case
makes sense though as I'm not sure there's an SoC BootROM that can read
from anything but chip 0). Then we'll respect the properties when
writing but not when reading and thus have an ECC mismatch.
If you don't set the properties, then boot_blks will be 0 and page <
pages_per_blk * 0) will always be false and the four conditions blocks
changed in this patch will anyway not be entered regardless of the chip
so there cannot be a mismatch.
Is the above correct? Are there more cases that this patch here fixes
and that I missed?
I can suggest to reword the commit log to:
"""
A Rockchip NAND Flash Controller (NFC) supports multiple
flash devices. The SoC boot ROM only checks for NAND chip 0
and with reduced ECC strength. Currently only the read page
functions have this condition check added in the U-Boot
driver.
In single- and multi-chip nodes with rockchip,boot-blks and
rockchip,boot-ecc-strength set, the chips that aren't chip 0 will thus
have an ECC mismatch when writing and reading.
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.
NAND_IS_BOOT_MEDIUM is automatically set when nand-is-boot-medium
property is set in the Device Tree, which is a requirement for
rockchip,boot-blks and rockchip,boot-ecc-strength and will thus apply to
all chips in single- and multi-chips nodes, regardless of the CS.
"""
What do you think? If you agree and this covers all possible issues
fixed by this patch, can you send a v4 for this?
Giving my:
Reviewed-by: Quentin Schulz <[email protected]>
as the diff is correct.
Thanks!
Quentin
