Boot failure regression on 6.0.10 stable kernel on iMX7

Boot failure regression on 6.0.10 stable kernel on iMX7

[Francesco Dolcini]

Hello Marek,
it looks like commit 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller
size-cells"), that was backported to stable 6.0.10, introduce a boot
regression on colibri-imx7, at least.

What I get is

[    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.

Francesco



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Boot failure regression on 6.0.10 stable kernel on iMX7

[Marek Vasut]

On 11/30/22 14:52, Francesco Dolcini wrote:
> Hello Marek,

Hi,

> it looks like commit 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller
> size-cells"), that was backported to stable 6.0.10, introduce a boot
> regression on colibri-imx7, at least.
> 
> What I get is
> 
> [    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 ? 
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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Boot failure regression on 6.0.10 stable kernel on iMX7

[Francesco Dolcini]

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.

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Boot failure regression on 6.0.10 stable kernel on iMX7

[Marek Vasut]

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.

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> ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Boot failure regression on 6.0.10 stable kernel on iMX7

[Francesco Dolcini]

+ 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.

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.

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*).

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,

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Boot failure regression on 6.0.10 stable kernel on iMX7

[Marek Vasut]

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 !

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Boot failure regression on 6.0.10 stable kernel on iMX7 #forregzbot

[Thorsten Leemhuis]

[Note: this mail contains only information relevant for regzbot, the
Linux kernel regression tracking bot. That's why I removed most
recipients of the quoted mail and only left what looked like a mailing
lists. These mails contain '#forregzbot' in the subject, to make them
easy to spot and filter out.]

On 30.11.22 14:52, Francesco Dolcini wrote:
> it looks like commit 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller
> size-cells"), that was backported to stable 6.0.10, introduce a boot
> regression on colibri-imx7, at least.
> [...]

Thanks for the report. To be sure below issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
tracking bot:

#regzbot ^introduced 753395ea1e45
#regzbot title ARM: dts: imx7: Boot failure on iMX7
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply -- ideally with also
telling regzbot about it, as explained here:
https://linux-regtracking.leemhuis.info/tracked-regression/

Reminder for developers: When fixing the issue, add 'Link:' tags
pointing to the report (the mail this one replies to), as explained for
in the Linux kernel's documentation; above webpage explains why this is
important for tracked regressions.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Boot failure regression on 6.0.10 stable kernel on iMX7

[Francesco Dolcini]

+ 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Boot failure regression on 6.0.10 stable kernel on iMX7

[Marek Vasut]

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 !

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel