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: Fri, 2 Dec 2022 16:00:30 +0100	[thread overview]
Message-ID: <20221202160030.1b8d0b8a@xps-13> (raw)
In-Reply-To: <2b6fc52d-60b9-d0f4-ab91-4cf7a8095999@denx.de>

Hi Marek,

marex@denx.de wrote on Fri, 2 Dec 2022 15:31:40 +0100:

> On 12/2/22 15:05, Miquel Raynal wrote:
> > Hi Francesco,  
> 
> Hi,
> 
> [...]
> 
> > I still strongly disagree with the initial proposal but what I think we
> > can do is:
> > 
> > 1. To prevent future breakages:
> >    Fix fdt_fixup_mtdparts() in u-boot. This way newer U-Boot + any
> >    kernel should work.
> > 
> > 2. To help tracking down situations like that:
> >    Keep the warning in ofpart.c but continue to fail.
> > 
> > 3. To fix the current situation:
> >     Immediately revert commit (and prevent it from being backported):
> >     753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
> >     This way your own boot flow is fixed in the short term.  
> 
> Here I disagree, the fix is correct and I think we shouldn't
> proliferate incorrect DTs which don't match the binding document.

I agree we should not proliferate incorrect DTs, so let's use a modern
description then, with a controller and a child node which defines the
chip.

> Rather, if a bootloader generates incorrect (new) DT entries, I
> believe the driver should implement a fixup and warn user about this.
> PC does that as well with broken ACPI tables as far as I can tell.
> 
> I'm not convinced making a DT non-compliant with bindings again,

I am sorry to say so, but while warnings reported by the tools
should be fixed, it's not because the tool does not scream at you that
the description is valid. We are actively working on enhancing the
schema so that "all" improper descriptions get warnings (see the series
pointed earlier), but in no way this change makes the node compliant
with modern bindings.

I'm not saying the fix is wrong, but let's be pragmatic, it currently
leads to boot failures.

> only to work around a problem induced by bootloader, is the right approach
> here.

When a patch breaks a board and there is no straight fix, you revert
it, then you think harder. That's what I am saying. This is a temporary
solution.

> This would be setting a dangerous example, where anyone could request a DT fix to be reverted because their random bootloader does the wrong thing and with valid DT clean up, something broke.

Please, you know this is not valid DT clean up. We've been decoupling
controller and chip description since 2016. What I am proposing is a
valid DT cleanup, not to the latest standard, but way closer than the
current solution.

> > 4. There is no reason to partially fix a DT like what the above did
> >     besides trying to avoid warnings emitted by the DT check tools.  
> 
> Note that the 3. does not partially fix the DT, it fixes the node fully.
> 
> >     If
> >     complying with modern bindings is a goal (and I think it should
> >     be), then we can modernize this DT without breaking the boot flow:
> >     Instead of only setting #size-cell = <0>, you can as well define
> >     in your DT a subnode to define the NAND chip. NAND chips are not
> >     supposed to have #size-cells properties, but in the past they did,
> >     which means #address-cells and #size-cells are allowed (and marked
> >     deprecated in the schema). So in practice, the dt-schema will not
> >     warn you if they are there, which means you can still set
> >     #size-cell = <1>.  
> 
> I am really not convinced we should hack around this on the DT end and try to push some sort of convoluted workaround there,

"convoluted workaround" :-)

> instead of fixing it on the driver side (and bootloader side, in the
> long run).
>
> >     Please mind, the tools have been updated very recently to match

s/tools/schema/

> >     what I am describing above, so they will likely still report
> >     errors until v6.2-rc1, see:
> >     https://lore.kernel.org/linux-mtd/20221114090315.848208-1-miquel.raynal@bootlin.com/
> > 
> > Does this sound reasonable?  
> 
> [...]


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-02 15:01 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 [this message]
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
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=20221202160030.1b8d0b8a@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).