| Message ID | E1sCxVx-00EwVY-E2@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: remove obsolete phylink dsa_switch operations | expand |
On Fri, May 31, 2024 at 09:21:29AM +0100, Russell King (Oracle) wrote: > No driver now uses the DSA switch phylink members, so we can now remove > the shim functions and method pointers. Arrange to print an error > message and fail registration if a DSA driver does not provide the > phylink MAC operations structure. > > Signed-off-by: Russell King (oracle) <rmk+kernel@armlinux.org.uk> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Fri, May 31, 2024 at 09:21:29AM +0100, Russell King (Oracle) wrote: > No driver now uses the DSA switch phylink members, so we can now remove > the shim functions and method pointers. Arrange to print an error > message and fail registration if a DSA driver does not provide the > phylink MAC operations structure. > > Signed-off-by: Russell King (oracle) <rmk+kernel@armlinux.org.uk> > --- > include/net/dsa.h | 15 ---------- > net/dsa/dsa.c | 9 ++---- > net/dsa/port.c | 74 +---------------------------------------------- > 3 files changed, 4 insertions(+), 94 deletions(-) > > diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c > index 668c729946ea..ceeadb52d1cc 100644 > --- a/net/dsa/dsa.c > +++ b/net/dsa/dsa.c > @@ -1505,12 +1505,9 @@ static int dsa_switch_probe(struct dsa_switch *ds) > if (!ds->num_ports) > return -EINVAL; > > - if (ds->phylink_mac_ops) { > - if (ds->ops->phylink_mac_select_pcs || > - ds->ops->phylink_mac_config || > - ds->ops->phylink_mac_link_down || > - ds->ops->phylink_mac_link_up) > - return -EINVAL; > + if (!ds->phylink_mac_ops) { > + dev_err(ds->dev, "DSA switch driver does not provide phylink MAC operations"); > + return -EINVAL; > } > > if (np) { > diff --git a/net/dsa/port.c b/net/dsa/port.c > index e23db9507546..a31a5517a12f 100644 > --- a/net/dsa/port.c > +++ b/net/dsa/port.c > @@ -1625,12 +1560,8 @@ int dsa_port_phylink_create(struct dsa_port *dp) > } > } > > - 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); > + ds->phylink_mac_ops); > if (IS_ERR(pl)) { > pr_err("error creating PHYLINK: %ld\n", PTR_ERR(pl)); > return PTR_ERR(pl); Nack. The highlighted portions break dsa_loop, hellcreek and mv88e6060 [1], which are currently trivially integrated with phylink through the default dsa_port_phylink_mac_ops, but have no implementations of ds->ops->phylink_mac_* of their own. What I'm trying to point out is that we are not at the stage yet where we can enforce all drivers to populate ds->phylink_mac_ops. > @@ -1831,9 +1762,6 @@ static void dsa_shared_port_link_down(struct dsa_port *dp) > if (ds->phylink_mac_ops && ds->phylink_mac_ops->mac_link_down) Mostly irrelevant, but I'll point out an issue with the patch logic's consistency anyway: there is no need to check "if (ds->phylink_mac_ops)" more than once. The earlier probe failure is sufficient (although, as mentioned, breaking for 3 drivers). > ds->phylink_mac_ops->mac_link_down(&dp->pl_config, MLO_AN_FIXED, > PHY_INTERFACE_MODE_NA); > - else if (ds->ops->phylink_mac_link_down) > - ds->ops->phylink_mac_link_down(ds, dp->index, MLO_AN_FIXED, > - PHY_INTERFACE_MODE_NA); > } > > int dsa_shared_port_link_register_of(struct dsa_port *dp) > -- > 2.30.2 > [1] Quick way to check: compare the outputs of these 2 commands, and see which drivers from the first category are absent from the second: $ grep -r dsa_register_switch drivers/net/dsa/ | cut -d: -f1 | sort | uniq $ grep -r phylink_mac_ops drivers/net/dsa/ | cut -d: -f1 | sort | uniq
diff --git a/include/net/dsa.h b/include/net/dsa.h index f9ae3ca66b6f..d64bbd1f9f29 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -879,21 +879,6 @@ struct dsa_switch_ops { */ void (*phylink_get_caps)(struct dsa_switch *ds, int port, struct phylink_config *config); - struct phylink_pcs *(*phylink_mac_select_pcs)(struct dsa_switch *ds, - int port, - phy_interface_t iface); - void (*phylink_mac_config)(struct dsa_switch *ds, int port, - unsigned int mode, - const struct phylink_link_state *state); - void (*phylink_mac_link_down)(struct dsa_switch *ds, int port, - unsigned int mode, - phy_interface_t interface); - void (*phylink_mac_link_up)(struct dsa_switch *ds, int port, - unsigned int mode, - phy_interface_t interface, - struct phy_device *phydev, - int speed, int duplex, - bool tx_pause, bool rx_pause); void (*phylink_fixed_state)(struct dsa_switch *ds, int port, struct phylink_link_state *state); /* diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c index 668c729946ea..ceeadb52d1cc 100644 --- a/net/dsa/dsa.c +++ b/net/dsa/dsa.c @@ -1505,12 +1505,9 @@ static int dsa_switch_probe(struct dsa_switch *ds) if (!ds->num_ports) return -EINVAL; - if (ds->phylink_mac_ops) { - if (ds->ops->phylink_mac_select_pcs || - ds->ops->phylink_mac_config || - ds->ops->phylink_mac_link_down || - ds->ops->phylink_mac_link_up) - return -EINVAL; + if (!ds->phylink_mac_ops) { + dev_err(ds->dev, "DSA switch driver does not provide phylink MAC operations"); + return -EINVAL; } if (np) { diff --git a/net/dsa/port.c b/net/dsa/port.c index e23db9507546..a31a5517a12f 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1535,73 +1535,8 @@ void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp, cpu_dp->tag_ops = tag_ops; } -static struct phylink_pcs * -dsa_port_phylink_mac_select_pcs(struct phylink_config *config, - phy_interface_t interface) -{ - struct dsa_port *dp = dsa_phylink_to_port(config); - struct phylink_pcs *pcs = ERR_PTR(-EOPNOTSUPP); - struct dsa_switch *ds = dp->ds; - - if (ds->ops->phylink_mac_select_pcs) - pcs = ds->ops->phylink_mac_select_pcs(ds, dp->index, interface); - - return pcs; -} - -static void dsa_port_phylink_mac_config(struct phylink_config *config, - unsigned int mode, - const struct phylink_link_state *state) -{ - struct dsa_port *dp = dsa_phylink_to_port(config); - struct dsa_switch *ds = dp->ds; - - if (!ds->ops->phylink_mac_config) - return; - - ds->ops->phylink_mac_config(ds, dp->index, mode, state); -} - -static void dsa_port_phylink_mac_link_down(struct phylink_config *config, - unsigned int mode, - phy_interface_t interface) -{ - struct dsa_port *dp = dsa_phylink_to_port(config); - struct dsa_switch *ds = dp->ds; - - if (!ds->ops->phylink_mac_link_down) - return; - - ds->ops->phylink_mac_link_down(ds, dp->index, mode, interface); -} - -static void dsa_port_phylink_mac_link_up(struct phylink_config *config, - struct phy_device *phydev, - unsigned int mode, - phy_interface_t interface, - int speed, int duplex, - bool tx_pause, bool rx_pause) -{ - struct dsa_port *dp = dsa_phylink_to_port(config); - struct dsa_switch *ds = dp->ds; - - if (!ds->ops->phylink_mac_link_up) - return; - - ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev, - speed, duplex, tx_pause, rx_pause); -} - -static const struct phylink_mac_ops dsa_port_phylink_mac_ops = { - .mac_select_pcs = dsa_port_phylink_mac_select_pcs, - .mac_config = dsa_port_phylink_mac_config, - .mac_link_down = dsa_port_phylink_mac_link_down, - .mac_link_up = dsa_port_phylink_mac_link_up, -}; - 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; @@ -1625,12 +1560,8 @@ int dsa_port_phylink_create(struct dsa_port *dp) } } - 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); + ds->phylink_mac_ops); if (IS_ERR(pl)) { pr_err("error creating PHYLINK: %ld\n", PTR_ERR(pl)); return PTR_ERR(pl); @@ -1831,9 +1762,6 @@ static void dsa_shared_port_link_down(struct dsa_port *dp) if (ds->phylink_mac_ops && ds->phylink_mac_ops->mac_link_down) ds->phylink_mac_ops->mac_link_down(&dp->pl_config, MLO_AN_FIXED, PHY_INTERFACE_MODE_NA); - else if (ds->ops->phylink_mac_link_down) - ds->ops->phylink_mac_link_down(ds, dp->index, MLO_AN_FIXED, - PHY_INTERFACE_MODE_NA); } int dsa_shared_port_link_register_of(struct dsa_port *dp)
No driver now uses the DSA switch phylink members, so we can now remove the shim functions and method pointers. Arrange to print an error message and fail registration if a DSA driver does not provide the phylink MAC operations structure. Signed-off-by: Russell King (oracle) <rmk+kernel@armlinux.org.uk> --- include/net/dsa.h | 15 ---------- net/dsa/dsa.c | 9 ++---- net/dsa/port.c | 74 +---------------------------------------------- 3 files changed, 4 insertions(+), 94 deletions(-)