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

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:

quoted

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

quoted

+     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