Re: Boot failure regression on 6.0.10 stable kernel on iMX7
+ u-boot list
On Thu, Dec 01, 2022 at 12:25:34PM +0100, Marek Vasut wrote:
On 12/1/22 12:03, Francesco Dolcini wrote:
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.
... 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
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 ?
I guess that it is slightly more complicated.
U-Boot directly updates the nand-controller root node with the partitions, unless there is already a partitions child node present. In the first case (legacy OF partition definition) setting the #size-cells does not seems that correct, while in the second case I agree it should really do it. I'll see what I can come-up with.
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.
I tested it and it's working as expected, I'll send a proper patch soon.
Francesco
On 12/1/22 16:45, Francesco Dolcini wrote:
- u-boot list
On Thu, Dec 01, 2022 at 12:25:34PM +0100, Marek Vasut wrote:
On 12/1/22 12:03, Francesco Dolcini wrote:
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.
... 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
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 ?
I guess that it is slightly more complicated.
U-Boot directly updates the nand-controller root node with the partitions, unless there is already a partitions child node present. In the first case (legacy OF partition definition) setting the #size-cells does not seems that correct, while in the second case I agree it should really do it. I'll see what I can come-up with.
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.
I tested it and it's working as expected, I'll send a proper patch soon.
Much appreciated, thanks !
participants (2)
-
Francesco Dolcini -
Marek Vasut