Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
On Tue, 2023-01-24 at 10:44:44 UTC, Francesco Dolcini wrote: > From: Francesco Dolcini > > Add a mechanism to handle the case in which partitions are present as > direct child of the nand controller node and #size-cells is set to <0>. > > This could happen if the nand-controller node in the DTS is supposed to > have #size-cells set to 0, but for some historical reason/bug it was set > to 1 in the past, and the firmware (e.g. U-Boot) is adding the partition > as direct children of the nand-controller defaulting to #size-cells > being to 1. > > This prevents a real boot failure on colibri-imx7 that happened during v6.1 > development cycles. > > Link: > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/ > Link: > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/ > Signed-off-by: Francesco Dolcini > Reviewed-by: Greg Kroah-Hartman Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks. Miquel
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
On Fri, Feb 03, 2023 at 07:37:03PM +0100, Francesco Dolcini wrote:
>
>
> Il 3 febbraio 2023 19:16:23 CET, Miquel Raynal ha
> scritto:
> >Hi Francesco,
> >
> >france...@dolcini.it wrote on Fri, 03 Feb 2023 19:03:27 +0100:
> >
> >> Il 3 febbraio 2023 16:12:02 CET, Miquel Raynal
> >> ha scritto:
> >> >Hi Francesco,
> >> >
> >> >france...@dolcini.it wrote on Thu, 2 Feb 2023 12:33:34 +0100:
> >> >
> >> >> On Thu, Jan 26, 2023 at 10:12:04AM +0100, Miquel Raynal wrote:
> >> >> > gre...@linuxfoundation.org wrote on Thu, 26 Jan 2023 10:01:02 +0100:
> >> >> >
> >> >> > > On Wed, Jan 25, 2023 at 10:06:57PM +0100, Francesco Dolcini wrote:
> >> >> > >
> >> >> > > > Hello Miquel, Greg and all
> >> >> > > >
> >> >> > > > On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman
> >> >> > > > wrote:
> >> >> > > > > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini
> >> >> > > > > wrote:
> >> >> > > > > > From: Francesco Dolcini
> >> >> > > > > >
> >> >> > > > > > Add a mechanism to handle the case in which partitions are
> >> >> > > > > > present as
> >> >> > > > > > direct child of the nand controller node and #size-cells is
> >> >> > > > > > set to <0>.
> >> >> > > > > >
> >> >> > > > > > This could happen if the nand-controller node in the DTS is
> >> >> > > > > > supposed to
> >> >> > > > > > have #size-cells set to 0, but for some historical reason/bug
> >> >> > > > > > it was set
> >> >> > > > > > to 1 in the past, and the firmware (e.g. U-Boot) is adding
> >> >> > > > > > the partition
> >> >> > > > > > as direct children of the nand-controller defaulting to
> >> >> > > > > > #size-cells
> >> >> > > > > > being to 1.
> >> >> > > > > >
> >> >> > > > > > This prevents a real boot failure on colibri-imx7 that
> >> >> > > > > > happened during v6.1
> >> >> > > > > > development cycles.
> >> >> > > > > >
> >> >> > > > > > Link:
> >> >> > > > > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> >> >> > > > > > Link:
> >> >> > > > > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> >> >> > > > > > Signed-off-by: Francesco Dolcini
> >> >> > > > > >
> >> >> > > > > > Reviewed-by: Greg Kroah-Hartman
> >> >> > > > > > ---
> >> >> > > > > > I do not expect this patch to be backported to stable,
> >> >> > > > > > however I would expect
> >> >> > > > > > that we do not backport nand-controller dts cleanups neither.
> >> >> > > > > >
> >> >> > > > > > v4:
> >> >> > > > > > fixed wrong English spelling in the comment
> >> >> > > > > >
> >> >> > > > > > v3:
> >> >> > > > > > minor formatting change, removed not needed new-line and
> >> >> > > > > > space.
> >> >> > > > > >
> >> >> > > > > > v2:
> >> >> > > > > > fixup size-cells only when partitions are direct children of
> >> >> > > > > > the nand-controller
> >> >> > > > > > completely revised commit message, comments and warning print
> >> >> > > > > > use pr_warn instead of pr_warn_once
> >> >> > > > > > added Reviewed-by Greg
> >> >> > > > > > removed cc:stable@ and fixes tag, since the problematic
> >> >> > > > > > commit was reverted
> >> >> > > > > > ---
> >> >> > > > > > drivers/mtd/parsers/ofpart_core.c | 19 +++
> >> >> > > > > > 1 file changed, 19 insertions(+)
> >> >> > > > > >
> >> >> > > > > > diff --git a/drivers/mtd/parsers/ofpart_core.c
> >> >> > > > > > b/drivers/mtd/parsers/ofpart_core.c
> >> >> > > > > > index 192190c42fc8..e7b8e9d0a910 100644
> >> >> > > > > > --- a/drivers/mtd/parsers/ofpart_core.c
> >> >> > > > > > +++ b/drivers/mtd/parsers/ofpart_core.c
> >> >> > > > > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct
> >> >> > > > > > mtd_info *master,
> >> >> > > > > >
> >> >> > > > > > a_cells = of_n_addr_cells(pp);
> >> >> > > > > > s_cells = of_n_size_cells(pp);
> >> >> > > > > > + if (!dedicated && s_cells == 0) {
> >> >> > > > > > + /*
> >> >> > > > > > + * This is a ugly workaround to not
> >> >> > > > > > create
> >> >> > > > > > + * regression on devices that are still
> >> >> > > > > > creating
> >> >> > > > > > + * partitions as direct children of the
> >> >> > > > > > nand controller.
> >> >> > > > > > + * This can happen in case the nand
> >> >> > > > > > controller node has
> >> >> > > > > > + * #size-cells equal to 0 and the
> >> >> > > > > > firmware (e.g.
> >> >> > > > > > + * U-Boot) just add the partitions
> >> >> > > > > > there assuming
> >> >> > > > > > + * 32-bit addressing.
> >> >> > > > > > + *
> >> >> > > > > > + * If you get this warning your
> >> >> > > > > > firmware and/or DTS
> >> >> > > > > > + * should be really fixed.
> >> >> > > > > > + *
> >> >> > > > > > +
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
Il 3 febbraio 2023 19:16:23 CET, Miquel Raynal ha
scritto:
>Hi Francesco,
>
>france...@dolcini.it wrote on Fri, 03 Feb 2023 19:03:27 +0100:
>
>> Il 3 febbraio 2023 16:12:02 CET, Miquel Raynal
>> ha scritto:
>> >Hi Francesco,
>> >
>> >france...@dolcini.it wrote on Thu, 2 Feb 2023 12:33:34 +0100:
>> >
>> >> On Thu, Jan 26, 2023 at 10:12:04AM +0100, Miquel Raynal wrote:
>> >> > gre...@linuxfoundation.org wrote on Thu, 26 Jan 2023 10:01:02 +0100:
>> >> >
>> >> > > On Wed, Jan 25, 2023 at 10:06:57PM +0100, Francesco Dolcini wrote:
>> >> > > > Hello Miquel, Greg and all
>> >> > > >
>> >> > > > On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
>> >> > > >
>> >> > > > > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini
>> >> > > > > wrote:
>> >> > > > > > From: Francesco Dolcini
>> >> > > > > >
>> >> > > > > > Add a mechanism to handle the case in which partitions are
>> >> > > > > > present as
>> >> > > > > > direct child of the nand controller node and #size-cells is set
>> >> > > > > > to <0>.
>> >> > > > > >
>> >> > > > > > This could happen if the nand-controller node in the DTS is
>> >> > > > > > supposed to
>> >> > > > > > have #size-cells set to 0, but for some historical reason/bug
>> >> > > > > > it was set
>> >> > > > > > to 1 in the past, and the firmware (e.g. U-Boot) is adding the
>> >> > > > > > partition
>> >> > > > > > as direct children of the nand-controller defaulting to
>> >> > > > > > #size-cells
>> >> > > > > > being to 1.
>> >> > > > > >
>> >> > > > > > This prevents a real boot failure on colibri-imx7 that happened
>> >> > > > > > during v6.1
>> >> > > > > > development cycles.
>> >> > > > > >
>> >> > > > > > Link:
>> >> > > > > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
>> >> > > > > > Link:
>> >> > > > > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
>> >> > > > > > Signed-off-by: Francesco Dolcini
>> >> > > > > > Reviewed-by: Greg Kroah-Hartman
>> >> > > > > > ---
>> >> > > > > > I do not expect this patch to be backported to stable, however
>> >> > > > > > I would expect
>> >> > > > > > that we do not backport nand-controller dts cleanups neither.
>> >> > > > > >
>> >> > > > > > v4:
>> >> > > > > > fixed wrong English spelling in the comment
>> >> > > > > >
>> >> > > > > > v3:
>> >> > > > > > minor formatting change, removed not needed new-line and
>> >> > > > > > space.
>> >> > > > > >
>> >> > > > > > v2:
>> >> > > > > > fixup size-cells only when partitions are direct children of
>> >> > > > > > the nand-controller
>> >> > > > > > completely revised commit message, comments and warning print
>> >> > > > > > use pr_warn instead of pr_warn_once
>> >> > > > > > added Reviewed-by Greg
>> >> > > > > > removed cc:stable@ and fixes tag, since the problematic commit
>> >> > > > > > was reverted
>> >> > > > > > ---
>> >> > > > > > drivers/mtd/parsers/ofpart_core.c | 19 +++
>> >> > > > > > 1 file changed, 19 insertions(+)
>> >> > > > > >
>> >> > > > > > diff --git a/drivers/mtd/parsers/ofpart_core.c
>> >> > > > > > b/drivers/mtd/parsers/ofpart_core.c
>> >> > > > > > index 192190c42fc8..e7b8e9d0a910 100644
>> >> > > > > > --- a/drivers/mtd/parsers/ofpart_core.c
>> >> > > > > > +++ b/drivers/mtd/parsers/ofpart_core.c
>> >> > > > > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct
>> >> > > > > > mtd_info *master,
>> >> > > > > >
>> >> > > > > > a_cells = of_n_addr_cells(pp);
>> >> > > > > > s_cells = of_n_size_cells(pp);
>> >> > > > > > + if (!dedicated && s_cells == 0) {
>> >> > > > > > + /*
>> >> > > > > > +* This is a ugly workaround to not
>> >> > > > > > create
>> >> > > > > > +* regression on devices that are still
>> >> > > > > > creating
>> >> > > > > > +* partitions as direct children of the
>> >> > > > > > nand controller.
>> >> > > > > > +* This can happen in case the nand
>> >> > > > > > controller node has
>> >> > > > > > +* #size-cells equal to 0 and the
>> >> > > > > > firmware (e.g.
>> >> > > > > > +* U-Boot) just add the partitions
>> >> > > > > > there assuming
>> >> > > > > > +* 32-bit addressing.
>> >> > > > > > +*
>> >> > > > > > +* If you get this warning your
>> >> > > > > > firmware and/or DTS
>> >> > > > > > +* should be really fixed.
>> >> > > > > > +*
>> >> > > > > > +* This is working only for devices
>> >> > > > > > smaller than 4GiB.
>> >> > > > > > +*/
>> >> > > > > > + pr_warn("%s: ofpart partition %pOF
>> >> > > > > > (%pOF) #size-cells is wrongly set to <0>, assuming
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
Hi Francesco,
france...@dolcini.it wrote on Fri, 03 Feb 2023 19:03:27 +0100:
> Il 3 febbraio 2023 16:12:02 CET, Miquel Raynal ha
> scritto:
> >Hi Francesco,
> >
> >france...@dolcini.it wrote on Thu, 2 Feb 2023 12:33:34 +0100:
> >
> >> On Thu, Jan 26, 2023 at 10:12:04AM +0100, Miquel Raynal wrote:
> >> > gre...@linuxfoundation.org wrote on Thu, 26 Jan 2023 10:01:02 +0100:
> >> >
> >> > > On Wed, Jan 25, 2023 at 10:06:57PM +0100, Francesco Dolcini wrote:
> >> > > > Hello Miquel, Greg and all
> >> > > >
> >> > > > On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
> >> > > >
> >> > > > > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
> >> > > > >
> >> > > > > > From: Francesco Dolcini
> >> > > > > >
> >> > > > > > Add a mechanism to handle the case in which partitions are
> >> > > > > > present as
> >> > > > > > direct child of the nand controller node and #size-cells is set
> >> > > > > > to <0>.
> >> > > > > >
> >> > > > > > This could happen if the nand-controller node in the DTS is
> >> > > > > > supposed to
> >> > > > > > have #size-cells set to 0, but for some historical reason/bug it
> >> > > > > > was set
> >> > > > > > to 1 in the past, and the firmware (e.g. U-Boot) is adding the
> >> > > > > > partition
> >> > > > > > as direct children of the nand-controller defaulting to
> >> > > > > > #size-cells
> >> > > > > > being to 1.
> >> > > > > >
> >> > > > > > This prevents a real boot failure on colibri-imx7 that happened
> >> > > > > > during v6.1
> >> > > > > > development cycles.
> >> > > > > >
> >> > > > > > Link:
> >> > > > > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> >> > > > > > Link:
> >> > > > > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> >> > > > > > Signed-off-by: Francesco Dolcini
> >> > > > > > Reviewed-by: Greg Kroah-Hartman
> >> > > > > > ---
> >> > > > > > I do not expect this patch to be backported to stable, however I
> >> > > > > > would expect
> >> > > > > > that we do not backport nand-controller dts cleanups neither.
> >> > > > > >
> >> > > > > > v4:
> >> > > > > > fixed wrong English spelling in the comment
> >> > > > > >
> >> > > > > > v3:
> >> > > > > > minor formatting change, removed not needed new-line and space.
> >> > > > > >
> >> > > > > > v2:
> >> > > > > > fixup size-cells only when partitions are direct children of
> >> > > > > > the nand-controller
> >> > > > > > completely revised commit message, comments and warning print
> >> > > > > > use pr_warn instead of pr_warn_once
> >> > > > > > added Reviewed-by Greg
> >> > > > > > removed cc:stable@ and fixes tag, since the problematic commit
> >> > > > > > was reverted
> >> > > > > > ---
> >> > > > > > drivers/mtd/parsers/ofpart_core.c | 19 +++
> >> > > > > > 1 file changed, 19 insertions(+)
> >> > > > > >
> >> > > > > > diff --git a/drivers/mtd/parsers/ofpart_core.c
> >> > > > > > b/drivers/mtd/parsers/ofpart_core.c
> >> > > > > > index 192190c42fc8..e7b8e9d0a910 100644
> >> > > > > > --- a/drivers/mtd/parsers/ofpart_core.c
> >> > > > > > +++ b/drivers/mtd/parsers/ofpart_core.c
> >> > > > > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct
> >> > > > > > mtd_info *master,
> >> > > > > >
> >> > > > > > a_cells = of_n_addr_cells(pp);
> >> > > > > > s_cells = of_n_size_cells(pp);
> >> > > > > > +if (!dedicated && s_cells == 0) {
> >> > > > > > +/*
> >> > > > > > + * This is a ugly workaround to not
> >> > > > > > create
> >> > > > > > + * regression on devices that are still
> >> > > > > > creating
> >> > > > > > + * partitions as direct children of the
> >> > > > > > nand controller.
> >> > > > > > + * This can happen in case the nand
> >> > > > > > controller node has
> >> > > > > > + * #size-cells equal to 0 and the
> >> > > > > > firmware (e.g.
> >> > > > > > + * U-Boot) just add the partitions
> >> > > > > > there assuming
> >> > > > > > + * 32-bit addressing.
> >> > > > > > + *
> >> > > > > > + * If you get this warning your
> >> > > > > > firmware and/or DTS
> >> > > > > > + * should be really fixed.
> >> > > > > > + *
> >> > > > > > + * This is working only for devices
> >> > > > > > smaller than 4GiB.
> >> > > > > > + */
> >> > > > > > +pr_warn("%s: ofpart partition %pOF
> >> > > > > > (%pOF) #size-cells is wrongly set to <0>, assuming <1> for
> >> > > > > > parsing partitions.\n",
> >> > > > > > +master->name, pp, mtd_node);
> >> > > > > >
> >> > > > >
> >> >
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
Il 3 febbraio 2023 16:12:02 CET, Miquel Raynal ha
scritto:
>Hi Francesco,
>
>france...@dolcini.it wrote on Thu, 2 Feb 2023 12:33:34 +0100:
>
>> On Thu, Jan 26, 2023 at 10:12:04AM +0100, Miquel Raynal wrote:
>> > gre...@linuxfoundation.org wrote on Thu, 26 Jan 2023 10:01:02 +0100:
>> >
>> > > On Wed, Jan 25, 2023 at 10:06:57PM +0100, Francesco Dolcini wrote:
>> > > > Hello Miquel, Greg and all
>> > > >
>> > > > On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
>> > > > > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
>> > > > >
>> > > > > > From: Francesco Dolcini
>> > > > > >
>> > > > > > Add a mechanism to handle the case in which partitions are present
>> > > > > > as
>> > > > > > direct child of the nand controller node and #size-cells is set to
>> > > > > > <0>.
>> > > > > >
>> > > > > > This could happen if the nand-controller node in the DTS is
>> > > > > > supposed to
>> > > > > > have #size-cells set to 0, but for some historical reason/bug it
>> > > > > > was set
>> > > > > > to 1 in the past, and the firmware (e.g. U-Boot) is adding the
>> > > > > > partition
>> > > > > > as direct children of the nand-controller defaulting to #size-cells
>> > > > > > being to 1.
>> > > > > >
>> > > > > > This prevents a real boot failure on colibri-imx7 that happened
>> > > > > > during v6.1
>> > > > > > development cycles.
>> > > > > >
>> > > > > > Link:
>> > > > > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
>> > > > > > Link:
>> > > > > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
>> > > > > > Signed-off-by: Francesco Dolcini
>> > > > > > Reviewed-by: Greg Kroah-Hartman
>> > > > > > ---
>> > > > > > I do not expect this patch to be backported to stable, however I
>> > > > > > would expect
>> > > > > > that we do not backport nand-controller dts cleanups neither.
>> > > > > >
>> > > > > > v4:
>> > > > > > fixed wrong English spelling in the comment
>> > > > > >
>> > > > > > v3:
>> > > > > > minor formatting change, removed not needed new-line and space.
>> > > > > >
>> > > > > > v2:
>> > > > > > fixup size-cells only when partitions are direct children of the
>> > > > > > nand-controller
>> > > > > > completely revised commit message, comments and warning print
>> > > > > > use pr_warn instead of pr_warn_once
>> > > > > > added Reviewed-by Greg
>> > > > > > removed cc:stable@ and fixes tag, since the problematic commit
>> > > > > > was reverted
>> > > > > > ---
>> > > > > > drivers/mtd/parsers/ofpart_core.c | 19 +++
>> > > > > > 1 file changed, 19 insertions(+)
>> > > > > >
>> > > > > > diff --git a/drivers/mtd/parsers/ofpart_core.c
>> > > > > > b/drivers/mtd/parsers/ofpart_core.c
>> > > > > > index 192190c42fc8..e7b8e9d0a910 100644
>> > > > > > --- a/drivers/mtd/parsers/ofpart_core.c
>> > > > > > +++ b/drivers/mtd/parsers/ofpart_core.c
>> > > > > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct
>> > > > > > mtd_info *master,
>> > > > > >
>> > > > > >a_cells = of_n_addr_cells(pp);
>> > > > > >s_cells = of_n_size_cells(pp);
>> > > > > > + if (!dedicated && s_cells == 0) {
>> > > > > > + /*
>> > > > > > + * This is a ugly workaround to not create
>> > > > > > + * regression on devices that are still creating
>> > > > > > + * partitions as direct children of the nand
>> > > > > > controller.
>> > > > > > + * This can happen in case the nand controller
>> > > > > > node has
>> > > > > > + * #size-cells equal to 0 and the firmware (e.g.
>> > > > > > + * U-Boot) just add the partitions there
>> > > > > > assuming
>> > > > > > + * 32-bit addressing.
>> > > > > > + *
>> > > > > > + * If you get this warning your firmware and/or
>> > > > > > DTS
>> > > > > > + * should be really fixed.
>> > > > > > + *
>> > > > > > + * This is working only for devices smaller
>> > > > > > than 4GiB.
>> > > > > > + */
>> > > > > > + pr_warn("%s: ofpart partition %pOF (%pOF)
>> > > > > > #size-cells is wrongly set to <0>, assuming <1> for parsing
>> > > > > > partitions.\n",
>> > > > > > + master->name, pp, mtd_node);
>> > > > >
>> > > > > This is a driver, always use dev_*() calls, not pr_*() calls so that
>> > > > > we
>> > > > > know what is being referred to exactly.
>> > > >
>> > > > Is this reasonable here? Where can I get the struct device?
>> > >
>> > > Walk back up the call chain, there has to be a device somewhere
>> > > controlling this, right?
>> > >
>> > > > In general this file uses only pr_* debug API and messages are about OF
>> > > > nodes/properties, not about a device.
>> > >
>> > >
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
Hi Francesco,
france...@dolcini.it wrote on Thu, 2 Feb 2023 12:33:34 +0100:
> On Thu, Jan 26, 2023 at 10:12:04AM +0100, Miquel Raynal wrote:
> > gre...@linuxfoundation.org wrote on Thu, 26 Jan 2023 10:01:02 +0100:
> >
> > > On Wed, Jan 25, 2023 at 10:06:57PM +0100, Francesco Dolcini wrote:
> > > > Hello Miquel, Greg and all
> > > >
> > > > On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
> > > > > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
> > > > > > From: Francesco Dolcini
> > > > > >
> > > > > > Add a mechanism to handle the case in which partitions are present
> > > > > > as
> > > > > > direct child of the nand controller node and #size-cells is set to
> > > > > > <0>.
> > > > > >
> > > > > > This could happen if the nand-controller node in the DTS is
> > > > > > supposed to
> > > > > > have #size-cells set to 0, but for some historical reason/bug it
> > > > > > was set
> > > > > > to 1 in the past, and the firmware (e.g. U-Boot) is adding the
> > > > > > partition
> > > > > > as direct children of the nand-controller defaulting to #size-cells
> > > > > > being to 1.
> > > > > >
> > > > > > This prevents a real boot failure on colibri-imx7 that happened
> > > > > > during v6.1
> > > > > > development cycles.
> > > > > >
> > > > > > Link:
> > > > > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> > > > > > Link:
> > > > > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> > > > > > Signed-off-by: Francesco Dolcini
> > > > > > Reviewed-by: Greg Kroah-Hartman
> > > > > > ---
> > > > > > I do not expect this patch to be backported to stable, however I
> > > > > > would expect
> > > > > > that we do not backport nand-controller dts cleanups neither.
> > > > > >
> > > > > > v4:
> > > > > > fixed wrong English spelling in the comment
> > > > > >
> > > > > > v3:
> > > > > > minor formatting change, removed not needed new-line and space.
> > > > > >
> > > > > > v2:
> > > > > > fixup size-cells only when partitions are direct children of the
> > > > > > nand-controller
> > > > > > completely revised commit message, comments and warning print
> > > > > > use pr_warn instead of pr_warn_once
> > > > > > added Reviewed-by Greg
> > > > > > removed cc:stable@ and fixes tag, since the problematic commit was
> > > > > > reverted
> > > > > > ---
> > > > > > drivers/mtd/parsers/ofpart_core.c | 19 +++
> > > > > > 1 file changed, 19 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/mtd/parsers/ofpart_core.c
> > > > > > b/drivers/mtd/parsers/ofpart_core.c
> > > > > > index 192190c42fc8..e7b8e9d0a910 100644
> > > > > > --- a/drivers/mtd/parsers/ofpart_core.c
> > > > > > +++ b/drivers/mtd/parsers/ofpart_core.c
> > > > > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct
> > > > > > mtd_info *master,
> > > > > >
> > > > > > a_cells = of_n_addr_cells(pp);
> > > > > > s_cells = of_n_size_cells(pp);
> > > > > > + if (!dedicated && s_cells == 0) {
> > > > > > + /*
> > > > > > +* This is a ugly workaround to not create
> > > > > > +* regression on devices that are still creating
> > > > > > +* partitions as direct children of the nand
> > > > > > controller.
> > > > > > +* This can happen in case the nand controller
> > > > > > node has
> > > > > > +* #size-cells equal to 0 and the firmware (e.g.
> > > > > > +* U-Boot) just add the partitions there
> > > > > > assuming
> > > > > > +* 32-bit addressing.
> > > > > > +*
> > > > > > +* If you get this warning your firmware and/or
> > > > > > DTS
> > > > > > +* should be really fixed.
> > > > > > +*
> > > > > > +* This is working only for devices smaller
> > > > > > than 4GiB.
> > > > > > +*/
> > > > > > + pr_warn("%s: ofpart partition %pOF (%pOF)
> > > > > > #size-cells is wrongly set to <0>, assuming <1> for parsing
> > > > > > partitions.\n",
> > > > > > + master->name, pp, mtd_node);
> > > > >
> > > > > This is a driver, always use dev_*() calls, not pr_*() calls so that
> > > > > we
> > > > > know what is being referred to exactly.
> > > >
> > > > Is this reasonable here? Where can I get the struct device?
> > >
> > > Walk back up the call chain, there has to be a device somewhere
> > > controlling this, right?
> > >
> > > > In general this file uses only pr_* debug API and messages are about OF
> > > > nodes/properties, not about a device.
> > >
> > > OF nodes and properties are part of a device's properties :)
> >
> > Yes but the warning comes from a wrong DT description, hence it felt
> > better suited to
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
On Thu, Jan 26, 2023 at 10:12:04AM +0100, Miquel Raynal wrote:
> gre...@linuxfoundation.org wrote on Thu, 26 Jan 2023 10:01:02 +0100:
>
> > On Wed, Jan 25, 2023 at 10:06:57PM +0100, Francesco Dolcini wrote:
> > > Hello Miquel, Greg and all
> > >
> > > On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
> > > > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
> > > > > From: Francesco Dolcini
> > > > >
> > > > > Add a mechanism to handle the case in which partitions are present as
> > > > > direct child of the nand controller node and #size-cells is set to
> > > > > <0>.
> > > > >
> > > > > This could happen if the nand-controller node in the DTS is supposed
> > > > > to
> > > > > have #size-cells set to 0, but for some historical reason/bug it was
> > > > > set
> > > > > to 1 in the past, and the firmware (e.g. U-Boot) is adding the
> > > > > partition
> > > > > as direct children of the nand-controller defaulting to #size-cells
> > > > > being to 1.
> > > > >
> > > > > This prevents a real boot failure on colibri-imx7 that happened
> > > > > during v6.1
> > > > > development cycles.
> > > > >
> > > > > Link:
> > > > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> > > > > Link:
> > > > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> > > > > Signed-off-by: Francesco Dolcini
> > > > > Reviewed-by: Greg Kroah-Hartman
> > > > > ---
> > > > > I do not expect this patch to be backported to stable, however I
> > > > > would expect
> > > > > that we do not backport nand-controller dts cleanups neither.
> > > > >
> > > > > v4:
> > > > > fixed wrong English spelling in the comment
> > > > >
> > > > > v3:
> > > > > minor formatting change, removed not needed new-line and space.
> > > > >
> > > > > v2:
> > > > > fixup size-cells only when partitions are direct children of the
> > > > > nand-controller
> > > > > completely revised commit message, comments and warning print
> > > > > use pr_warn instead of pr_warn_once
> > > > > added Reviewed-by Greg
> > > > > removed cc:stable@ and fixes tag, since the problematic commit was
> > > > > reverted
> > > > > ---
> > > > > drivers/mtd/parsers/ofpart_core.c | 19 +++
> > > > > 1 file changed, 19 insertions(+)
> > > > >
> > > > > diff --git a/drivers/mtd/parsers/ofpart_core.c
> > > > > b/drivers/mtd/parsers/ofpart_core.c
> > > > > index 192190c42fc8..e7b8e9d0a910 100644
> > > > > --- a/drivers/mtd/parsers/ofpart_core.c
> > > > > +++ b/drivers/mtd/parsers/ofpart_core.c
> > > > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct
> > > > > mtd_info *master,
> > > > >
> > > > > a_cells = of_n_addr_cells(pp);
> > > > > s_cells = of_n_size_cells(pp);
> > > > > + if (!dedicated && s_cells == 0) {
> > > > > + /*
> > > > > + * This is a ugly workaround to not create
> > > > > + * regression on devices that are still creating
> > > > > + * partitions as direct children of the nand
> > > > > controller.
> > > > > + * This can happen in case the nand controller
> > > > > node has
> > > > > + * #size-cells equal to 0 and the firmware (e.g.
> > > > > + * U-Boot) just add the partitions there
> > > > > assuming
> > > > > + * 32-bit addressing.
> > > > > + *
> > > > > + * If you get this warning your firmware and/or
> > > > > DTS
> > > > > + * should be really fixed.
> > > > > + *
> > > > > + * This is working only for devices smaller
> > > > > than 4GiB.
> > > > > + */
> > > > > + pr_warn("%s: ofpart partition %pOF (%pOF)
> > > > > #size-cells is wrongly set to <0>, assuming <1> for parsing
> > > > > partitions.\n",
> > > > > + master->name, pp, mtd_node);
> > > >
> > > > This is a driver, always use dev_*() calls, not pr_*() calls so that we
> > > > know what is being referred to exactly.
> > >
> > > Is this reasonable here? Where can I get the struct device?
> >
> > Walk back up the call chain, there has to be a device somewhere
> > controlling this, right?
> >
> > > In general this file uses only pr_* debug API and messages are about OF
> > > nodes/properties, not about a device.
> >
> > OF nodes and properties are part of a device's properties :)
>
> Yes but the warning comes from a wrong DT description, hence it felt
> better suited to warn against the node name which is easily identifiable
> in a text file and must be fixed rather than the device which is a pure
> software component.
>
> Anyway, Francesco, please show us the resultant line and if it feels
> meaningful enough we'll take the dev_warn approach.
So, I tried, but I
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
Hi Greg,
gre...@linuxfoundation.org wrote on Thu, 26 Jan 2023 10:01:02 +0100:
> On Wed, Jan 25, 2023 at 10:06:57PM +0100, Francesco Dolcini wrote:
> > Hello Miquel, Greg and all
> >
> > On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
> > > > From: Francesco Dolcini
> > > >
> > > > Add a mechanism to handle the case in which partitions are present as
> > > > direct child of the nand controller node and #size-cells is set to <0>.
> > > >
> > > > This could happen if the nand-controller node in the DTS is supposed to
> > > > have #size-cells set to 0, but for some historical reason/bug it was set
> > > > to 1 in the past, and the firmware (e.g. U-Boot) is adding the partition
> > > > as direct children of the nand-controller defaulting to #size-cells
> > > > being to 1.
> > > >
> > > > This prevents a real boot failure on colibri-imx7 that happened during
> > > > v6.1
> > > > development cycles.
> > > >
> > > > Link:
> > > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> > > > Link:
> > > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> > > > Signed-off-by: Francesco Dolcini
> > > > Reviewed-by: Greg Kroah-Hartman
> > > > ---
> > > > I do not expect this patch to be backported to stable, however I would
> > > > expect
> > > > that we do not backport nand-controller dts cleanups neither.
> > > >
> > > > v4:
> > > > fixed wrong English spelling in the comment
> > > >
> > > > v3:
> > > > minor formatting change, removed not needed new-line and space.
> > > >
> > > > v2:
> > > > fixup size-cells only when partitions are direct children of the
> > > > nand-controller
> > > > completely revised commit message, comments and warning print
> > > > use pr_warn instead of pr_warn_once
> > > > added Reviewed-by Greg
> > > > removed cc:stable@ and fixes tag, since the problematic commit was
> > > > reverted
> > > > ---
> > > > drivers/mtd/parsers/ofpart_core.c | 19 +++
> > > > 1 file changed, 19 insertions(+)
> > > >
> > > > diff --git a/drivers/mtd/parsers/ofpart_core.c
> > > > b/drivers/mtd/parsers/ofpart_core.c
> > > > index 192190c42fc8..e7b8e9d0a910 100644
> > > > --- a/drivers/mtd/parsers/ofpart_core.c
> > > > +++ b/drivers/mtd/parsers/ofpart_core.c
> > > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct mtd_info
> > > > *master,
> > > >
> > > > a_cells = of_n_addr_cells(pp);
> > > > s_cells = of_n_size_cells(pp);
> > > > + if (!dedicated && s_cells == 0) {
> > > > + /*
> > > > +* This is a ugly workaround to not create
> > > > +* regression on devices that are still creating
> > > > +* partitions as direct children of the nand
> > > > controller.
> > > > +* This can happen in case the nand controller
> > > > node has
> > > > +* #size-cells equal to 0 and the firmware (e.g.
> > > > +* U-Boot) just add the partitions there
> > > > assuming
> > > > +* 32-bit addressing.
> > > > +*
> > > > +* If you get this warning your firmware and/or
> > > > DTS
> > > > +* should be really fixed.
> > > > +*
> > > > +* This is working only for devices smaller
> > > > than 4GiB.
> > > > +*/
> > > > + pr_warn("%s: ofpart partition %pOF (%pOF)
> > > > #size-cells is wrongly set to <0>, assuming <1> for parsing
> > > > partitions.\n",
> > > > + master->name, pp, mtd_node);
> > >
> > > This is a driver, always use dev_*() calls, not pr_*() calls so that we
> > > know what is being referred to exactly.
> >
> > Is this reasonable here? Where can I get the struct device?
>
> Walk back up the call chain, there has to be a device somewhere
> controlling this, right?
>
> > In general this file uses only pr_* debug API and messages are about OF
> > nodes/properties, not about a device.
>
> OF nodes and properties are part of a device's properties :)
Yes but the warning comes from a wrong DT description, hence it felt
better suited to warn against the node name which is easily identifiable
in a text file and must be fixed rather than the device which is a pure
software component.
Anyway, Francesco, please show us the resultant line and if it feels
meaningful enough we'll take the dev_warn approach.
Thanks,
Miquèl
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
On Wed, Jan 25, 2023 at 10:06:57PM +0100, Francesco Dolcini wrote:
> Hello Miquel, Greg and all
>
> On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
> > > From: Francesco Dolcini
> > >
> > > Add a mechanism to handle the case in which partitions are present as
> > > direct child of the nand controller node and #size-cells is set to <0>.
> > >
> > > This could happen if the nand-controller node in the DTS is supposed to
> > > have #size-cells set to 0, but for some historical reason/bug it was set
> > > to 1 in the past, and the firmware (e.g. U-Boot) is adding the partition
> > > as direct children of the nand-controller defaulting to #size-cells
> > > being to 1.
> > >
> > > This prevents a real boot failure on colibri-imx7 that happened during
> > > v6.1
> > > development cycles.
> > >
> > > Link:
> > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> > > Link:
> > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> > > Signed-off-by: Francesco Dolcini
> > > Reviewed-by: Greg Kroah-Hartman
> > > ---
> > > I do not expect this patch to be backported to stable, however I would
> > > expect
> > > that we do not backport nand-controller dts cleanups neither.
> > >
> > > v4:
> > > fixed wrong English spelling in the comment
> > >
> > > v3:
> > > minor formatting change, removed not needed new-line and space.
> > >
> > > v2:
> > > fixup size-cells only when partitions are direct children of the
> > > nand-controller
> > > completely revised commit message, comments and warning print
> > > use pr_warn instead of pr_warn_once
> > > added Reviewed-by Greg
> > > removed cc:stable@ and fixes tag, since the problematic commit was
> > > reverted
> > > ---
> > > drivers/mtd/parsers/ofpart_core.c | 19 +++
> > > 1 file changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/mtd/parsers/ofpart_core.c
> > > b/drivers/mtd/parsers/ofpart_core.c
> > > index 192190c42fc8..e7b8e9d0a910 100644
> > > --- a/drivers/mtd/parsers/ofpart_core.c
> > > +++ b/drivers/mtd/parsers/ofpart_core.c
> > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct mtd_info
> > > *master,
> > >
> > > a_cells = of_n_addr_cells(pp);
> > > s_cells = of_n_size_cells(pp);
> > > + if (!dedicated && s_cells == 0) {
> > > + /*
> > > + * This is a ugly workaround to not create
> > > + * regression on devices that are still creating
> > > + * partitions as direct children of the nand controller.
> > > + * This can happen in case the nand controller node has
> > > + * #size-cells equal to 0 and the firmware (e.g.
> > > + * U-Boot) just add the partitions there assuming
> > > + * 32-bit addressing.
> > > + *
> > > + * If you get this warning your firmware and/or DTS
> > > + * should be really fixed.
> > > + *
> > > + * This is working only for devices smaller than 4GiB.
> > > + */
> > > + pr_warn("%s: ofpart partition %pOF (%pOF) #size-cells
> > > is wrongly set to <0>, assuming <1> for parsing partitions.\n",
> > > + master->name, pp, mtd_node);
> >
> > This is a driver, always use dev_*() calls, not pr_*() calls so that we
> > know what is being referred to exactly.
>
> Is this reasonable here? Where can I get the struct device?
Walk back up the call chain, there has to be a device somewhere
controlling this, right?
> In general this file uses only pr_* debug API and messages are about OF
> nodes/properties, not about a device.
OF nodes and properties are part of a device's properties :)
thanks,
greg k-h
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
Hi Greg,
france...@dolcini.it wrote on Wed, 25 Jan 2023 22:06:57 +0100:
> Hello Miquel, Greg and all
>
> On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
> > > From: Francesco Dolcini
> > >
> > > Add a mechanism to handle the case in which partitions are present as
> > > direct child of the nand controller node and #size-cells is set to <0>.
> > >
> > > This could happen if the nand-controller node in the DTS is supposed to
> > > have #size-cells set to 0, but for some historical reason/bug it was set
> > > to 1 in the past, and the firmware (e.g. U-Boot) is adding the partition
> > > as direct children of the nand-controller defaulting to #size-cells
> > > being to 1.
> > >
> > > This prevents a real boot failure on colibri-imx7 that happened during
> > > v6.1
> > > development cycles.
> > >
> > > Link:
> > > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> > > Link:
> > > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> > > Signed-off-by: Francesco Dolcini
> > > Reviewed-by: Greg Kroah-Hartman
> > > ---
> > > I do not expect this patch to be backported to stable, however I would
> > > expect
> > > that we do not backport nand-controller dts cleanups neither.
> > >
> > > v4:
> > > fixed wrong English spelling in the comment
> > >
> > > v3:
> > > minor formatting change, removed not needed new-line and space.
> > >
> > > v2:
> > > fixup size-cells only when partitions are direct children of the
> > > nand-controller
> > > completely revised commit message, comments and warning print
> > > use pr_warn instead of pr_warn_once
> > > added Reviewed-by Greg
> > > removed cc:stable@ and fixes tag, since the problematic commit was
> > > reverted
> > > ---
> > > drivers/mtd/parsers/ofpart_core.c | 19 +++
> > > 1 file changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/mtd/parsers/ofpart_core.c
> > > b/drivers/mtd/parsers/ofpart_core.c
> > > index 192190c42fc8..e7b8e9d0a910 100644
> > > --- a/drivers/mtd/parsers/ofpart_core.c
> > > +++ b/drivers/mtd/parsers/ofpart_core.c
> > > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct mtd_info
> > > *master,
> > >
> > > a_cells = of_n_addr_cells(pp);
> > > s_cells = of_n_size_cells(pp);
> > > + if (!dedicated && s_cells == 0) {
> > > + /*
> > > + * This is a ugly workaround to not create
> > > + * regression on devices that are still creating
> > > + * partitions as direct children of the nand controller.
> > > + * This can happen in case the nand controller node has
> > > + * #size-cells equal to 0 and the firmware (e.g.
> > > + * U-Boot) just add the partitions there assuming
> > > + * 32-bit addressing.
> > > + *
> > > + * If you get this warning your firmware and/or DTS
> > > + * should be really fixed.
> > > + *
> > > + * This is working only for devices smaller than 4GiB.
> > > + */
> > > + pr_warn("%s: ofpart partition %pOF (%pOF) #size-cells
> > > is wrongly set to <0>, assuming <1> for parsing partitions.\n",
> > > + master->name, pp, mtd_node);
> >
> > This is a driver, always use dev_*() calls, not pr_*() calls so that we
> > know what is being referred to exactly.
>
> Is this reasonable here? Where can I get the struct device?
>
> In general this file uses only pr_* debug API and messages are about OF
> nodes/properties, not about a device.
I'm also skeptical here, this is not a device driver, it's a generic
parser and it seems more appropriate to warn about an of node rather
than a struct device.
MTD devices inherit from struct device (mtd->dev) which I guess
might be used here. The bus infrastructure device
(mtd->device->parents) is less appropriate as it sometimes points at the
controller (raw NAND) and sometimes at the spi device (SPI-NAND, SPI
NOR).
pr_warn is fine here IMHO, but if Greg insist, switch it to dev_warn, I
don't mind. Maybe it is worth testing that dev_warn still displays an
easy-to-understand message in that case.
> > I take back my "reviewed-by" line above, please fix this up to not need
> > pr_warn, but to use dev_warn() instead.
>
> Francesco
Thanks,
Miquèl
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
Hello Miquel, Greg and all
On Tue, Jan 24, 2023 at 04:38:59PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
> > From: Francesco Dolcini
> >
> > Add a mechanism to handle the case in which partitions are present as
> > direct child of the nand controller node and #size-cells is set to <0>.
> >
> > This could happen if the nand-controller node in the DTS is supposed to
> > have #size-cells set to 0, but for some historical reason/bug it was set
> > to 1 in the past, and the firmware (e.g. U-Boot) is adding the partition
> > as direct children of the nand-controller defaulting to #size-cells
> > being to 1.
> >
> > This prevents a real boot failure on colibri-imx7 that happened during v6.1
> > development cycles.
> >
> > Link:
> > https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> > Link:
> > https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> > Signed-off-by: Francesco Dolcini
> > Reviewed-by: Greg Kroah-Hartman
> > ---
> > I do not expect this patch to be backported to stable, however I would
> > expect
> > that we do not backport nand-controller dts cleanups neither.
> >
> > v4:
> > fixed wrong English spelling in the comment
> >
> > v3:
> > minor formatting change, removed not needed new-line and space.
> >
> > v2:
> > fixup size-cells only when partitions are direct children of the
> > nand-controller
> > completely revised commit message, comments and warning print
> > use pr_warn instead of pr_warn_once
> > added Reviewed-by Greg
> > removed cc:stable@ and fixes tag, since the problematic commit was reverted
> > ---
> > drivers/mtd/parsers/ofpart_core.c | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/mtd/parsers/ofpart_core.c
> > b/drivers/mtd/parsers/ofpart_core.c
> > index 192190c42fc8..e7b8e9d0a910 100644
> > --- a/drivers/mtd/parsers/ofpart_core.c
> > +++ b/drivers/mtd/parsers/ofpart_core.c
> > @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct mtd_info
> > *master,
> >
> > a_cells = of_n_addr_cells(pp);
> > s_cells = of_n_size_cells(pp);
> > + if (!dedicated && s_cells == 0) {
> > + /*
> > +* This is a ugly workaround to not create
> > +* regression on devices that are still creating
> > +* partitions as direct children of the nand controller.
> > +* This can happen in case the nand controller node has
> > +* #size-cells equal to 0 and the firmware (e.g.
> > +* U-Boot) just add the partitions there assuming
> > +* 32-bit addressing.
> > +*
> > +* If you get this warning your firmware and/or DTS
> > +* should be really fixed.
> > +*
> > +* This is working only for devices smaller than 4GiB.
> > +*/
> > + pr_warn("%s: ofpart partition %pOF (%pOF) #size-cells
> > is wrongly set to <0>, assuming <1> for parsing partitions.\n",
> > + master->name, pp, mtd_node);
>
> This is a driver, always use dev_*() calls, not pr_*() calls so that we
> know what is being referred to exactly.
Is this reasonable here? Where can I get the struct device?
In general this file uses only pr_* debug API and messages are about OF
nodes/properties, not about a device.
> I take back my "reviewed-by" line above, please fix this up to not need
> pr_warn, but to use dev_warn() instead.
Francesco
Re: [PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
On Tue, Jan 24, 2023 at 11:44:44AM +0100, Francesco Dolcini wrote:
> From: Francesco Dolcini
>
> Add a mechanism to handle the case in which partitions are present as
> direct child of the nand controller node and #size-cells is set to <0>.
>
> This could happen if the nand-controller node in the DTS is supposed to
> have #size-cells set to 0, but for some historical reason/bug it was set
> to 1 in the past, and the firmware (e.g. U-Boot) is adding the partition
> as direct children of the nand-controller defaulting to #size-cells
> being to 1.
>
> This prevents a real boot failure on colibri-imx7 that happened during v6.1
> development cycles.
>
> Link:
> https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
> Link:
> https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
> Signed-off-by: Francesco Dolcini
> Reviewed-by: Greg Kroah-Hartman
> ---
> I do not expect this patch to be backported to stable, however I would expect
> that we do not backport nand-controller dts cleanups neither.
>
> v4:
> fixed wrong English spelling in the comment
>
> v3:
> minor formatting change, removed not needed new-line and space.
>
> v2:
> fixup size-cells only when partitions are direct children of the
> nand-controller
> completely revised commit message, comments and warning print
> use pr_warn instead of pr_warn_once
> added Reviewed-by Greg
> removed cc:stable@ and fixes tag, since the problematic commit was reverted
> ---
> drivers/mtd/parsers/ofpart_core.c | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/mtd/parsers/ofpart_core.c
> b/drivers/mtd/parsers/ofpart_core.c
> index 192190c42fc8..e7b8e9d0a910 100644
> --- a/drivers/mtd/parsers/ofpart_core.c
> +++ b/drivers/mtd/parsers/ofpart_core.c
> @@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct mtd_info
> *master,
>
> a_cells = of_n_addr_cells(pp);
> s_cells = of_n_size_cells(pp);
> + if (!dedicated && s_cells == 0) {
> + /*
> + * This is a ugly workaround to not create
> + * regression on devices that are still creating
> + * partitions as direct children of the nand controller.
> + * This can happen in case the nand controller node has
> + * #size-cells equal to 0 and the firmware (e.g.
> + * U-Boot) just add the partitions there assuming
> + * 32-bit addressing.
> + *
> + * If you get this warning your firmware and/or DTS
> + * should be really fixed.
> + *
> + * This is working only for devices smaller than 4GiB.
> + */
> + pr_warn("%s: ofpart partition %pOF (%pOF) #size-cells
> is wrongly set to <0>, assuming <1> for parsing partitions.\n",
> + master->name, pp, mtd_node);
This is a driver, always use dev_*() calls, not pr_*() calls so that we
know what is being referred to exactly.
I take back my "reviewed-by" line above, please fix this up to not need
pr_warn, but to use dev_warn() instead.
thanks,
greg k-h
[PATCH v4] mtd: parsers: ofpart: add workaround for #size-cells 0
From: Francesco Dolcini
Add a mechanism to handle the case in which partitions are present as
direct child of the nand controller node and #size-cells is set to <0>.
This could happen if the nand-controller node in the DTS is supposed to
have #size-cells set to 0, but for some historical reason/bug it was set
to 1 in the past, and the firmware (e.g. U-Boot) is adding the partition
as direct children of the nand-controller defaulting to #size-cells
being to 1.
This prevents a real boot failure on colibri-imx7 that happened during v6.1
development cycles.
Link: https://lore.kernel.org/all/y4dgbtgnwpm6s...@francesco-nb.int.toradex.com/
Link: https://lore.kernel.org/all/20221202071900.1143950-1-france...@dolcini.it/
Signed-off-by: Francesco Dolcini
Reviewed-by: Greg Kroah-Hartman
---
I do not expect this patch to be backported to stable, however I would expect
that we do not backport nand-controller dts cleanups neither.
v4:
fixed wrong English spelling in the comment
v3:
minor formatting change, removed not needed new-line and space.
v2:
fixup size-cells only when partitions are direct children of the
nand-controller
completely revised commit message, comments and warning print
use pr_warn instead of pr_warn_once
added Reviewed-by Greg
removed cc:stable@ and fixes tag, since the problematic commit was reverted
---
drivers/mtd/parsers/ofpart_core.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/mtd/parsers/ofpart_core.c
b/drivers/mtd/parsers/ofpart_core.c
index 192190c42fc8..e7b8e9d0a910 100644
--- a/drivers/mtd/parsers/ofpart_core.c
+++ b/drivers/mtd/parsers/ofpart_core.c
@@ -122,6 +122,25 @@ static int parse_fixed_partitions(struct mtd_info *master,
a_cells = of_n_addr_cells(pp);
s_cells = of_n_size_cells(pp);
+ if (!dedicated && s_cells == 0) {
+ /*
+* This is a ugly workaround to not create
+* regression on devices that are still creating
+* partitions as direct children of the nand controller.
+* This can happen in case the nand controller node has
+* #size-cells equal to 0 and the firmware (e.g.
+* U-Boot) just add the partitions there assuming
+* 32-bit addressing.
+*
+* If you get this warning your firmware and/or DTS
+* should be really fixed.
+*
+* This is working only for devices smaller than 4GiB.
+*/
+ pr_warn("%s: ofpart partition %pOF (%pOF) #size-cells
is wrongly set to <0>, assuming <1> for parsing partitions.\n",
+ master->name, pp, mtd_node);
+ s_cells = 1;
+ }
if (len / 4 != a_cells + s_cells) {
pr_debug("%s: ofpart partition %pOF (%pOF) error
parsing reg property.\n",
master->name, pp,
--
2.25.1
