| Message ID | 20230113184547.487322-1-francesco@dolcini.it |
|---|---|
| Headers | show |
| Series | fdt: Fix mtparts fixup | expand |
On Fri, Jan 13, 2023 at 07:45:44PM +0100, Francesco Dolcini wrote: > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > Recently we had a boot regression on colibri-imx7 because of a cleanup change > on Linux imx7.dtsi setting nand controller node #size-cells from 1 to 0. > > Because of that Linux partition parser was no longer able to properly > parse the OF partitions leading to a boot failure, the above change was > reverted in the meantime as an immediate workaround, but some improvement > is required on both Linux and U-Boot. > > This change improve the U-Boot part of it, #size-cell is set to 1 when > it has an invalid value. This has the limitation to work only with devices > smaller than 4GiB. In general the suggestion from the Linux MTD maintainer would > be to just deprecate using this U-Boot function and pass the MTD partitions > from the command line, unless they are statically defined in the DTS file > in the first place. > > This series therefore convert colibri-imx6ull and colibri-imx7 to pass the > partition list from the command line instead of fixing up the DT. > > Link: https://lore.kernel.org/all/20221202071900.1143950-1-francesco@dolcini.it/ > Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/ My higher level question / concern here is, is using one of the dts partition schemes still valid / preferred, or should everyone now have reverted to passing via the kernel command line? If device tree still, is mtd/partitions/fixed-partitions.yaml the one to follow or something else?
Hi Tom, trini@konsulko.com wrote on Fri, 13 Jan 2023 14:34:11 -0500: > On Fri, Jan 13, 2023 at 07:45:44PM +0100, Francesco Dolcini wrote: > > > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > > > Recently we had a boot regression on colibri-imx7 because of a cleanup change > > on Linux imx7.dtsi setting nand controller node #size-cells from 1 to 0. > > > > Because of that Linux partition parser was no longer able to properly > > parse the OF partitions leading to a boot failure, the above change was > > reverted in the meantime as an immediate workaround, but some improvement > > is required on both Linux and U-Boot. > > > > This change improve the U-Boot part of it, #size-cell is set to 1 when > > it has an invalid value. This has the limitation to work only with devices > > smaller than 4GiB. In general the suggestion from the Linux MTD maintainer would > > be to just deprecate using this U-Boot function and pass the MTD partitions > > from the command line, unless they are statically defined in the DTS file > > in the first place. > > > > This series therefore convert colibri-imx6ull and colibri-imx7 to pass the > > partition list from the command line instead of fixing up the DT. > > > > Link: https://lore.kernel.org/all/20221202071900.1143950-1-francesco@dolcini.it/ > > Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/ > > My higher level question / concern here is, is using one of the dts > partition schemes still valid / preferred, or should everyone now have > reverted to passing via the kernel command line? If device tree still, > is mtd/partitions/fixed-partitions.yaml the one to follow or something > else? I don't think we can "prefer" one mode over the other between cmdline and DTS. Both should work pretty well. Of course on the cmdline you can only define fixed partitions and many devices require more advanced parsers, which are only available through DTS, but for simple partitions, it works totally okay. The only thing that I would like to avoid is the need to write code in the bootloaders to tweak the FDT in order to add partitions. That is clearly not needed, error prone, and do not follow evolution of the "standard", as we just discovered. Thanks, Miquèl
On Mon, Jan 23, 2023 at 11:06:06AM +0100, Miquel Raynal wrote: > Hi Tom, > > trini@konsulko.com wrote on Fri, 13 Jan 2023 14:34:11 -0500: > > > On Fri, Jan 13, 2023 at 07:45:44PM +0100, Francesco Dolcini wrote: > > > > > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > > > > > Recently we had a boot regression on colibri-imx7 because of a cleanup change > > > on Linux imx7.dtsi setting nand controller node #size-cells from 1 to 0. > > > > > > Because of that Linux partition parser was no longer able to properly > > > parse the OF partitions leading to a boot failure, the above change was > > > reverted in the meantime as an immediate workaround, but some improvement > > > is required on both Linux and U-Boot. > > > > > > This change improve the U-Boot part of it, #size-cell is set to 1 when > > > it has an invalid value. This has the limitation to work only with devices > > > smaller than 4GiB. In general the suggestion from the Linux MTD maintainer would > > > be to just deprecate using this U-Boot function and pass the MTD partitions > > > from the command line, unless they are statically defined in the DTS file > > > in the first place. > > > > > > This series therefore convert colibri-imx6ull and colibri-imx7 to pass the > > > partition list from the command line instead of fixing up the DT. > > > > > > Link: https://lore.kernel.org/all/20221202071900.1143950-1-francesco@dolcini.it/ > > > Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/ > > > > My higher level question / concern here is, is using one of the dts > > partition schemes still valid / preferred, or should everyone now have > > reverted to passing via the kernel command line? If device tree still, > > is mtd/partitions/fixed-partitions.yaml the one to follow or something > > else? > > I don't think we can "prefer" one mode over the other between cmdline > and DTS. Both should work pretty well. Of course on the cmdline you can > only define fixed partitions and many devices require more advanced > parsers, which are only available through DTS, but for simple > partitions, it works totally okay. When both are present, which one is used? > The only thing that I would like to avoid is the need to write code in > the bootloaders to tweak the FDT in order to add partitions. That is > clearly not needed, error prone, and do not follow evolution of the > "standard", as we just discovered. I'm not sure about this. Looking around in U-Boot today, I see two types of cases. One of which, the colibri case, can clearly be not done and either passed on the command line, or put in to the device tree as there's nothing run-time related being tweaked here. That's a fine path to take on those platforms and Francesco's patches should be updated to remove the unused C code too from the board code. But the other cases are doing something dynamic and run-time related. There's the omap3 igep00x0 family (which yes, legacy) that is doing NAND or oneNAND and adjusting things at run time. I don't know how much anyone has interest in those platforms at this point, nor exactly who to contact (for Linux or U-Boot). There's also the stm32mp1 family doing something that's very not obvious at first glance, so I've cc'd the maintainers there.
Hi, On 1/23/23 21:01, Tom Rini wrote: > On Mon, Jan 23, 2023 at 11:06:06AM +0100, Miquel Raynal wrote: >> Hi Tom, >> >> trini@konsulko.com wrote on Fri, 13 Jan 2023 14:34:11 -0500: >> >>> On Fri, Jan 13, 2023 at 07:45:44PM +0100, Francesco Dolcini wrote: >>> >>>> From: Francesco Dolcini <francesco.dolcini@toradex.com> >>>> >>>> Recently we had a boot regression on colibri-imx7 because of a cleanup change >>>> on Linux imx7.dtsi setting nand controller node #size-cells from 1 to 0. >>>> >>>> Because of that Linux partition parser was no longer able to properly >>>> parse the OF partitions leading to a boot failure, the above change was >>>> reverted in the meantime as an immediate workaround, but some improvement >>>> is required on both Linux and U-Boot. >>>> >>>> This change improve the U-Boot part of it, #size-cell is set to 1 when >>>> it has an invalid value. This has the limitation to work only with devices >>>> smaller than 4GiB. In general the suggestion from the Linux MTD maintainer would >>>> be to just deprecate using this U-Boot function and pass the MTD partitions >>>> from the command line, unless they are statically defined in the DTS file >>>> in the first place. >>>> >>>> This series therefore convert colibri-imx6ull and colibri-imx7 to pass the >>>> partition list from the command line instead of fixing up the DT. >>>> >>>> Link: https://lore.kernel.org/all/20221202071900.1143950-1-francesco@dolcini.it/ >>>> Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/ >>> My higher level question / concern here is, is using one of the dts >>> partition schemes still valid / preferred, or should everyone now have >>> reverted to passing via the kernel command line? If device tree still, >>> is mtd/partitions/fixed-partitions.yaml the one to follow or something >>> else? >> I don't think we can "prefer" one mode over the other between cmdline >> and DTS. Both should work pretty well. Of course on the cmdline you can >> only define fixed partitions and many devices require more advanced >> parsers, which are only available through DTS, but for simple >> partitions, it works totally okay. > When both are present, which one is used? > >> The only thing that I would like to avoid is the need to write code in >> the bootloaders to tweak the FDT in order to add partitions. That is >> clearly not needed, error prone, and do not follow evolution of the >> "standard", as we just discovered. > I'm not sure about this. Looking around in U-Boot today, I see two types > of cases. One of which, the colibri case, can clearly be not done and > either passed on the command line, or put in to the device tree as > there's nothing run-time related being tweaked here. That's a fine path > to take on those platforms and Francesco's patches should be updated to > remove the unused C code too from the board code. > > But the other cases are doing something dynamic and run-time related. > There's the omap3 igep00x0 family (which yes, legacy) that is doing NAND > or oneNAND and adjusting things at run time. I don't know how much > anyone has interest in those platforms at this point, nor exactly who to > contact (for Linux or U-Boot). There's also the stm32mp1 family doing > something that's very not obvious at first glance, so I've cc'd the > maintainers there. > For information, today for stm32mp1 family we are using the build of MTDPARTS and fdt fixup, only for backward compatibility issue (the MTD partitions change for boot with or without OP-TEE, with or wihtout FIP, with SPL). Today we are already plan to remove this dynamic management and to switch to static MTD partition defined in device tree, as already proposed by Tom in the serie "mtd: spi: nor: force mtd name to "nor%d"" http://patchwork.ozlabs.org/project/uboot/patch/20210916155040.v3.2.Ia461e670c7438478aa8f8939209d45c818ccd284@changeid/ This patchset is already ready, we are currently testing it internally and it should be pushed when it will be validated in our donwstream. Regards Patrick
On Thu, Feb 23, 2023 at 11:23:41AM +0100, Patrick DELAUNAY wrote: > Hi, > > On 1/23/23 21:01, Tom Rini wrote: > > On Mon, Jan 23, 2023 at 11:06:06AM +0100, Miquel Raynal wrote: > > > Hi Tom, > > > > > > trini@konsulko.com wrote on Fri, 13 Jan 2023 14:34:11 -0500: > > > > > > > On Fri, Jan 13, 2023 at 07:45:44PM +0100, Francesco Dolcini wrote: > > > > > > > > > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > > > > > > > > > Recently we had a boot regression on colibri-imx7 because of a cleanup change > > > > > on Linux imx7.dtsi setting nand controller node #size-cells from 1 to 0. > > > > > > > > > > Because of that Linux partition parser was no longer able to properly > > > > > parse the OF partitions leading to a boot failure, the above change was > > > > > reverted in the meantime as an immediate workaround, but some improvement > > > > > is required on both Linux and U-Boot. > > > > > > > > > > This change improve the U-Boot part of it, #size-cell is set to 1 when > > > > > it has an invalid value. This has the limitation to work only with devices > > > > > smaller than 4GiB. In general the suggestion from the Linux MTD maintainer would > > > > > be to just deprecate using this U-Boot function and pass the MTD partitions > > > > > from the command line, unless they are statically defined in the DTS file > > > > > in the first place. > > > > > > > > > > This series therefore convert colibri-imx6ull and colibri-imx7 to pass the > > > > > partition list from the command line instead of fixing up the DT. > > > > > > > > > > Link: https://lore.kernel.org/all/20221202071900.1143950-1-francesco@dolcini.it/ > > > > > Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/ > > > > My higher level question / concern here is, is using one of the dts > > > > partition schemes still valid / preferred, or should everyone now have > > > > reverted to passing via the kernel command line? If device tree still, > > > > is mtd/partitions/fixed-partitions.yaml the one to follow or something > > > > else? > > > I don't think we can "prefer" one mode over the other between cmdline > > > and DTS. Both should work pretty well. Of course on the cmdline you can > > > only define fixed partitions and many devices require more advanced > > > parsers, which are only available through DTS, but for simple > > > partitions, it works totally okay. > > When both are present, which one is used? > > > > > The only thing that I would like to avoid is the need to write code in > > > the bootloaders to tweak the FDT in order to add partitions. That is > > > clearly not needed, error prone, and do not follow evolution of the > > > "standard", as we just discovered. > > I'm not sure about this. Looking around in U-Boot today, I see two types > > of cases. One of which, the colibri case, can clearly be not done and > > either passed on the command line, or put in to the device tree as > > there's nothing run-time related being tweaked here. That's a fine path > > to take on those platforms and Francesco's patches should be updated to > > remove the unused C code too from the board code. > > > > But the other cases are doing something dynamic and run-time related. > > There's the omap3 igep00x0 family (which yes, legacy) that is doing NAND > > or oneNAND and adjusting things at run time. I don't know how much > > anyone has interest in those platforms at this point, nor exactly who to > > contact (for Linux or U-Boot). There's also the stm32mp1 family doing > > something that's very not obvious at first glance, so I've cc'd the > > maintainers there. > > > > For information, today for stm32mp1 family we are using the build > > of MTDPARTS and fdt fixup, only for backward compatibility issue > > (the MTD partitions change for boot with or without OP-TEE, > > with or wihtout FIP, with SPL). > > > Today we are already plan to remove this dynamic management > > and to switch to static MTD partition defined in device tree, > > as already proposed by Tom in the serie > > "mtd: spi: nor: force mtd name to "nor%d"" > > http://patchwork.ozlabs.org/project/uboot/patch/20210916155040.v3.2.Ia461e670c7438478aa8f8939209d45c818ccd284@changeid/ > > > This patchset is already ready, we are currently testing it internally > > and it should be pushed when it will be validated in our donwstream. Great, thanks.
From: Francesco Dolcini <francesco.dolcini@toradex.com> Recently we had a boot regression on colibri-imx7 because of a cleanup change on Linux imx7.dtsi setting nand controller node #size-cells from 1 to 0. Because of that Linux partition parser was no longer able to properly parse the OF partitions leading to a boot failure, the above change was reverted in the meantime as an immediate workaround, but some improvement is required on both Linux and U-Boot. This change improve the U-Boot part of it, #size-cell is set to 1 when it has an invalid value. This has the limitation to work only with devices smaller than 4GiB. In general the suggestion from the Linux MTD maintainer would be to just deprecate using this U-Boot function and pass the MTD partitions from the command line, unless they are statically defined in the DTS file in the first place. This series therefore convert colibri-imx6ull and colibri-imx7 to pass the partition list from the command line instead of fixing up the DT. Link: https://lore.kernel.org/all/20221202071900.1143950-1-francesco@dolcini.it/ Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/ Francesco Dolcini (3): fdt: validate/fix cells count on mtdpart fixup colibri-imx7: specify MTD partitions on command line colibri-imx6ull: specify MTD partitions on command line common/fdt_support.c | 45 ++++++++++++++++++++++++------- configs/colibri-imx6ull_defconfig | 1 - configs/colibri_imx7_defconfig | 1 - include/configs/colibri-imx6ull.h | 2 +- include/configs/colibri_imx7.h | 2 +- 5 files changed, 37 insertions(+), 14 deletions(-)