Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[David Miller]

From: Heiner Kallweit 
Date: Tue, 21 Jan 2020 22:09:33 +0100

> Convert suitable drivers to use new helper phy_do_ioctl_running.
> 
> Signed-off-by: Heiner Kallweit 
> ---
> v2: I forgot the netdev mailing list

Applied to net-next.

Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Heiner Kallweit]

On 22.01.2020 05:04, Florian Fainelli wrote:
> 
> 
> On 1/21/2020 1:09 PM, Heiner Kallweit wrote:
>> Convert suitable drivers to use new helper phy_do_ioctl_running.
>>
>> Signed-off-by: Heiner Kallweit 
> The vast majority of drivers that you are converting use the following
> convention:
> 
> - !netif_running -> return -EINVAL
> - !dev->phydev -> return -ENODEV
> 
> so it may make sense to change the helper to accommodate the majority
> here, not that I believe this is going to make much practical
> difference, but if there were test cases that were specifically looking
> for such an error code, they could be failing after this changeset.
> 
Right, I also stumbled across the different error codes, mainly as you
say -EINVAL. However there is no "wrong value", if netdev isn't running,
then typically the PHY is not attached, and from a netdev point of view
it's not there. So ENODEV seems to be best suited.
In kernel code the changed return code doesn't make a difference,
but yes, in theory there could be userspace programs checking for
-EINVAL. However such userspace programs should check for ENODEV too
anyway to cover the second check that already returns -ENODEV.

> For bgmac.c, bcmgenet.c and cpmac.c:
> 
> Acked-by: Florian Fainelli 
> 
> Whether you decide to spin another version or not.
> 
Heiner

Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Florian Fainelli]

On 1/21/2020 1:09 PM, Heiner Kallweit wrote:
> Convert suitable drivers to use new helper phy_do_ioctl_running.
> 
> Signed-off-by: Heiner Kallweit 
The vast majority of drivers that you are converting use the following
convention:

- !netif_running -> return -EINVAL
- !dev->phydev -> return -ENODEV

so it may make sense to change the helper to accommodate the majority
here, not that I believe this is going to make much practical
difference, but if there were test cases that were specifically looking
for such an error code, they could be failing after this changeset.

For bgmac.c, bcmgenet.c and cpmac.c:

Acked-by: Florian Fainelli 

Whether you decide to spin another version or not.
-- 
Florian

Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Timur Tabi]

On 1/21/20 3:09 PM, Heiner Kallweit wrote:

  drivers/net/ethernet/qualcomm/emac/emac.c  | 14 +-


Acked-by: Timur Tabi 

[PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Heiner Kallweit]

Convert suitable drivers to use new helper phy_do_ioctl_running.

Signed-off-by: Heiner Kallweit 
---
v2: I forgot the netdev mailing list
---
 drivers/net/ethernet/allwinner/sun4i-emac.c| 15 +--
 drivers/net/ethernet/amd/au1000_eth.c  | 13 +
 drivers/net/ethernet/arc/emac_main.c   | 14 +-
 drivers/net/ethernet/broadcom/bgmac.c  | 10 +-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 14 +-
 drivers/net/ethernet/dnet.c| 15 +--
 .../ethernet/freescale/fs_enet/fs_enet-main.c  | 10 +-
 drivers/net/ethernet/hisilicon/hisi_femac.c| 14 +-
 drivers/net/ethernet/hisilicon/hns/hns_enet.c  | 16 +---
 drivers/net/ethernet/nxp/lpc_eth.c | 15 +--
 drivers/net/ethernet/qualcomm/emac/emac.c  | 14 +-
 drivers/net/ethernet/renesas/sh_eth.c  | 18 ++
 drivers/net/ethernet/smsc/smsc911x.c   | 11 +--
 drivers/net/ethernet/smsc/smsc9420.c   | 11 +--
 drivers/net/ethernet/ti/cpmac.c| 12 +---
 drivers/net/ethernet/toshiba/tc35815.c | 12 +---
 drivers/net/ethernet/xilinx/ll_temac_main.c| 13 +
 drivers/net/usb/ax88172a.c | 13 +
 drivers/net/usb/lan78xx.c  | 10 +-
 19 files changed, 20 insertions(+), 230 deletions(-)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c 
b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 5ea806423..22cadfbee 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -207,19 +207,6 @@ static void emac_inblk_32bit(void __iomem *reg, void 
*data, int count)
readsl(reg, data, round_up(count, 4) / 4);
 }
 
-static int emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   struct phy_device *phydev = dev->phydev;
-
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!phydev)
-   return -ENODEV;
-
-   return phy_mii_ioctl(phydev, rq, cmd);
-}
-
 /* ethtool ops */
 static void emac_get_drvinfo(struct net_device *dev,
  struct ethtool_drvinfo *info)
@@ -791,7 +778,7 @@ static const struct net_device_ops emac_netdev_ops = {
.ndo_start_xmit = emac_start_xmit,
.ndo_tx_timeout = emac_timeout,
.ndo_set_rx_mode= emac_set_rx_mode,
-   .ndo_do_ioctl   = emac_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
.ndo_validate_addr  = eth_validate_addr,
.ndo_set_mac_address= emac_set_mac_address,
 #ifdef CONFIG_NET_POLL_CONTROLLER
diff --git a/drivers/net/ethernet/amd/au1000_eth.c 
b/drivers/net/ethernet/amd/au1000_eth.c
index 6acf5aa99..089a4fbc6 100644
--- a/drivers/net/ethernet/amd/au1000_eth.c
+++ b/drivers/net/ethernet/amd/au1000_eth.c
@@ -1053,23 +1053,12 @@ static void au1000_multicast_list(struct net_device 
*dev)
writel(reg, >mac->control);
 }
 
-static int au1000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!dev->phydev)
-   return -EINVAL; /* PHY not controllable */
-
-   return phy_mii_ioctl(dev->phydev, rq, cmd);
-}
-
 static const struct net_device_ops au1000_netdev_ops = {
.ndo_open   = au1000_open,
.ndo_stop   = au1000_close,
.ndo_start_xmit = au1000_tx,
.ndo_set_rx_mode= au1000_multicast_list,
-   .ndo_do_ioctl   = au1000_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
.ndo_tx_timeout = au1000_tx_timeout,
.ndo_set_mac_address= eth_mac_addr,
.ndo_validate_addr  = eth_validate_addr,
diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 6f2c86778..17bda4e8c 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -781,18 +781,6 @@ static int arc_emac_set_address(struct net_device *ndev, 
void *p)
return 0;
 }
 
-static int arc_emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!dev->phydev)
-   return -ENODEV;
-
-   return phy_mii_ioctl(dev->phydev, rq, cmd);
-}
-
-
 /**
  * arc_emac_restart - Restart EMAC
  * @ndev:  Pointer to net_device structure.
@@ -857,7 +845,7 @@ static const struct net_device_ops arc_emac_netdev_ops = {
.ndo_set_mac_address= arc_emac_set_address,
.ndo_get_stats  = arc_emac_stats,
.ndo_set_rx_mode= arc_emac_set_rx_mode,
-   .ndo_do_ioctl   = arc_emac_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
 #ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller