2017-09-22 19:44:27

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v2 0/3] net: dsa: use slave device phydev

This patchset removes the private phy_device in favor of the one
provided by the slave net_device, makes slave open and close symmetrical
and finally provides helpers for enabling or disabling a DSA port.

Changes in v2:
- do not remove the phy argument from port enable/disable

Vivien Didelot (3):
net: dsa: use slave device phydev
net: dsa: make slave close symmetrical to open
net: dsa: add port enable and disable helpers

net/dsa/dsa_priv.h | 3 +-
net/dsa/port.c | 31 +++++++++++-
net/dsa/slave.c | 143 +++++++++++++++++++++++------------------------------
3 files changed, 94 insertions(+), 83 deletions(-)

--
2.14.1


2017-09-22 19:44:28

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v2 1/3] net: dsa: use slave device phydev

There is no need to store a phy_device in dsa_slave_priv since
net_device already provides one. Simply s/p->phy/dev->phydev/.

While at it, return -ENODEV when it is NULL instead of -EOPNOTSUPP.

Signed-off-by: Vivien Didelot <[email protected]>
---
net/dsa/slave.c | 126 ++++++++++++++++++++++++++------------------------------
1 file changed, 58 insertions(+), 68 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 02ace7d462c4..3760472bf41d 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -99,15 +99,15 @@ static int dsa_slave_open(struct net_device *dev)
}

if (ds->ops->port_enable) {
- err = ds->ops->port_enable(ds, p->dp->index, p->phy);
+ err = ds->ops->port_enable(ds, p->dp->index, dev->phydev);
if (err)
goto clear_promisc;
}

dsa_port_set_state_now(p->dp, stp_state);

- if (p->phy)
- phy_start(p->phy);
+ if (dev->phydev)
+ phy_start(dev->phydev);

return 0;

@@ -130,8 +130,8 @@ static int dsa_slave_close(struct net_device *dev)
struct net_device *master = dsa_master_netdev(p);
struct dsa_switch *ds = p->dp->ds;

- if (p->phy)
- phy_stop(p->phy);
+ if (dev->phydev)
+ phy_stop(dev->phydev);

dev_mc_unsync(master, dev);
dev_uc_unsync(master, dev);
@@ -144,7 +144,7 @@ static int dsa_slave_close(struct net_device *dev)
dev_uc_del(master, dev->dev_addr);

if (ds->ops->port_disable)
- ds->ops->port_disable(ds, p->dp->index, p->phy);
+ ds->ops->port_disable(ds, p->dp->index, dev->phydev);

dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);

@@ -273,12 +273,10 @@ dsa_slave_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,

static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
{
- struct dsa_slave_priv *p = netdev_priv(dev);
+ if (!dev->phydev)
+ return -ENODEV;

- if (p->phy != NULL)
- return phy_mii_ioctl(p->phy, ifr, cmd);
-
- return -EOPNOTSUPP;
+ return phy_mii_ioctl(dev->phydev, ifr, cmd);
}

