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

[Vladimir Oltean <olteanv@gmail.com>]

On Wed, Jan 14, 2026 at 11:15:40PM +0000, Daniel Golle wrote:
> On Thu, Jan 15, 2026 at 12:57:36AM +0200, Vladimir Oltean wrote:
> > On Mon, Jan 12, 2026 at 01:52:52PM +0000, Daniel Golle wrote:
> > > Add very basic DSA driver for MaxLinear's MxL862xx switches.
> > > 
> > > In contrast to previous MaxLinear switches the MxL862xx has a built-in
> > > processor that runs a sophisticated firmware based on Zephyr RTOS.
> > > Interaction between the host and the switch hence is organized using a
> > > software API of that firmware rather than accessing hardware registers
> > > directly.
> > > 
> > > Add descriptions of the most basic firmware API calls to access the
> > > built-in MDIO bus hosting the 2.5GE PHYs, basic port control as well as
> > > setting up the CPU port.
> > > 
> > > Implement a very basic DSA driver using that API which is sufficient to
> > > get packets flowing between the user ports and the CPU port.
> > > 
> > > The firmware offers all features one would expect from a modern switch
> > > hardware, they will be added one by one in follow-up patch series.
> > > 
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > ---
> > > v5:
> > >  * output warning in .setup regarding unknown pre-configuration
> > >  * add comment explaining why CFGGET is used in reset function
> > > 
> > > RFC v4:
> > >  * poll switch readiness after reset
> > >  * implement driver shutdown
> > >  * added port_fast_aging API call and driver op
> > >  * unified port setup in new .port_setup op
> > >  * improve comment explaining special handlign for unaligned API read
> > >  * various typos
> > > 
> > > RFC v3:
> > >  * fix return value being uninitialized on error in mxl862xx_api_wrap()
> > >  * add missing descrition in kerneldoc comment of
> > >    struct mxl862xx_ss_sp_tag
> > > 
> > > RFC v2:
> > >  * make use of struct mdio_device
> > >  * add phylink_mac_ops stubs
> > >  * drop leftover nonsense from mxl862xx_phylink_get_caps()
> > >  * use __le32 instead of enum types in over-the-wire structs
> > >  * use existing MDIO_* macros whenever possible
> > >  * simplify API constants to be more readable
> > >  * use readx_poll_timeout instead of open-coding poll timeout loop
> > >  * add mxl862xx_reg_read() and mxl862xx_reg_write() helpers
> > >  * demystify error codes returned by the firmware
> > >  * add #defines for mxl862xx_ss_sp_tag member values
> > >  * move reset to dedicated function, clarify magic number being the
> > >    reset command ID
> > > 
> > >  MAINTAINERS                              |   1 +
> > >  drivers/net/dsa/Kconfig                  |   2 +
> > >  drivers/net/dsa/Makefile                 |   1 +
> > >  drivers/net/dsa/mxl862xx/Kconfig         |  12 +
> > >  drivers/net/dsa/mxl862xx/Makefile        |   3 +
> > >  drivers/net/dsa/mxl862xx/mxl862xx-api.h  | 177 +++++++++
> > >  drivers/net/dsa/mxl862xx/mxl862xx-cmd.h  |  32 ++
> > >  drivers/net/dsa/mxl862xx/mxl862xx-host.c | 230 ++++++++++++
> > >  drivers/net/dsa/mxl862xx/mxl862xx-host.h |   5 +
> > >  drivers/net/dsa/mxl862xx/mxl862xx.c      | 433 +++++++++++++++++++++++
> > >  drivers/net/dsa/mxl862xx/mxl862xx.h      |  24 ++
> > >  11 files changed, 920 insertions(+)
> > >  create mode 100644 drivers/net/dsa/mxl862xx/Kconfig
> > >  create mode 100644 drivers/net/dsa/mxl862xx/Makefile
> > >  create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-api.h
> > >  create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-cmd.h
> > >  create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-host.c
> > >  create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-host.h
> > >  create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx.c
> > >  create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx.h
> > > 
> > > +static int mxl862xx_setup(struct dsa_switch *ds)
> > > +{
> > > +	struct mxl862xx_priv *priv = ds->priv;
> > > +	int ret;
> > > +
> > > +	ret = mxl862xx_reset(priv);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = mxl862xx_wait_ready(ds);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = mxl862xx_setup_mdio(ds);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	dev_warn(ds->dev, "Unknown switch pre-configuration, ports may be bridged!\n");
> > 
> > Nack. User space needs to be in control of the forwarding domain of the
> > ports, and isolating user ports is the bare minimum requirement,
> > otherwise you cannot even connect the ports of this device to a switch
> > without creating L2 loops.
> > 
> > It seems that it is too early for this switch to be supported by
> > mainline. Maybe in staging...
> 
> In order to avoid the detour via staging, from my perspective there are two
> ways to go from here:
> 
> a) Keep nagging MaxLinear to provide a switch firmware with an additional
> firmware command which flushes the pre-configuration and puts the switch
> in a well-defined state (all ports isolated, learning disabled) for DSA.
> 
> b) Extend the patch to cover all the API calls needed to do this
> manually (more than double of LoC).
> 
> Obviously a) would be better for me and you, but MaxLinear indicated they
> prefer not to release an new firmware adding that feature at this point.
> 
> b) would allow me to proceed right away, but it would burden reviewers
> with a rather huge patch for initial support for this switch.
> For the sake of making review more easy I'd prefer to still keep this
> in a series of not terribly huge patches rather than a single patch
> which immediately brings in everything (ie. have bridge and bridgeport
> configuration in one patch, FDB access in the next, ...). Would a
> series adding everything needed to end up with isolated ports be
> acceptable?
> 
> Please let me know what you think.

Do you have the additional work required to isolate user ports prepared
on some branch that can be pre-reviewed? I'm not sure that I have all
information to make an informed comment.