Re: [PATCH v1] mtd: parsers: ofpart: Fix parsing when size-cells is 0

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Marek,

marex@denx.de wrote on Fri, 16 Dec 2022 15:32:28 +0100:

> On 12/16/22 14:37, Miquel Raynal wrote:
> 
> Hi,
> 
> [...]
> 
> >>> 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.  
> > 
> > I really don't understand the reasoning here.
> > 
> > What I say is: let's fix the boards known to be incorrectly described
> > when we break them so they continue working with a broken firmware.  
> 
> The second part of the message, as far as I understand it, is "ignore problems this will cause to users of boards we do not know about, let them run into unbootable systems after some linux kernel update, 

Now you know what kernel update will break them, so you can prevent it
from happening. 

For boards without even a dtsi in the kernel, should we care?

> and once they suffer through system recovery, make them add compatible
> string to the arch-side workaround".
> 
> > What regression could this possibly bring? I don't care about catching
> > the 2k boards out there which work but wrongly describe their
> > partitions. If they work, they will continue working.  
> 
> Those boards would start failing once the Linux-side DT size-cells is corrected.
> 
> Also, this got missed in the previous discussion. If you use only board compatible string in arch-side workaround, the workaround would be applied even on systems with updated bootloaders, which is likely not what we want.

If the heuristics here needs to be improved somehow, let's discuss that ;)

> > You and Marek say: let's blindly always change a property in the DT, no
> > matter if the board is broken, even if we don't know if this is the
> > right thing to do, and apply this to the entire world.  
> 
> As far as I can tell, if we have partitions in the NAND controller node and size-cells=0, then the right thing to do is to override size-cells to 1 , because partitions with size-cells=0 make no sense.

How do you know the firmware did not set #size-cell=0 on purpose to
avoid using the partitions? How would this be more stupid than
updating partitions without setting #size-cell to the right value? Can
you be so sure every board in the world will never do that? And how do
you handle the 64-bit case as well? You _do_no_know_ what applies in
all the cases, guessing is dangerous here, you'll just support new
broken cases or even break existing setups! What you do know however is
what applies for a number of cases your clearly identified. Applying
this logic to *all* cases in simply _broken_ and utterly stupid (tm). I
don't know how to express that so we stop bike-shedding.

> If the heuristics here needs to be improved somehow, let's discuss that.
> 
> > But with this approach you're not worried about regressions.
> > 
> > I am sorry it does not stand.  
> 
> [...]

Thanks,
Miquèl

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