static int dsa_slave_port_attr_set(struct net_device *dev,
@@ -435,12 +433,10 @@ static int
dsa_slave_get_link_ksettings(struct net_device *dev,
struct ethtool_link_ksettings *cmd)
{
- struct dsa_slave_priv *p = netdev_priv(dev);
+ if (!dev->phydev)
+ return -ENODEV;

- if (!p->phy)
- return -EOPNOTSUPP;
-
- phy_ethtool_ksettings_get(p->phy, cmd);
+ phy_ethtool_ksettings_get(dev->phydev, cmd);

return 0;
}
@@ -449,12 +445,10 @@ static int
dsa_slave_set_link_ksettings(struct net_device *dev,
const struct ethtool_link_ksettings *cmd)
{
- struct dsa_slave_priv *p = netdev_priv(dev);
+ if (!dev->phydev)
+ return -ENODEV;

- if (p->phy != NULL)
- return phy_ethtool_ksettings_set(p->phy, cmd);
-
- return -EOPNOTSUPP;
+ return phy_ethtool_ksettings_set(dev->phydev, cmd);
}

static void dsa_slave_get_drvinfo(struct net_device *dev,
@@ -488,24 +482,20 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)

static int dsa_slave_nway_reset(struct net_device *dev)
{
- struct dsa_slave_priv *p = netdev_priv(dev);
+ if (!dev->phydev)
+ return -ENODEV;

- if (p->phy != NULL)
- return genphy_restart_aneg(p->phy);
-
- return -EOPNOTSUPP;
+ return genphy_restart_aneg(dev->phydev);
}

static u32 dsa_slave_get_link(struct net_device *dev)
{
- struct dsa_slave_priv *p = netdev_priv(dev);
+ if (!dev->phydev)
+ return -ENODEV;

- if (p->phy != NULL) {
- genphy_update_link(p->phy);
- return p->phy->link;
- }
+ genphy_update_link(dev->phydev);

- return -EOPNOTSUPP;
+ return dev->phydev->link;
}

static int dsa_slave_get_eeprom_len(struct net_device *dev)
@@ -640,7 +630,7 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
int ret;

/* Port's PHY and MAC both need to be EEE capable */
- if (!p->phy)
+ if (!dev->phydev)
return -ENODEV;

if (!ds->ops->set_mac_eee)
@@ -651,12 +641,12 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
return ret;

if (e->eee_enabled) {
- ret = phy_init_eee(p->phy, 0);
+ ret = phy_init_eee(dev->phydev, 0);
if (ret)
return ret;
}

- return phy_ethtool_set_eee(p->phy, e);
+ return phy_ethtool_set_eee(dev->phydev, e);
}

static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
@@ -666,7 +656,7 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
int ret;

/* Port's PHY and MAC both need to be EEE capable */
- if (!p->phy)
+ if (!dev->phydev)
return -ENODEV;

if (!ds->ops->get_mac_eee)
@@ -676,7 +666,7 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
if (ret)
return ret;

- return phy_ethtool_get_eee(p->phy, e);
+ return phy_ethtool_get_eee(dev->phydev, e);
}

#ifdef CONFIG_NET_POLL_CONTROLLER
@@ -985,26 +975,26 @@ static void dsa_slave_adjust_link(struct net_device *dev)
struct dsa_switch *ds = p->dp->ds;
unsigned int status_changed = 0;

- if (p->old_link != p->phy->link) {
+ if (p->old_link != dev->phydev->link) {
status_changed = 1;
- p->old_link = p->phy->link;
+ p->old_link = dev->phydev->link;
}

- if (p->old_duplex != p->phy->duplex) {
+ if (p->old_duplex != dev->phydev->duplex) {
status_changed = 1;
- p->old_duplex = p->phy->duplex;
+ p->old_duplex = dev->phydev->duplex;
}

- if (p->old_pause != p->phy->pause) {
+ if (p->old_pause != dev->phydev->pause) {
status_changed = 1;
- p->old_pause = p->phy->pause;
+ p->old_pause = dev->phydev->pause;
}

if (ds->ops->adjust_link && status_changed)
- ds->ops->adjust_link(ds, p->dp->index, p->phy);
+ ds->ops->adjust_link(ds, p->dp->index, dev->phydev);

if (status_changed)
- phy_print_status(p->phy);
+ phy_print_status(dev->phydev);
}

static int dsa_slave_fixed_link_update(struct net_device *dev,
@@ -1029,17 +1019,18 @@ static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
struct dsa_slave_priv *p = netdev_priv(slave_dev);
struct dsa_switch *ds = p->dp->ds;

- p->phy = mdiobus_get_phy(ds->slave_mii_bus, addr);
- if (!p->phy) {
+ slave_dev->phydev = mdiobus_get_phy(ds->slave_mii_bus, addr);
+ if (!slave_dev->phydev) {
netdev_err(slave_dev, "no phy at %d\n", addr);
return -ENODEV;
}

/* Use already configured phy mode */
if (p->phy_interface == PHY_INTERFACE_MODE_NA)
- p->phy_interface = p->phy->interface;
- return phy_connect_direct(slave_dev, p->phy, dsa_slave_adjust_link,
- p->phy_interface);
+ p->phy_interface = slave_dev->phydev->interface;
+
+ return phy_connect_direct(slave_dev, slave_dev->phydev,
+ dsa_slave_adjust_link, p->phy_interface);
}

static int dsa_slave_phy_setup(struct net_device *slave_dev)
@@ -1091,22 +1082,23 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
return ret;
}
} else {
- p->phy = of_phy_connect(slave_dev, phy_dn,
- dsa_slave_adjust_link,
- phy_flags,
- p->phy_interface);
+ slave_dev->phydev = of_phy_connect(slave_dev, phy_dn,
+ dsa_slave_adjust_link,
+ phy_flags,
+ p->phy_interface);
}

of_node_put(phy_dn);
}

