| Message ID | E1rs1Rp-005g7S-3j@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: allow phylink_mac_ops in DSA drivers | expand |
On Wed, Apr 03, 2024 at 03:18:41PM +0100, Russell King (Oracle) wrote: > diff --git a/net/dsa/port.c b/net/dsa/port.c > index 02bf1c306bdc..4cafbc505009 100644 > --- a/net/dsa/port.c > +++ b/net/dsa/port.c > @@ -1662,6 +1662,7 @@ static const struct phylink_mac_ops dsa_port_phylink_mac_ops = { > > int dsa_port_phylink_create(struct dsa_port *dp) > { > + const struct phylink_mac_ops *mac_ops; > struct dsa_switch *ds = dp->ds; > phy_interface_t mode; > struct phylink *pl; > @@ -1685,8 +1686,12 @@ int dsa_port_phylink_create(struct dsa_port *dp) > } > } > > - pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), > - mode, &dsa_port_phylink_mac_ops); > + mac_ops = &dsa_port_phylink_mac_ops; > + if (ds->phylink_mac_ops) > + mac_ops = ds->phylink_mac_ops; > + > + pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), mode, > + mac_ops); > if (IS_ERR(pl)) { > pr_err("error creating PHYLINK: %ld\n", PTR_ERR(pl)); > return PTR_ERR(pl); > -- > 2.30.2 > This is not sufficient. We will have to make DSA call the driver through the mac_ops it provides, rather than through ds->ops, here: dsa_shared_port_link_register_of() if (!ds->ops->adjust_link) { if (missing_link_description) { dev_warn(ds->dev, "Skipping phylink registration for %s port %d\n", dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index); } else { if (ds->ops->phylink_mac_link_down) ds->ops->phylink_mac_link_down(ds, port, MLO_AN_FIXED, PHY_INTERFACE_MODE_NA); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ return dsa_shared_port_phylink_register(dp); } return 0; } Coincidentally mv88e6xxx is exactly one of those drivers which needs the early mac_link_down() call that isn't driven by phylink.
On Fri, Apr 05, 2024 at 07:21:00PM +0300, Vladimir Oltean wrote: > On Wed, Apr 03, 2024 at 03:18:41PM +0100, Russell King (Oracle) wrote: > > diff --git a/net/dsa/port.c b/net/dsa/port.c > > index 02bf1c306bdc..4cafbc505009 100644 > > --- a/net/dsa/port.c > > +++ b/net/dsa/port.c > > @@ -1662,6 +1662,7 @@ static const struct phylink_mac_ops dsa_port_phylink_mac_ops = { > > > > int dsa_port_phylink_create(struct dsa_port *dp) > > { > > + const struct phylink_mac_ops *mac_ops; > > struct dsa_switch *ds = dp->ds; > > phy_interface_t mode; > > struct phylink *pl; > > @@ -1685,8 +1686,12 @@ int dsa_port_phylink_create(struct dsa_port *dp) > > } > > } > > > > - pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), > > - mode, &dsa_port_phylink_mac_ops); > > + mac_ops = &dsa_port_phylink_mac_ops; > > + if (ds->phylink_mac_ops) > > + mac_ops = ds->phylink_mac_ops; > > + > > + pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), mode, > > + mac_ops); > > if (IS_ERR(pl)) { > > pr_err("error creating PHYLINK: %ld\n", PTR_ERR(pl)); > > return PTR_ERR(pl); > > -- > > 2.30.2 > > > > This is not sufficient. We will have to make DSA call the driver through > the mac_ops it provides, rather than through ds->ops, here: > > dsa_shared_port_link_register_of() > > if (!ds->ops->adjust_link) { > if (missing_link_description) { > dev_warn(ds->dev, > "Skipping phylink registration for %s port %d\n", > dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index); > } else { > if (ds->ops->phylink_mac_link_down) > ds->ops->phylink_mac_link_down(ds, port, > MLO_AN_FIXED, PHY_INTERFACE_MODE_NA); > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > return dsa_shared_port_phylink_register(dp); > } > return 0; > } > > Coincidentally mv88e6xxx is exactly one of those drivers which needs the > early mac_link_down() call that isn't driven by phylink. Thanks for the review, I'd forgotten that this path exists!
diff --git a/include/net/dsa.h b/include/net/dsa.h index f228b479a5fd..7edfd8de8882 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -457,6 +457,11 @@ struct dsa_switch { */ const struct dsa_switch_ops *ops; + /* + * Allow a DSA switch driver to override the phylink MAC ops + */ + const struct phylink_mac_ops *phylink_mac_ops; + /* * User mii_bus and devices for the individual ports. */ diff --git a/net/dsa/port.c b/net/dsa/port.c index 02bf1c306bdc..4cafbc505009 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1662,6 +1662,7 @@ static const struct phylink_mac_ops dsa_port_phylink_mac_ops = { int dsa_port_phylink_create(struct dsa_port *dp) { + const struct phylink_mac_ops *mac_ops; struct dsa_switch *ds = dp->ds; phy_interface_t mode; struct phylink *pl; @@ -1685,8 +1686,12 @@ int dsa_port_phylink_create(struct dsa_port *dp) } } - pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), - mode, &dsa_port_phylink_mac_ops); + mac_ops = &dsa_port_phylink_mac_ops; + if (ds->phylink_mac_ops) + mac_ops = ds->phylink_mac_ops; + + pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), mode, + mac_ops); if (IS_ERR(pl)) { pr_err("error creating PHYLINK: %ld\n", PTR_ERR(pl)); return PTR_ERR(pl);
Rather than having a shim for each and every phylink MAC operation, allow DSA switch drivers to provide their own ops structure. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- include/net/dsa.h | 5 +++++ net/dsa/port.c | 9 +++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)