Thread (13 messages) 13 messages, 4 authors, 2026-02-02

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

From: Vladimir Oltean <olteanv@gmail.com>
Date: 2026-02-02 12:57:27
Also in: linux-devicetree, lkml

On Mon, Feb 02, 2026 at 10:49:47AM +0000, Daniel Golle wrote:
quoted
quoted
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.
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help