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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/5/22 14:49, Miquel Raynal wrote:
Hi Francesco,

Hi,

francesco@xxxxxxxxxx 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 ?



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux