Re: [PATCH net-next v7 4/9] net: dsa: lan9645x: add basic dsa driver for LAN9645X

[Andrew Lunn <andrew@lunn.ch>]

On Tue, Jun 09, 2026 at 09:08:13AM +0200, Jens Emil Schulz Ostergaard wrote:
> On Mon, 2026-06-08 at 20:00 +0200, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > > +     dsa_switch_for_each_user_port(dp, ds) {
> > > +             if (dp->cpu_dp->ds != ds) {
> > > +                     dev_err(ds->dev,
> > > +                             "NPI port on a remote switch is not supported\n");
> > > +                     return -EINVAL;
> > > +             }
> > > +
> > > +             if (first_cpu_dp && dp->cpu_dp != first_cpu_dp) {
> > > +                     dev_err(ds->dev, "Multiple NPI ports not supported\n");
> > > +                     return -EINVAL;
> > > +             }
> > > +
> > > +             first_cpu_dp = dp->cpu_dp;
> > 
> > The reason i asked about NPI ports is because this is looping over
> > user ports. Yet you say one of these user ports is a CPU port. That
> > cannot be correct.
> > 
> > The first port returned by dsa_tree_for_each_cpu_port() would be
> > first_cpu_dp.
> > 
> >         Andrew
> 
> 
> I tried to mimic the approach in drivers/net/dsa/ocelot/felix.c:
> 
> static int felix_tag_npi_setup(struct dsa_switch *ds)
> {
> 	struct dsa_port *dp, *first_cpu_dp = NULL;
> 	struct ocelot *ocelot = ds->priv;
> 
> 	dsa_switch_for_each_user_port(dp, ds) {
> 		if (first_cpu_dp && dp->cpu_dp != first_cpu_dp) {
> 			dev_err(ds->dev, "Multiple NPI ports not supported\n");
> 			return -EINVAL;
> 		}
> 
> 		first_cpu_dp = dp->cpu_dp;
> 	}
> 
> 	if (!first_cpu_dp)
> 		return -EINVAL;
> 
> 	felix_npi_port_init(ocelot, first_cpu_dp->index);
> 
> 	return 0;
> }
> 
> Perhaps I misunderstand you, but there could be confusion about
> terminology. The chip designers have a concept of CPU port, which is
> used liberally in the datasheet, and in this driver code.

> However, the concept is different from the DSA concept of a CPU port.

And that is a problem because somebody reviewing this code is likely
to know DSA concepts much more than the individual devices concepts.
To aid overall Maintenance of all the DSA drivers, the driver should
try to keep with DSA meanings.

> Let us call the first switch CPU port, and the second DSA CPU port.
> 
> The port we want to use as a DSA CPU port, i.e. the port with the
> 'ethernet = <&host_port>;' property in the device tree, will be
> configured to be an NPI port for injection/extraction for the switch CPU
> port (which is not a physical port on the device).

> Therefore, this NPI port is not iterated by
> dsa_switch_for_each_user_port. The switch CPU port (index 9) is also not
> iterated by it. It is a chip internal construct with no representation
> in the device tree, and no struct dsa_port.

So first_cpu_dp is not a DSA CPU port. It is also not a NAPI port,
since that is a DSA CPU port. Then what is it? Why do you need the
concept of a switch CPU port?

	Andrew