- if (p->phy && phy_is_fixed)
- fixed_phy_set_link_update(p->phy, dsa_slave_fixed_link_update);
+ if (slave_dev->phydev && phy_is_fixed)
+ fixed_phy_set_link_update(slave_dev->phydev,
+ dsa_slave_fixed_link_update);

/* We could not connect to a designated PHY, so use the switch internal
* MDIO bus instead
*/
- if (!p->phy) {
+ if (!slave_dev->phydev) {
ret = dsa_slave_phy_connect(slave_dev, p->dp->index);
if (ret) {
netdev_err(slave_dev, "failed to connect to port %d: %d\n",
@@ -1117,7 +1109,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
}
}

- phy_attached_info(p->phy);
+ phy_attached_info(slave_dev->phydev);

return 0;
}
@@ -1137,12 +1129,12 @@ int dsa_slave_suspend(struct net_device *slave_dev)

netif_device_detach(slave_dev);

- if (p->phy) {
- phy_stop(p->phy);
+ if (slave_dev->phydev) {
+ phy_stop(slave_dev->phydev);
p->old_pause = -1;
p->old_link = -1;
p->old_duplex = -1;
- phy_suspend(p->phy);
+ phy_suspend(slave_dev->phydev);
}

return 0;
@@ -1150,13 +1142,11 @@ int dsa_slave_suspend(struct net_device *slave_dev)

int dsa_slave_resume(struct net_device *slave_dev)
{
- struct dsa_slave_priv *p = netdev_priv(slave_dev);
-
netif_device_attach(slave_dev);

- if (p->phy) {
- phy_resume(p->phy);
- phy_start(p->phy);
+ if (slave_dev->phydev) {
+ phy_resume(slave_dev->phydev);
+ phy_start(slave_dev->phydev);
}

return 0;
@@ -1249,8 +1239,8 @@ void dsa_slave_destroy(struct net_device *slave_dev)
port_dn = p->dp->dn;

netif_carrier_off(slave_dev);
- if (p->phy) {
- phy_disconnect(p->phy);
+ if (slave_dev->phydev) {
+ phy_disconnect(slave_dev->phydev);

if (of_phy_is_fixed_link(port_dn))
of_phy_deregister_fixed_link(port_dn);
--
2.14.1

2017-09-22 19:45:03

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v2 2/3] net: dsa: make slave close symmetrical to open

The DSA slave open function configures the unicast MAC addresses on the
master device, enable the switch port, change its STP state, then start
the PHY device.

Make the close function symmetric, by first stopping the PHY device,
then changing the STP state, disabling the switch port and restore the
master device.

Signed-off-by: Vivien Didelot <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
net/dsa/slave.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3760472bf41d..0aab29928152 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -133,6 +133,11 @@ static int dsa_slave_close(struct net_device *dev)
if (dev->phydev)
phy_stop(dev->phydev);

+ dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
+
+ if (ds->ops->port_disable)
+ ds->ops->port_disable(ds, p->dp->index, dev->phydev);
+
dev_mc_unsync(master, dev);
dev_uc_unsync(master, dev);
if (dev->flags & IFF_ALLMULTI)
@@ -143,11 +148,6 @@ static int dsa_slave_close(struct net_device *dev)
if (!ether_addr_equal(dev->dev_addr, master->dev_addr))
dev_uc_del(master, dev->dev_addr);

- if (ds->ops->port_disable)
- ds->ops->port_disable(ds, p->dp->index, dev->phydev);
-
- dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
-
return 0;
}

--
2.14.1

2017-09-22 19:45:02

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v2 3/3] net: dsa: add port enable and disable helpers

Provide dsa_port_enable and dsa_port_disable helpers to respectively
enable and disable a switch port. This makes the dsa_port_set_state_now
helper static.

Signed-off-by: Vivien Didelot <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
net/dsa/dsa_priv.h | 3 ++-
net/dsa/port.c | 31 ++++++++++++++++++++++++++++++-
net/dsa/slave.c | 19 +++++--------------
3 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9803952a5b40..0298a0f6a349 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -117,7 +117,8 @@ void dsa_master_ethtool_restore(struct net_device *dev);
/* port.c */
int dsa_port_set_state(struct dsa_port *dp, u8 state,
struct switchdev_trans *trans);
-void dsa_port_set_state_now(struct dsa_port *dp, u8 state);
+int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy);
+void dsa_port_disable(struct dsa_port *dp, struct phy_device *phy);
int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br);
void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br);
int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 76d43a82d397..72c8dbd3d3f2 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -56,7 +56,7 @@ int dsa_port_set_state(struct dsa_port *dp, u8 state,
return 0;
}

