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/2/22 17:42, Miquel Raynal wrote:
Hi Marek,

Hi,

[...]

However, it should not be empty, at the very least a reg property
should indicate on which CS it is wired, as expected there:
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/Documentation/devicetree/bindings/mtd/nand-chip.yaml?h=mtd/next

OK, I see your point. So basically this?

&gpmi {
    #size-cells = <1>;
    ...
    nand-chip@0 {
      reg = <0>;
    };
};

btw. the GPMI NAND controller supports only one chipselect, so the reg in nand-chip node makes little sense.

I randomly opened a reference manual (IMX6DQL.pdf), they say:

	"Up to four NAND devices, supported by four chip-selects and one
	 ganged ready/ busy."

Doh, and MX7D has the same controller, so size-cells = <1>; makes sense with nand-chip@N {} .

Anyway, the NAND controller generic bindings which require this reg
property, what the controller or the driver actually supports, or even
how it is used on current designs is not relevant here.

But, as nand-chip.yaml references mtd.yaml, you can as well use
whatever is described here:
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/Documentation/devicetree/bindings/mtd/mtd.yaml?h=mtd/next
What would be the gpmi controller size cells (X) in that case, still 0, right ? So how does that help solve this problem, wouldn't U-Boot still populate the partitions directly under the gpmi node or into partitions sub-node ?

The commit that was pointed in the original fix clearly stated that the
NAND chip node was targeted

I think this is another miscommunication here. The commit

753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")

modifies the size-cells of the NAND controller. The nand-chip is not involved in this at all . In the examples above, it's the "&gpmi" node size-cells that is modified.

Yes I know. I was referring to this commit, sorry:
36fee2f7621e ("common: fdt_support: add support for "partitions" subnode to fdt_fixup_mtdparts()")

The log says:

	Listing MTD partitions directly in the flash mode has been
	deprecated for a while for kernel Device Trees. Look for a node "partitions" in the
	found flash nodes and use it instead of the flash node itself for the
	partition list when it exists, so Device Trees following the current
	best practices can be fixed up.

Which (I hope) means U-boot will equivalently try to play with the
partitions container, either in the controller node or in the chip node.

, not the NAND controller node. I hope this
is correctly supported in U-Boot though. So if there is a NAND chip
subnode, I suppose U-Boot would try to create the partitions that are
inside, or even in the sub "partitions" container.

My understanding is that U-Boot checks the nand-controller node size-cells, not the nand-chip{} or partitions{} subnode size-cells .

I don't think U-Boot cares.

Francesco, can you please share the DT, including the U-Boot generated partitions, which is passed to Linux on Colibri MX7 ? I think that should make all confusion go away.

Please also do it with the NAND chip described. If, when the NAND chip
is described U-Boot tries to create partitions in the controller node,
then the situation is even worse than I thought. But I believe
describing the node like a suggest in the DT should prevent the boot
failure while still allowing a rather good description of the hardware.

BTW I still think the relevant action right now is to revert the DT
patch.

I am starting to bank toward that variant as well (thanks for clarifying the rationale in the discussion, that helped a lot).

But then, the follow up fix would be what exactly, update the binding document to require #size-cells = <1>; ?



[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