linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Francesco Dolcini <francesco@dolcini.it>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Marek Vasut <marex@denx.de>,
	Francesco Dolcini <francesco@dolcini.it>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	linux-mtd@lists.infradead.org,
	Francesco Dolcini <francesco.dolcini@toradex.com>,
	Shawn Guo <shawnguo@kernel.org>,
	linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org,
	u-boot@lists.denx.de
Subject: Re: [PATCH v1] mtd: parsers: ofpart: Fix parsing when size-cells is 0
Date: Fri, 16 Dec 2022 13:37:31 +0100	[thread overview]
Message-ID: <Y5xmi62hR6JeYUt1@francesco-nb.int.toradex.com> (raw)
In-Reply-To: <20221216120155.4b78e5cf@xps-13>

On Fri, Dec 16, 2022 at 12:01:55PM +0100, Miquel Raynal wrote:
> marex@denx.de wrote on Fri, 16 Dec 2022 11:46:18 +0100:
> > On 12/16/22 08:45, Francesco Dolcini wrote:
> > > On Thu, Dec 15, 2022 at 08:16:04AM +0100, Miquel Raynal wrote:  
> > >> I am still against piggy hacks in the generic ofpart.c driver, but
> > >> what we could do however is a DT fixup in the init_machine (or the
> > >> dt_fixup) hook for imx7 Colibri, very much like this:
> > >> https://elixir.bootlin.com/linux/latest/source/arch/arm/mach-mvebu/board-v7.c#L111
> > >> Plus a warning there saying "your dt is broken, update your firmware".  
> > > 
> > > I have a couple of concerns/question with this approach:
> > >   - do we have a single point to handle this? Different architectures are
> > >     affected by these issue. Duplicating the fixup code in multiple place
> > >     does not seems a great idea
> > >   - If we believe that the device tree is wrong, in the i.MX7 case
> > >     because of #size-cells should be set to 0 and not 1, we should not
> > >     alter the FDT. Other part of the code could rely on this being
> > >     correctly set to 0 moving forward.
> > > 
> > > If I understood you are proposing to have a fixup at the machine level
> > > that is converting a valid nand-controller node definition to a "broken"
> > > one. Unless I misunderstood you and you are thinking about rewriting the
> > > whole MTD partition from a broken definition to a proper one.
> 
> No, quite the opposite.
>
> Either size-cell is wrong which makes the description totally
> inconsistent (if size-cell is there, it must have a use, otherwise why
> do we keep it?) and we must fix it, or it is right and we should not
> touch it.
> 
> What I propose is to check very early whether the description is
> consistent on the board known to have this problem. If the description
> is wrong, we fix it and the generic parser can then do its work
> properly.

What if we add `nand-chip{}` children in the future (the i.MX nand
controller has nothing implemented not described in the schema so far,
but it is something that is supported by the hw)? Will this idea still
works?

> > > On Thu, Dec 15, 2022 at 09:04:46AM +0100, Miquel Raynal wrote:  
> > >> marex@denx.de wrote on Thu, 15 Dec 2022 08:45:33 +0100:  
> > >>> Sadly, it does only fix the known cases, not the unknown cases like
> > >>> downstream forks which never get any bootloader updates ever, and
> > >>> which you can't find in upstream U-Boot, and which you therefore
> > >>> cannot easily catch in the arch side fixup.  
> > >>
> > >> And ?  
> > > 
> > > I'm not personally and directly concerned, since the machine I care are
> > > all available upstream and known, however this is a general problem with
> > > U-Boot code being at the same time widely used on a range of embedded
> > > products and producing a broken MTD partition list.
> > > 
> > > I think we will just silently break boards and just creating a lot of
> > > issues to people. We would just introduce regression to the users, being
> > > aware of it and deliberately decide to not care and move the problem to
> > > someone else. I do not think this is a good way to go.  
> 
> What?

Let me rephrase, I was not clear enough.