-void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
+static void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
{
int err;

@@ -65,6 +65,35 @@ void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
pr_err("DSA: failed to set STP state %u (%d)\n", state, err);
}

+int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
+{
+ u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
+ struct dsa_switch *ds = dp->ds;
+ int port = dp->index;
+ int err;
+
+ if (ds->ops->port_enable) {
+ err = ds->ops->port_enable(ds, port, phy);
+ if (err)
+ return err;
+ }
+
+ dsa_port_set_state_now(dp, stp_state);
+
+ return 0;
+}
+
+void dsa_port_disable(struct dsa_port *dp, struct phy_device *phy)
+{
+ struct dsa_switch *ds = dp->ds;
+ int port = dp->index;
+
+ dsa_port_set_state_now(dp, BR_STATE_DISABLED);
+
+ if (ds->ops->port_disable)
+ ds->ops->port_disable(ds, port, phy);
+}
+
int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
{
struct dsa_notifier_bridge_info info = {
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 0aab29928152..4ea1c6eb0da8 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -73,9 +73,7 @@ static int dsa_slave_open(struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_port *dp = p->dp;
- struct dsa_switch *ds = dp->ds;
struct net_device *master = dsa_master_netdev(p);
- u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
int err;

if (!(master->flags & IFF_UP))
@@ -98,13 +96,9 @@ static int dsa_slave_open(struct net_device *dev)
goto clear_allmulti;
}

- if (ds->ops->port_enable) {
- err = ds->ops->port_enable(ds, p->dp->index, dev->phydev);
- if (err)
- goto clear_promisc;
- }
-
- dsa_port_set_state_now(p->dp, stp_state);
+ err = dsa_port_enable(dp, dev->phydev);
+ if (err)
+ goto clear_promisc;

if (dev->phydev)
phy_start(dev->phydev);
@@ -128,15 +122,12 @@ static int dsa_slave_close(struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct net_device *master = dsa_master_netdev(p);
- struct dsa_switch *ds = p->dp->ds;
+ struct dsa_port *dp = p->dp;

if (dev->phydev)
phy_stop(dev->phydev);

- dsa_port_set_state_now(p->dp, BR_STATE_DISABLED);
-
- if (ds->ops->port_disable)
- ds->ops->port_disable(ds, p->dp->index, dev->phydev);
+ dsa_port_disable(dp, dev->phydev);

dev_mc_unsync(master, dev);
dev_uc_unsync(master, dev);
--
2.14.1

2017-09-22 20:00:41

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev

On 09/22/2017 12:40 PM, Vivien Didelot wrote:
> There is no need to store a phy_device in dsa_slave_priv since
> net_device already provides one. Simply s/p->phy/dev->phydev/.

You can therefore remove the phy_device from dsa_slave_priv, see below
for more comments. I will have to regress test the heck out of this,
this should take a few hours.

>
> While at it, return -ENODEV when it is NULL instead of -EOPNOTSUPP.
>
> Signed-off-by: Vivien Didelot <[email protected]>
> ---

> static int dsa_slave_port_attr_set(struct net_device *dev,
> @@ -435,12 +433,10 @@ static int
> dsa_slave_get_link_ksettings(struct net_device *dev,
> struct ethtool_link_ksettings *cmd)
> {
> - struct dsa_slave_priv *p = netdev_priv(dev);
> + if (!dev->phydev)
> + return -ENODEV;
>
> - if (!p->phy)
> - return -EOPNOTSUPP;
> -
> - phy_ethtool_ksettings_get(p->phy, cmd);
> + phy_ethtool_ksettings_get(dev->phydev, cmd);

This can be replaced by phy_ethtool_get_link_ksettings()

>
> return 0;
> }
> @@ -449,12 +445,10 @@ static int
> dsa_slave_set_link_ksettings(struct net_device *dev,
> const struct ethtool_link_ksettings *cmd)
> {
> - struct dsa_slave_priv *p = netdev_priv(dev);
> + if (!dev->phydev)
> + return -ENODEV;
>
> - if (p->phy != NULL)
> - return phy_ethtool_ksettings_set(p->phy, cmd);
> -
> - return -EOPNOTSUPP;
> + return phy_ethtool_ksettings_set(dev->phydev, cmd);
> }

This can disappear and you can assign this ethtool operation to
phy_ethtool_set_link_ksettings()

>
> static void dsa_slave_get_drvinfo(struct net_device *dev,
> @@ -488,24 +482,20 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
>
> static int dsa_slave_nway_reset(struct net_device *dev)
> {
> - struct dsa_slave_priv *p = netdev_priv(dev);
> + if (!dev->phydev)
> + return -ENODEV;
>
> - if (p->phy != NULL)
> - return genphy_restart_aneg(p->phy);
> -
> - return -EOPNOTSUPP;
> + return genphy_restart_aneg(dev->phydev);
> }

This can now disappear and you can use phy_ethtool_nway_reset() directly
in ethtool_ops

>
> static u32 dsa_slave_get_link(struct net_device *dev)
> {
> - struct dsa_slave_priv *p = netdev_priv(dev);
> + if (!dev->phydev)
> + return -ENODEV;
>
> - if (p->phy != NULL) {
> - genphy_update_link(p->phy);
> - return p->phy->link;
> - }
> + genphy_update_link(dev->phydev);
>
> - return -EOPNOTSUPP;
> + return dev->phydev->link;
> }

This should certainly be just ethtool_op_get_link(), not sure why we
kept that around here...
--
Florian

2017-09-22 20:03:11

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev

On Fri, Sep 22, 2017 at 03:40:43PM -0400, Vivien Didelot wrote:
> There is no need to store a phy_device in dsa_slave_priv since
> net_device already provides one. Simply s/p->phy/dev->phydev/.
>
> While at it, return -ENODEV when it is NULL instead of -EOPNOTSUPP.

I just did a quick poll for calling phy_mii_ioctl(). ENODEV seems the
most popular, second to EINVAL. Marvell drivers all use EOPNOTSUPP.

> static int dsa_slave_nway_reset(struct net_device *dev)
> {
> - struct dsa_slave_priv *p = netdev_priv(dev);
> + if (!dev->phydev)
> + return -ENODEV;
>
> - if (p->phy != NULL)
> - return genphy_restart_aneg(p->phy);
> -
> - return -EOPNOTSUPP;
> + return genphy_restart_aneg(dev->phydev);
> }

It looks like this can now be replaced with phy_ethtool_nway_reset().

It could be there are other phy_ethtool_ helpers which can be used,
now that we have phydev in ndev.

Andrew

2017-09-22 20:15:23

by Vivien Didelot

[permalink] [raw]
Subject: Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev

Hi Florian,

Florian Fainelli <[email protected]> writes:

> On 09/22/2017 12:40 PM, Vivien Didelot wrote:
>> There is no need to store a phy_device in dsa_slave_priv since
>> net_device already provides one. Simply s/p->phy/dev->phydev/.
>
> You can therefore remove the phy_device from dsa_slave_priv, see below
> for more comments. I will have to regress test the heck out of this,
> this should take a few hours.

OK, since this is a sensible topic, I will respin a v3 without this
patch, so that a future patchset can address your comments below and
also gives you time to test this one patch alone.

>> static int dsa_slave_port_attr_set(struct net_device *dev,
>> @@ -435,12 +433,10 @@ static int
>> dsa_slave_get_link_ksettings(struct net_device *dev,
>> struct ethtool_link_ksettings *cmd)
>> {
>> - struct dsa_slave_priv *p = netdev_priv(dev);
>> + if (!dev->phydev)
>> + return -ENODEV;
>>
>> - if (!p->phy)
>> - return -EOPNOTSUPP;
>> -
>> - phy_ethtool_ksettings_get(p->phy, cmd);
>> + phy_ethtool_ksettings_get(dev->phydev, cmd);
>
> This can be replaced by phy_ethtool_get_link_ksettings()
>
>>
>> return 0;
>> }
>> @@ -449,12 +445,10 @@ static int
>> dsa_slave_set_link_ksettings(struct net_device *dev,
>> const struct ethtool_link_ksettings *cmd)
>> {
>> - struct dsa_slave_priv *p = netdev_priv(dev);
>> + if (!dev->phydev)
>> + return -ENODEV;
>>
>> - if (p->phy != NULL)
>> - return phy_ethtool_ksettings_set(p->phy, cmd);
>> -
>> - return -EOPNOTSUPP;
>> + return phy_ethtool_ksettings_set(dev->phydev, cmd);
>> }
>
> This can disappear and you can assign this ethtool operation to
> phy_ethtool_set_link_ksettings()
>
>>
>> static void dsa_slave_get_drvinfo(struct net_device *dev,
>> @@ -488,24 +482,20 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
>>
>> static int dsa_slave_nway_reset(struct net_device *dev)
>> {
>> - struct dsa_slave_priv *p = netdev_priv(dev);
>> + if (!dev->phydev)
>> + return -ENODEV;
>>
>> - if (p->phy != NULL)
>> - return genphy_restart_aneg(p->phy);
>> -
>> - return -EOPNOTSUPP;
>> + return genphy_restart_aneg(dev->phydev);
>> }
>
> This can now disappear and you can use phy_ethtool_nway_reset() directly
> in ethtool_ops
>
>>
>> static u32 dsa_slave_get_link(struct net_device *dev)
>> {
>> - struct dsa_slave_priv *p = netdev_priv(dev);
>> + if (!dev->phydev)
>> + return -ENODEV;
>>
>> - if (p->phy != NULL) {
>> - genphy_update_link(p->phy);
>> - return p->phy->link;
>> - }
>> + genphy_update_link(dev->phydev);
>>
>> - return -EOPNOTSUPP;
>> + return dev->phydev->link;
>> }
>
> This should certainly be just ethtool_op_get_link(), not sure why we
> kept that around here...

Haaa, good to read that! I wasn't sure about this, but with this patch
the slave phy ethtool functions seemed indeed quite generic...


Thanks,

Vivien