Re: fwnode_for_each_child_node() and OF backend discrepancy

From: Michael Walle
Date: Thu Jun 30 2022 - 17:32:31 EST


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