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

[Marek Vasut]

On 12/2/22 17:57, Miquel Raynal wrote:
> Hi Marek,

Hi,

>> 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 at 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 at N {} .
> 
> Actually #address-cells is here for that. You need to point at one CS,
> so in most cases this is:
> 
> controller {
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	chip at N {
> 		reg = <N>;
> 	};
> };

Right ... thanks for spotting this.

But then the size-cells in the controller node should be zero. And 
753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells") is 
therefore correct ?

But that correction in 753395ea1e45 breaks setup where old U-Boot 
injects partition at N directly into the nand-controller node, without 
updating the nand-controller node size-cells, i.e. this:
nand-controller {
	#address-cells = <1>;
	#size-cells = <0>;

+       partition at 0 { ... };
};
Because the size-cells is 0 in that case, and U-Boot does not update the 
size-cells .

It used to work before because Linux DT contained size-cells=<1> in the 
nand-controller node before 753395ea1e45 ("ARM: dts: imx7: Fix NAND 
controller size-cells").

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.