On 12/1/22 12:03, Francesco Dolcini wrote:
+ MTD maintainers/list
On Wed, Nov 30, 2022 at 11:59:04PM +0100, Marek Vasut wrote:
On 11/30/22 21:51, Francesco Dolcini wrote:
On Wed, Nov 30, 2022 at 03:41:13PM +0100, Marek Vasut wrote:
On 11/30/22 14:52, Francesco Dolcini wrote:
[ 0.000000] Booting Linux on physical CPU 0x0
[ 0.000000] Linux version 6.0.10 (francesco@francesco-nb) (arm-linux-gnueabihf-gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.
4.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #36 SMP Wed Nov 30 14:07:15 CET 2022
...
[ 4.407499] gpmi-nand: error parsing ofpart partition /soc/nand-controller@33002000/partition@0 (/soc/nand-controller
@33002000)
[ 4.438401] gpmi-nand 33002000.nand-controller: driver registered.
...
[ 5.933906] VFS: Cannot open root device "ubi0:rootfs" or unknown-block(0,0): error -19
[ 5.946504] Please append a correct "root=" boot option; here are the available partitions:
...
Any idea? I'm not familiar with the gpmi-nand driver and I would just revert it, but
maybe you have a better idea.
Can you share the relevant snippet of your nand controller DT node ?
We just have
from imx7-colibri.dtsi,
&gpmi {
fsl,use-minimum-ecc;
nand-ecc-mode = "hw";
nand-on-flash-bbt;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_gpmi_nand>;
};
OF partition are created by U-Boot from
mtdparts=mtdparts=gpmi-nand:512k(mx7-bcb),1536k(u-boot1)ro,1536k(u-boot2)ro,512k(u-boot-env),-(ubi)
env variables calling fdt_fixup_mtdparts from colibri_imx7.c
Everything is available in the upstream Linux/U-Boot git, no downstream
repo of any sort.
Probably up to first partition is enough. I suspect you need to fill in the
correct address-cells/size-cells there, which might be currently missing in
your DT and worked by chance.
This is generated by U-Boot, I would need to dump what he did generate
from the standard fdt_fixup_mtdparts(). I will try to do it tomorrow
unless what I wrote here is already enough to understand what's going
on.
Oh drat ... I see. It's the u-boot fdt_node_set_part_info() which checks the
current NAND controller #size-cells and uses that when generating MTD
partitions 'reg' properties. Since #size-cells is now zero, the reg
properties would be malformed.
I think the issue is slightly different, the u-boot code checks it and
if not set it defaults to #size-cells = <1>. Said that u-boot
never set #size-cells anywhere.
Which it really should, can you send a patch there too ?
What is failing is ofpart_core.c:parse_fixed_partitions() in Linux with
#size-cells = <0>.
Now, what I am unsure is whether the right fix is to update mx7 colibri DT
and include &gpmi { #size-cells=<1>; }; , or , revert this patch. The former
fixes the problem for colibri and retains the correct #size-cells=<0>
behavior for any other board which does not specify MTD partitions in the
GPMI NAND node. The later also covers boards which we don't know about which
might also use generated MTD partitions in DT using fdt_fixup_mtdparts() in
U-Boot, but I am not convinced that is correct.
So, would you be OK with fixing up the colibri mx7 DT with #size-cells=<1> ?
I am also not sure what is the right fix, however I am convinced that
the fix needs to be in Linux, we cannot really break the boot flow.
I agree
In a very pragmatic way I could just add the property to colibri-imx7
dtsi, but we are really breaking potential other users of it, anybody
using U-Boot to generate the partitions in the end ... (and the list is
not empty and not just the colibri*).
I agree here too
Would it make any sense to do something like that (untested!) ?
diff --git a/drivers/mtd/parsers/ofpart_core.c b/drivers/mtd/parsers/ofpart_core.c
index 192190c42fc8..fffd60acd926 100644
--- a/drivers/mtd/parsers/ofpart_core.c
+++ b/drivers/mtd/parsers/ofpart_core.c
@@ -122,6 +122,8 @@ static int parse_fixed_partitions(struct mtd_info *master,
a_cells = of_n_addr_cells(pp);
s_cells = of_n_size_cells(pp);
+ if (s_cells == 0)
+ s_cells = 1; // for backward compatibility
if (len / 4 != a_cells + s_cells) {
pr_debug("%s: ofpart partition %pOF (%pOF) error parsing reg property.\n",
master->name, pp,
You might want to print a warning too, so users would fix their DTs,
since once there is MTD partition > 4 GiB, this would break. Otherwise I
like this option.
Thanks for looking into this !