> Since when my proposal is breaking boards? My proposal leads to a
> situation where:
> - If you have a board that has an inconsistent description but worked,
>   it will still work.
> - If you have a board that has a consistent description and worked, it
>   will still work.
> - If your have a board that has an inconsistent description and got
>   broken *recently* by another change (typically you "fix" the DT in
>   Linux to comply with the bindings), then you get a warning that leads
>   you on the right path, you then update your bootloader if you can,
>   but either way you add your machine compatible to the list of devices
>   which need the early fix and your boot is fixed.

This implies that we can proactively catch all the affected boards. I do
not believe this is reasonable and because of that my comment before
about creating regression to the users.

Francesco



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

  reply	other threads:[~2022-12-16 12:38 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-02  7:19 [PATCH v1] mtd: parsers: ofpart: Fix parsing when size-cells is 0 Francesco Dolcini
2022-12-02  9:14 ` Miquel Raynal
2022-12-02 10:12   ` Francesco Dolcini
2022-12-02 10:24     ` Francesco Dolcini
2022-12-02 10:53       ` Miquel Raynal
2022-12-02 11:23         ` Francesco Dolcini
2022-12-02 14:05           ` Miquel Raynal
2022-12-02 14:31             ` Marek Vasut
2022-12-02 15:00               ` Miquel Raynal
2022-12-02 15:23                 ` Marek Vasut
2022-12-02 15:49                   ` Miquel Raynal
2022-12-02 16:01                     ` Miquel Raynal
2022-12-02 16:17                     ` Marek Vasut
2022-12-02 16:42                       ` Miquel Raynal
2022-12-02 16:52                         ` Marek Vasut
2022-12-02 16:57                           ` Miquel Raynal
2022-12-02 17:08                             ` Marek Vasut
2022-12-05 11:26                               ` Francesco Dolcini
2022-12-05 13:49                                 ` Miquel Raynal
2022-12-05 16:25                                   ` Marek Vasut
2022-12-15  7:16                                     ` Miquel Raynal
2022-12-15  7:45                                       ` Marek Vasut
2022-12-15  8:04                                         ` Miquel Raynal
2022-12-16  0:36                                           ` Marek Vasut
2022-12-16  7:52                                             ` Francesco Dolcini
2022-12-16  7:45                                       ` Francesco Dolcini
2022-12-16 10:46                                         ` Marek Vasut
2022-12-16 11:01                                           ` Miquel Raynal
2022-12-16 12:37                                             ` Francesco Dolcini [this message]
2022-12-16 13:37                                               ` Miquel Raynal
2022-12-16 14:32                                                 ` Marek Vasut
2022-12-16 15:35                                                   ` Miquel Raynal
2022-12-16 16:30                                                     ` Francesco Dolcini
2023-01-02  9:40                                                       ` Miquel Raynal
2023-01-05 11:33                                                         ` Miquel Raynal
2023-01-05 12:47                                                           ` Francesco Dolcini
2023-01-05 14:51                                                             ` Marek Vasut
2023-01-05 15:03                                                               ` Miquel Raynal
2022-12-02 17:20                         ` Francesco Dolcini
2022-12-05 11:30                           ` Francesco Dolcini
2022-12-05 15:28                             ` Miquel Raynal
2022-12-02 16:45                       ` Francesco Dolcini
2022-12-02 17:05                         ` Miquel Raynal
2022-12-02 15:56               ` Thorsten Leemhuis
2022-12-04 12:50                 ` Marek Vasut
2022-12-04 12:59                   ` Thorsten Leemhuis
2022-12-04 15:50                     ` Marek Vasut
2022-12-02 12:43 ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y5xmi62hR6JeYUt1@francesco-nb.int.toradex.com \
    --to=francesco@dolcini.it \
    --cc=francesco.dolcini@toradex.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marex@denx.de \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=shawnguo@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=u-boot@lists.denx.de \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).