| Message ID | 20180522120257.13232-6-marek.vasut+renesas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 22, 2018 at 02:02:54PM +0200, Marek Vasut wrote: > Drop the MTD partitioning from DT, since it does not describe HW > and to give way to a more flexible kernel command line partition > passing. > > To retain the original partitioning, assure you have enabled > CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the > following to your kernel command line: > > mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Geert Uytterhoeven <geert+renesas@glider.be> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Simon Horman <horms+renesas@verge.net.au> > Cc: Wolfram Sang <wsa@the-dreams.de> > Cc: linux-renesas-soc@vger.kernel.org Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > Drop the MTD partitioning from DT, since it does not describe HW > and to give way to a more flexible kernel command line partition > passing. > > To retain the original partitioning, assure you have enabled > CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the > following to your kernel command line: > > mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) I think the "@0" can be dropped, as it's optional? 4m? (Gaining more knowledge during reviewing ;-) > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Gr{oetje,eeting}s, Geert
On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote: > On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >> Drop the MTD partitioning from DT, since it does not describe HW >> and to give way to a more flexible kernel command line partition >> passing. >> >> To retain the original partitioning, assure you have enabled >> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the >> following to your kernel command line: >> >> mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) > > I think the "@0" can be dropped, as it's optional? > 4m? My take on this is that the loader is actually at offset 0x0 of the MTD device and we explicitly state that in the mtdparts to anchor the first partition within the MTD device and all the other partitions are at offset +(sum of the sizes of all partitions listed before the current one) relative to that first partition. Removing the @0 feels fragile at best and it seems to depend on the current behavior of the code. > (Gaining more knowledge during reviewing ;-) > >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > > Gr{oetje,eeting}s, > > Geert >
Hi Marek, On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote: > On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote: >> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >>> Drop the MTD partitioning from DT, since it does not describe HW >>> and to give way to a more flexible kernel command line partition >>> passing. >>> >>> To retain the original partitioning, assure you have enabled >>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the >>> following to your kernel command line: >>> >>> mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) >> >> I think the "@0" can be dropped, as it's optional? >> 4m? > > My take on this is that the loader is actually at offset 0x0 of the MTD > device and we explicitly state that in the mtdparts to anchor the first > partition within the MTD device and all the other partitions are at > offset +(sum of the sizes of all partitions listed before the current > one) relative to that first partition. Where is this explicitly states for the first partition? > Removing the @0 feels fragile at best and it seems to depend on the > current behavior of the code. Better, it also depends on the documented behavior: Documentation/admin-guide/kernel-parameters.txt refers to drivers/mtd/cmdlinepart.c, which states: * <offset> := standard linux memsize * if omitted the part will immediately follow the previous part * or 0 if the first part None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig help text, or in a defconfig bundled with the kernel, use @0 for the first partition. Gr{oetje,eeting}s, Geert
On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote: > Hi Marek, > > On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote: >> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote: >>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >>>> Drop the MTD partitioning from DT, since it does not describe HW >>>> and to give way to a more flexible kernel command line partition >>>> passing. >>>> >>>> To retain the original partitioning, assure you have enabled >>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the >>>> following to your kernel command line: >>>> >>>> mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) >>> >>> I think the "@0" can be dropped, as it's optional? >>> 4m? >> >> My take on this is that the loader is actually at offset 0x0 of the MTD >> device and we explicitly state that in the mtdparts to anchor the first >> partition within the MTD device and all the other partitions are at >> offset +(sum of the sizes of all partitions listed before the current >> one) relative to that first partition. > > Where is this explicitly states for the first partition? > >> Removing the @0 feels fragile at best and it seems to depend on the >> current behavior of the code. > > Better, it also depends on the documented behavior: > > Documentation/admin-guide/kernel-parameters.txt refers to > drivers/mtd/cmdlinepart.c, which states: > > * <offset> := standard linux memsize > * if omitted the part will immediately follow the previous part > * or 0 if the first part > > None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig > help text, or in a defconfig bundled with the kernel, use @0 for the first > partition. I think this is exceptionally fragile and dangerous to depend on this, but so be it.
On Thu, May 24, 2018 at 04:52:59PM +0200, Marek Vasut wrote: > On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote: > > Hi Marek, > > > > On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote: > >> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote: > >>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > >>>> Drop the MTD partitioning from DT, since it does not describe HW > >>>> and to give way to a more flexible kernel command line partition > >>>> passing. > >>>> > >>>> To retain the original partitioning, assure you have enabled > >>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the > >>>> following to your kernel command line: > >>>> > >>>> mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) > >>> > >>> I think the "@0" can be dropped, as it's optional? > >>> 4m? > >> > >> My take on this is that the loader is actually at offset 0x0 of the MTD > >> device and we explicitly state that in the mtdparts to anchor the first > >> partition within the MTD device and all the other partitions are at > >> offset +(sum of the sizes of all partitions listed before the current > >> one) relative to that first partition. > > > > Where is this explicitly states for the first partition? > > > >> Removing the @0 feels fragile at best and it seems to depend on the > >> current behavior of the code. > > > > Better, it also depends on the documented behavior: > > > > Documentation/admin-guide/kernel-parameters.txt refers to > > drivers/mtd/cmdlinepart.c, which states: > > > > * <offset> := standard linux memsize > > * if omitted the part will immediately follow the previous part > > * or 0 if the first part > > > > None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig > > help text, or in a defconfig bundled with the kernel, use @0 for the first > > partition. > > I think this is exceptionally fragile and dangerous to depend on this, > but so be it. Could you respin with this change? I would also like to ask for another change, in light of recent feedback from Olof Johansson ("Re: [GIT PULL] Renesas ARM64 Based SoC DT Updates for v4.18"). Please consolidate the dts patches into a single patch? I have applied the defconfig patch, so there is no need to repost that one.
Hi Simon, On Mon, May 28, 2018 at 10:41 AM, Simon Horman <horms@verge.net.au> wrote: > On Thu, May 24, 2018 at 04:52:59PM +0200, Marek Vasut wrote: >> On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote: >> > On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote: >> >> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote: >> >>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >> >>>> Drop the MTD partitioning from DT, since it does not describe HW >> >>>> and to give way to a more flexible kernel command line partition >> >>>> passing. >> >>>> >> >>>> To retain the original partitioning, assure you have enabled >> >>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the >> >>>> following to your kernel command line: >> >>>> >> >>>> mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) >> >>> >> >>> I think the "@0" can be dropped, as it's optional? >> >>> 4m? >> >> >> >> My take on this is that the loader is actually at offset 0x0 of the MTD >> >> device and we explicitly state that in the mtdparts to anchor the first >> >> partition within the MTD device and all the other partitions are at >> >> offset +(sum of the sizes of all partitions listed before the current >> >> one) relative to that first partition. >> > >> > Where is this explicitly states for the first partition? >> > >> >> Removing the @0 feels fragile at best and it seems to depend on the >> >> current behavior of the code. >> > >> > Better, it also depends on the documented behavior: >> > >> > Documentation/admin-guide/kernel-parameters.txt refers to >> > drivers/mtd/cmdlinepart.c, which states: >> > >> > * <offset> := standard linux memsize >> > * if omitted the part will immediately follow the previous part >> > * or 0 if the first part >> > >> > None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig >> > help text, or in a defconfig bundled with the kernel, use @0 for the first >> > partition. >> >> I think this is exceptionally fragile and dangerous to depend on this, >> but so be it. > > Could you respin with this change? > > I would also like to ask for another change, in light of recent > feedback from Olof Johansson ("Re: [GIT PULL] Renesas ARM64 Based SoC DT > Updates for v4.18"). > > Please consolidate the dts patches into a single patch? I think it's better to keep them split, as each commit description mentions what needs to be passed on the kernel command line for the corresponding board. Combining it in a single patch makes it much harder to extract this information. Unless you're fine with a list: koelsch: ... wheat: mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) Gr{oetje,eeting}s, Geert
On Mon, May 28, 2018 at 10:54:57AM +0200, Geert Uytterhoeven wrote: > Hi Simon, > > On Mon, May 28, 2018 at 10:41 AM, Simon Horman <horms@verge.net.au> wrote: > > On Thu, May 24, 2018 at 04:52:59PM +0200, Marek Vasut wrote: > >> On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote: > >> > On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote: > >> >> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote: > >> >>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > >> >>>> Drop the MTD partitioning from DT, since it does not describe HW > >> >>>> and to give way to a more flexible kernel command line partition > >> >>>> passing. > >> >>>> > >> >>>> To retain the original partitioning, assure you have enabled > >> >>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the > >> >>>> following to your kernel command line: > >> >>>> > >> >>>> mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) > >> >>> > >> >>> I think the "@0" can be dropped, as it's optional? > >> >>> 4m? > >> >> > >> >> My take on this is that the loader is actually at offset 0x0 of the MTD > >> >> device and we explicitly state that in the mtdparts to anchor the first > >> >> partition within the MTD device and all the other partitions are at > >> >> offset +(sum of the sizes of all partitions listed before the current > >> >> one) relative to that first partition. > >> > > >> > Where is this explicitly states for the first partition? > >> > > >> >> Removing the @0 feels fragile at best and it seems to depend on the > >> >> current behavior of the code. > >> > > >> > Better, it also depends on the documented behavior: > >> > > >> > Documentation/admin-guide/kernel-parameters.txt refers to > >> > drivers/mtd/cmdlinepart.c, which states: > >> > > >> > * <offset> := standard linux memsize > >> > * if omitted the part will immediately follow the previous part > >> > * or 0 if the first part > >> > > >> > None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig > >> > help text, or in a defconfig bundled with the kernel, use @0 for the first > >> > partition. > >> > >> I think this is exceptionally fragile and dangerous to depend on this, > >> but so be it. > > > > Could you respin with this change? > > > > I would also like to ask for another change, in light of recent > > feedback from Olof Johansson ("Re: [GIT PULL] Renesas ARM64 Based SoC DT > > Updates for v4.18"). > > > > Please consolidate the dts patches into a single patch? > > I think it's better to keep them split, as each commit description mentions > what needs to be passed on the kernel command line for the corresponding > board. > > Combining it in a single patch makes it much harder to extract this information. > Unless you're fine with a list: > > koelsch: ... > wheat: mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) Lets try a list.
On 05/28/2018 11:36 AM, Simon Horman wrote: > On Mon, May 28, 2018 at 10:54:57AM +0200, Geert Uytterhoeven wrote: >> Hi Simon, >> >> On Mon, May 28, 2018 at 10:41 AM, Simon Horman <horms@verge.net.au> wrote: >>> On Thu, May 24, 2018 at 04:52:59PM +0200, Marek Vasut wrote: >>>> On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote: >>>>> On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote: >>>>>> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote: >>>>>>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >>>>>>>> Drop the MTD partitioning from DT, since it does not describe HW >>>>>>>> and to give way to a more flexible kernel command line partition >>>>>>>> passing. >>>>>>>> >>>>>>>> To retain the original partitioning, assure you have enabled >>>>>>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the >>>>>>>> following to your kernel command line: >>>>>>>> >>>>>>>> mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) >>>>>>> >>>>>>> I think the "@0" can be dropped, as it's optional? >>>>>>> 4m? >>>>>> >>>>>> My take on this is that the loader is actually at offset 0x0 of the MTD >>>>>> device and we explicitly state that in the mtdparts to anchor the first >>>>>> partition within the MTD device and all the other partitions are at >>>>>> offset +(sum of the sizes of all partitions listed before the current >>>>>> one) relative to that first partition. >>>>> >>>>> Where is this explicitly states for the first partition? >>>>> >>>>>> Removing the @0 feels fragile at best and it seems to depend on the >>>>>> current behavior of the code. >>>>> >>>>> Better, it also depends on the documented behavior: >>>>> >>>>> Documentation/admin-guide/kernel-parameters.txt refers to >>>>> drivers/mtd/cmdlinepart.c, which states: >>>>> >>>>> * <offset> := standard linux memsize >>>>> * if omitted the part will immediately follow the previous part >>>>> * or 0 if the first part >>>>> >>>>> None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig >>>>> help text, or in a defconfig bundled with the kernel, use @0 for the first >>>>> partition. >>>> >>>> I think this is exceptionally fragile and dangerous to depend on this, >>>> but so be it. >>> >>> Could you respin with this change? >>> >>> I would also like to ask for another change, in light of recent >>> feedback from Olof Johansson ("Re: [GIT PULL] Renesas ARM64 Based SoC DT >>> Updates for v4.18"). >>> >>> Please consolidate the dts patches into a single patch? >> >> I think it's better to keep them split, as each commit description mentions >> what needs to be passed on the kernel command line for the corresponding >> board. >> >> Combining it in a single patch makes it much harder to extract this information. >> Unless you're fine with a list: >> >> koelsch: ... >> wheat: mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) > > Lets try a list. Reposted with a list, twice :/
On Wed, May 30, 2018 at 12:13:31PM +0200, Marek Vasut wrote: > On 05/28/2018 11:36 AM, Simon Horman wrote: > > On Mon, May 28, 2018 at 10:54:57AM +0200, Geert Uytterhoeven wrote: > >> Hi Simon, > >> > >> On Mon, May 28, 2018 at 10:41 AM, Simon Horman <horms@verge.net.au> wrote: > >>> On Thu, May 24, 2018 at 04:52:59PM +0200, Marek Vasut wrote: > >>>> On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote: > >>>>> On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote: > >>>>>> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote: > >>>>>>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > >>>>>>>> Drop the MTD partitioning from DT, since it does not describe HW > >>>>>>>> and to give way to a more flexible kernel command line partition > >>>>>>>> passing. > >>>>>>>> > >>>>>>>> To retain the original partitioning, assure you have enabled > >>>>>>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the > >>>>>>>> following to your kernel command line: > >>>>>>>> > >>>>>>>> mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) > >>>>>>> > >>>>>>> I think the "@0" can be dropped, as it's optional? > >>>>>>> 4m? > >>>>>> > >>>>>> My take on this is that the loader is actually at offset 0x0 of the MTD > >>>>>> device and we explicitly state that in the mtdparts to anchor the first > >>>>>> partition within the MTD device and all the other partitions are at > >>>>>> offset +(sum of the sizes of all partitions listed before the current > >>>>>> one) relative to that first partition. > >>>>> > >>>>> Where is this explicitly states for the first partition? > >>>>> > >>>>>> Removing the @0 feels fragile at best and it seems to depend on the > >>>>>> current behavior of the code. > >>>>> > >>>>> Better, it also depends on the documented behavior: > >>>>> > >>>>> Documentation/admin-guide/kernel-parameters.txt refers to > >>>>> drivers/mtd/cmdlinepart.c, which states: > >>>>> > >>>>> * <offset> := standard linux memsize > >>>>> * if omitted the part will immediately follow the previous part > >>>>> * or 0 if the first part > >>>>> > >>>>> None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig > >>>>> help text, or in a defconfig bundled with the kernel, use @0 for the first > >>>>> partition. > >>>> > >>>> I think this is exceptionally fragile and dangerous to depend on this, > >>>> but so be it. > >>> > >>> Could you respin with this change? > >>> > >>> I would also like to ask for another change, in light of recent > >>> feedback from Olof Johansson ("Re: [GIT PULL] Renesas ARM64 Based SoC DT > >>> Updates for v4.18"). > >>> > >>> Please consolidate the dts patches into a single patch? > >> > >> I think it's better to keep them split, as each commit description mentions > >> what needs to be passed on the kernel command line for the corresponding > >> board. > >> > >> Combining it in a single patch makes it much harder to extract this information. > >> Unless you're fine with a list: > >> > >> koelsch: ... > >> wheat: mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) > > > > Lets try a list. > > Reposted with a list, twice :/ Thanks, got it :)
diff --git a/arch/arm/boot/dts/r8a7792-wheat.dts b/arch/arm/boot/dts/r8a7792-wheat.dts index db01de7a3811..93f78716225a 100644 --- a/arch/arm/boot/dts/r8a7792-wheat.dts +++ b/arch/arm/boot/dts/r8a7792-wheat.dts @@ -217,27 +217,6 @@ spi-cpol; spi-cpha; m25p,fast-read; - - partitions { - compatible = "fixed-partitions"; - #address-cells = <1>; - #size-cells = <1>; - - partition@0 { - label = "loader"; - reg = <0x00000000 0x00040000>; - read-only; - }; - partition@40000 { - label = "user"; - reg = <0x00040000 0x00400000>; - read-only; - }; - partition@440000 { - label = "flash"; - reg = <0x00440000 0x03bc0000>; - }; - }; }; };
Drop the MTD partitioning from DT, since it does not describe HW and to give way to a more flexible kernel command line partition passing. To retain the original partitioning, assure you have enabled CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the following to your kernel command line: mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash) Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Cc: Geert Uytterhoeven <geert+renesas@glider.be> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Simon Horman <horms+renesas@verge.net.au> Cc: Wolfram Sang <wsa@the-dreams.de> Cc: linux-renesas-soc@vger.kernel.org --- arch/arm/boot/dts/r8a7792-wheat.dts | 21 --------------------- 1 file changed, 21 deletions(-)