Re: fwnode_for_each_child_node() and OF backend discrepancy

Michael Walle
2022-06-30T21:32:30Z

Am 2022-06-30 23:21, schrieb Vladimir Oltean:
> On Thu, Jun 30, 2022 at 11:00:37PM +0200, Michael Walle wrote:
>> > > > It is not possible to have a defined for the MAX number of ports that
>> > > > supported by lan966x. Which is 8. And assigned that define to
>> > > > num_phys_ports instead of counting the entries in DT?
>> > >
>> > > You mean also for the lan9662? I'm pretty sure that doesn't
>> > > work. Have a look where num_phys_ports is used. One random
>> > > example:
>> > > https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/microchip/lan966x/lan966x_main.c#L874
>> > >
>> > > So if your switch only has 4 ports, then I'd guess you'll
>> > > access a non-existing register.
>> >
>> > Underneath lan662 and lan668 is the same chip. The HW people disable
>> > some ports/features on each platform but from what I know you will still
>> > be able to access the registers.
>>
>> I noticed that there are still 8 ports in the register description and
>> assumed that it was wrong [1]. But ok, that makes sense in some way.
>> OTOH that means, we cannot do the guesswork Vladimir proposed.
>>
>> -michael
>>
>> [1]
>> https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html
>
> Are you 100% positive that the default values for the flooding PGIDs
> are
> GENMASK(8, 0) for a 4-port switch? And that the packet buffer has the
> same size for a switch with half as many ports? Ok...
>
> But in that case, what exactly is the problem if the port count of 8 is
> a synthesis time constant for lan966x, and if the CPU port module is
> always at index 8 in the analyzer (with a gap between indices 4 and 7)?
> Just hardcode lan966x->num_phys_ports to 8 and work with that
> throughout.
> Allocate lan966x->ports as an array of 8 pointers to struct
> lan966x_port
> (which they are already), and the pointers themselves are populated as
> being the netdev_priv of the interfaces that are actually present and
> used. Literally the only thing you need to fix is that you need to
> hardcode num_phys_ports to 8, problem solved. It means that lan9662 is
> nothing but a lan9668 where the last 4 ports have 'status = "disabled"'
> in the device tree.

If that's the case, sure. The last four ports can just be omitted no?
Of course you'll lose the possibility to validate the device tree ports
input, i.e. port@5 would perfectly valid although it doesn't make any
sense for the lan9662. (Regardless if it is disabled, or if it's
omitted)

-michael