linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Marek Vasut <marex@denx.de>
Cc: 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: Thu, 15 Dec 2022 08:16:04 +0100	[thread overview]
Message-ID: <20221215081604.5385fa56@xps-13> (raw)
In-Reply-To: <ecca019d-b0b7-630c-4221-2684cb51634c@denx.de>

Hi Marek & Francesco,

marex@denx.de wrote on Mon, 5 Dec 2022 17:25:11 +0100:

> On 12/5/22 14:49, Miquel Raynal wrote:
> > Hi Francesco,  
> 
> Hi,
> 
> > francesco@dolcini.it wrote on Mon, 5 Dec 2022 12:26:44 +0100:
> >   
> >> On Fri, Dec 02, 2022 at 06:08:22PM +0100, Marek Vasut wrote:  
> >>> But here I would say this is a firmware bug and it might have to be handled
> >>> like a firmware bug, i.e. with fixup in the partition parser. I seem to be
> >>> changing my opinion here again.  
> >>
> >> I was thinking at this over the weekend, and I came to the following
> >> ideas:
> >>
> >>   - we need some improvement on the fixup we already have in the
> >>     partition parser. We cannot ignore the fdt produced by U-Boot - as
> >>     bad as it is.
> >>   - the proposed fixup is fine for the immediate need, but it is
> >>     not going to be enough to cover the general issue with the U-Boot
> >>     generated partitions. U-Boot might keep generating partitions as direct
> >>     child of the nand controller even when a partitions{} node is
> >>     available. In this case the current parser just fails since it looks
> >>     only into it and it will find it empty.
> >>   - the current U-Boot only handle partitions{} as a direct child of the
> >>     nand-controller, the nand-chip is ignored. This is not the way it is
> >>     supposed to work. U-Boot code would need to be improved.  
> > 
> > I've been thinking about it this weekend as well and the current fix
> > which "just set" s_cell to 1 seems risky for me, it is typically the
> > type of quick & dirty fix that might even break other board (nobody
> > knew that U-Boot current logic expected #size-cells to be set in the
> > DT, what if another "broken" DT expects the opposite...)  
> 
> Then with the current configuration, such broken DT would not work, since current DT does set #size-cells=<1> (wrongly).
> 
> > , not
> > mentioning potential issues with big storages (> 4GiB).
> > 
> > All in all, I really think we should revert the DT change now, reverting
> > as little to no drawbacks besides a dt_binding_check warning and gives
> > us time to deal with it properly (both in U-Boot and Linux).  
> 
> I am really not happy with this, but if that's marked as intermediate fix, go for it.
> 
> How do we deal with this in the long run however? Parser-side fix like this one, maybe with better heuristics ?

Yesterday while talking about an ACPI mis-description which needed
fixing, I realized fixing up what the firmware provides to Linux should
preferably be handled as early as possible. So my first first idea was
to avoid using the broken "fixup mtdparts" function in U-Boot and I am
still convinced this is what we should do in priority. However, as
rightly pointed in this thread, we need to take care about the case
where someone would use a newer DT (let's say, with the reverted changed
reverted again) with an old U-Boot. 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".

So next time someone stumbles upon this issue, we can tell them "fix
your bootloader", and apply the same hack in their board family (there
are three or four IIRC which might be concerned some day).

That would fix all cases and only have an impact on the affected boards.

Thanks,
Miquèl

_______________________________________________
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-15  7:17 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 [this message]
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
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=20221215081604.5385fa56@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=francesco.dolcini@toradex.com \
    --cc=francesco@dolcini.it \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marex@denx.de \
    --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).