Re: fwnode_for_each_child_node() and OF backend discrepancy

[Krzysztof Kozlowski]

On 28/06/2022 15:47, Michael Walle wrote:
> [adding Horatiu Vultur, because we now digress to the bug
> in the switch, rather than that odd OF behavior]
> 
> Am 2022-06-28 15:29, schrieb Andy Shevchenko:
>> On Tue, Jun 28, 2022 at 3:23 PM Michael Walle <michael@xxxxxxxx> wrote:
>>>
>>>>> I was trying to fix the lan966x driver [1] which doesn't work if there
>>>>> are disabled nodes in between.
>>>>
>>>> Can you elaborate what's wrong now in the behaviour of the driver? In
>>>> the code it uses twice the _available variant.
>>>
>>> Imagine the following device tree snippet:
>>>   port0 {
>>>     reg = <0>;
>>>     status = "okay";
>>>   }
>>>   port1 {
>>>     reg = <1>;
>>>     status = "disabled";
>>>   }
>>>   port@2 {
>>>     reg = <2>;
>>>     status = "okay";
>>>   }
>>>
>>> The driver will set num_phys_ports to 2. When port@2 is probed, it
>>> will have the (correct!) physical port number 2. That will then
>>> trigger various EINVAL checks with "port_num >= num_phys_ports" or
>>> WARN()s.
>>
>> It means the above mentioned condition is wrong: it should be
>>
>> "port_idx >= num_phys_ports" (if the port_idx doesn't exists, that's
>> the bug in the first place)
> 
> I can't follow you here. Please note, that you need the actual
> physical port number. It's not a made up number, but corresponds
> to a physical port on that ethernet switch. So you can't just skip
> the disabled ones. port@2 must have port number 2.

The number "2" you get from the reg property, so where is exactly the
problem?

If you want to validate it against some maximum number of ports, based
on DTS, it makes no sense... One can supply a DTS with port number 10000
and 10000 nodes, or with specific property "maximum-port-number=10000".
It would make sense if you validate it against driver-hard-coded values
(which you later mentioned in your reply).


Best regards,
Krzysztof