Re: [net-next,v11,4/4] net: dsa: add basic initial driver for MxL862xx switches

[Vladimir Oltean]

On Mon, Feb 02, 2026 at 10:49:47AM +0000, Daniel Golle wrote:
> > > The dsa_to_port() function can return NULL if the port is not found in
> > > the switch tree. The code stores the result in dp but doesn't check if
> > > dp is NULL before dereferencing dp->cpu_dp->index.
> > > 
> > > Looking at include/net/dsa.h, dsa_to_port() returns NULL when the port
> > > is not found:
> > > 
> > >     static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
> > >     {
> > >         struct dsa_switch_tree *dst = ds->dst;
> > >         struct dsa_port *dp;
> > > 
> > >         list_for_each_entry(dp, &dst->ports, list)
> > >             if (dp->ds == ds && dp->index == p)
> > >                 return dp;
> > > 
> > >         return NULL;
> > >     }
> > > 
> > > Additionally, even if dp is non-NULL, dp->cpu_dp could also be NULL if
> > > the CPU port hasn't been properly assigned during initialization.
> > 
> > mxl862xx_add_single_port_bridge() has been called when all other port
> > types except user ports have been excluded. All user and DSA ports have
> > a non-NULL dp->cpu_dp pointer after dsa_tree_setup_cpu_ports() runs,
> > i.e. also at the time of ds->ops->port_setup().
> 
> here, as well as in mxl862xx_setup_cpu_bridge(), right?

Yeah. If you think adding a comment helps keep spirits calm, you can also do that.

> > Technically ds->ops->setup() runs under dsa2_mutex, but the "static int idx"
> > is still not ideal due to the ever-increasing index upon unbinding and
> > rebinding.
> 
> As mentioned in my reply to Jakub[1] many drivers follow this pattern and it would
> imho be a good idea to agree on a convention or even provide a helper function
> which names the MII bus for DSA drivers. What do you think?
> 
> [1]: https://patchwork.kernel.org/comment/26768088/

I don't really have an opinion, the internal MDIO bus is part of each
driver's housekeeping, and DSA tries to stay out of that as much as possible.

Maybe PHY maintainers may know more what user space tooling may break if
the mii_bus->id gets changed, or may prefer a naming convention irrespective
of the bus being part of a DSA switch or not.