diff mbox series

[PATCHv2,1/3] usbnet: specify naming of usbnet_set/get_link_ksettings

Message ID 20210121125731.19425-2-oneukum@suse.com (mailing list archive)
State New, archived
Headers show
Series usbnet: speed reporting for devices without MDIO | expand

Commit Message

Oliver Neukum Jan. 21, 2021, 12:57 p.m. UTC 
The old generic functions assume that the devices includes
an MDIO interface. This is true only for genuine ethernet.
Devices with a higher level of abstraction or based on different
technologies do not have it. So in preparation for
supporting that, we rename the old functions to something specific.

v2: adjusted to recent changes

Signed-off-by : Oliver Neukum <oneukum@suse.com>
Tested-by: Roland Dreier <roland@kernel.org>
---
 drivers/net/usb/asix_devices.c | 12 ++++++------
 drivers/net/usb/cdc_ncm.c      |  4 ++--
 drivers/net/usb/dm9601.c       |  4 ++--
 drivers/net/usb/mcs7830.c      |  4 ++--
 drivers/net/usb/sierra_net.c   |  4 ++--
 drivers/net/usb/smsc75xx.c     |  4 ++--
 drivers/net/usb/sr9700.c       |  4 ++--
 drivers/net/usb/sr9800.c       |  4 ++--
 drivers/net/usb/usbnet.c       | 15 +++++++++------
 include/linux/usb/usbnet.h     |  4 ++--
 10 files changed, 31 insertions(+), 28 deletions(-)

Comments

Andrew Lunn Jan. 22, 2021, 1:10 a.m. UTC | #1 
On Thu, Jan 21, 2021 at 01:57:29PM +0100, Oliver Neukum wrote:
> The old generic functions assume that the devices includes
> an MDIO interface. This is true only for genuine ethernet.
> Devices with a higher level of abstraction or based on different
> technologies do not have it. So in preparation for
> supporting that, we rename the old functions to something specific.
> 
> v2: adjusted to recent changes

Hi Oliver

It  looks like my comment:

https://www.spinics.net/lists/netdev/msg711869.html

was ignored. Do you not like the name mii?

    Andrew
Oliver Neukum Jan. 26, 2021, 9:42 a.m. UTC | #2 
Am Freitag, den 22.01.2021, 02:10 +0100 schrieb Andrew Lunn:
> On Thu, Jan 21, 2021 at 01:57:29PM +0100, Oliver Neukum wrote:
> > The old generic functions assume that the devices includes
> > an MDIO interface. This is true only for genuine ethernet.
> > Devices with a higher level of abstraction or based on different
> > technologies do not have it. So in preparation for
> > supporting that, we rename the old functions to something specific.
> > 
> > v2: adjusted to recent changes
> 
> Hi Oliver
> 
> It  looks like my comment:
> 
> https://www.spinics.net/lists/netdev/msg711869.html
> 
> was ignored. Do you not like the name mii?

Hi,

sorry for not replying earlier.

It was my understanding that on the hardware level of the
networking devices we are using MII, but to control MII we
use MDIO, don't we?
So it seems to me that hardware could use MII but not
MDIO, yet for this purpose we require MDIO. So could
you please explain your reasoning about networking stuff?

I do not want to create false impressions in users.

	Regards
		Oliver
Andrew Lunn Jan. 26, 2021, 1:34 p.m. UTC | #3 
On Tue, Jan 26, 2021 at 10:42:09AM +0100, Oliver Neukum wrote:
> Am Freitag, den 22.01.2021, 02:10 +0100 schrieb Andrew Lunn:
> > On Thu, Jan 21, 2021 at 01:57:29PM +0100, Oliver Neukum wrote:
> > > The old generic functions assume that the devices includes
> > > an MDIO interface. This is true only for genuine ethernet.
> > > Devices with a higher level of abstraction or based on different
> > > technologies do not have it. So in preparation for
> > > supporting that, we rename the old functions to something specific.
> > > 
> > > v2: adjusted to recent changes
> > 
> > Hi Oliver
> > 
> > It  looks like my comment:
> > 
> > https://www.spinics.net/lists/netdev/msg711869.html
> > 
> > was ignored. Do you not like the name mii?
> 
> Hi,
> 
> sorry for not replying earlier.
> 
> It was my understanding that on the hardware level of the
> networking devices we are using MII, but to control MII we
> use MDIO, don't we?
> So it seems to me that hardware could use MII but not
> MDIO, yet for this purpose we require MDIO. So could
> you please explain your reasoning about networking stuff?

Hi Oliver

To some extent, it is a terminology problem. First off, MII includes
the two MDIO pins. MDIO is a subset of MII.

However, the bigger issue is Linux has two different bits of code
which can be used to talk to the PHY. There is the old mii code,
driver/net/mii.c. This code assumes the PHY exactly follows 802.3
clause 22.

Then we have drivers/net/mdio, drivers/net/phy, phylib, and a
collection of PHYs drivers. The MDIO drivers implement the MDIO bus,
allowing transfers over the bus. And the PHY drivers then handle the
devices on this bus. These PHY drivers can handle nearly any quirk the
PHY might have which deviate from C22. It also allows drivers to use
C45, the alternative register set PHYs can use. And it allows for
added extras, like temperature sensors, cable diagnostics and
statistics, none of which is standardised.

The code you are changing makes use of the older mii code.  There are
however some USB devices which use phylib. By using the postfix _mii
for these ops, it makes it clear it is using the older mii code. In
the future, there might be _phylib versions of these ops. It is very
unlikely any USB device driver will directly use an MDIO bus drivers,
so _mdio does not really make sense, from the perspective of Linux
code.

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 6e13d8165852..f5006890873d 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -125,8 +125,8 @@  static const struct ethtool_ops ax88172_ethtool_ops = {
 	.get_eeprom		= asix_get_eeprom,
 	.set_eeprom		= asix_set_eeprom,
 	.nway_reset		= usbnet_nway_reset,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 static void ax88172_set_multicast(struct net_device *net)
@@ -291,8 +291,8 @@  static const struct ethtool_ops ax88772_ethtool_ops = {
 	.get_eeprom		= asix_get_eeprom,
 	.set_eeprom		= asix_set_eeprom,
 	.nway_reset		= usbnet_nway_reset,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 static int ax88772_link_reset(struct usbnet *dev)
@@ -782,8 +782,8 @@  static const struct ethtool_ops ax88178_ethtool_ops = {
 	.get_eeprom		= asix_get_eeprom,
 	.set_eeprom		= asix_set_eeprom,
 	.nway_reset		= usbnet_nway_reset,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 static int marvell_phy_init(struct usbnet *dev)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 291e76d32abe..04174704bf7c 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -142,8 +142,8 @@  static const struct ethtool_ops cdc_ncm_ethtool_ops = {
 	.get_sset_count    = cdc_ncm_get_sset_count,
 	.get_strings       = cdc_ncm_get_strings,
 	.get_ethtool_stats = cdc_ncm_get_ethtool_stats,
-	.get_link_ksettings      = usbnet_get_link_ksettings,
-	.set_link_ksettings      = usbnet_set_link_ksettings,
+	.get_link_ksettings      = usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings      = usbnet_set_link_ksettings_mdio,
 };
 
 static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx)
diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index b5d2ac55a874..9065aa13540b 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -282,8 +282,8 @@  static const struct ethtool_ops dm9601_ethtool_ops = {
 	.get_eeprom_len	= dm9601_get_eeprom_len,
 	.get_eeprom	= dm9601_get_eeprom,
 	.nway_reset	= usbnet_nway_reset,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 static void dm9601_set_multicast(struct net_device *net)
diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index fc512b780d15..c52f52b1e619 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -452,8 +452,8 @@  static const struct ethtool_ops mcs7830_ethtool_ops = {
 	.get_msglevel		= usbnet_get_msglevel,
 	.set_msglevel		= usbnet_set_msglevel,
 	.nway_reset		= usbnet_nway_reset,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 static const struct net_device_ops mcs7830_netdev_ops = {
diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index 55a244eca5ca..cbab5c9f19b8 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -629,8 +629,8 @@  static const struct ethtool_ops sierra_net_ethtool_ops = {
 	.get_msglevel = usbnet_get_msglevel,
 	.set_msglevel = usbnet_set_msglevel,
 	.nway_reset = usbnet_nway_reset,
-	.get_link_ksettings = usbnet_get_link_ksettings,
-	.set_link_ksettings = usbnet_set_link_ksettings,
+	.get_link_ksettings = usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings = usbnet_set_link_ksettings_mdio,
 };
 
 static int sierra_net_get_fw_attr(struct usbnet *dev, u16 *datap)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 4353b370249f..e4c6c7f3c69b 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -741,8 +741,8 @@  static const struct ethtool_ops smsc75xx_ethtool_ops = {
 	.set_eeprom	= smsc75xx_ethtool_set_eeprom,
 	.get_wol	= smsc75xx_ethtool_get_wol,
 	.set_wol	= smsc75xx_ethtool_set_wol,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 static int smsc75xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index 878557ad03ad..e552ab95ced2 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -250,8 +250,8 @@  static const struct ethtool_ops sr9700_ethtool_ops = {
 	.get_eeprom_len	= sr9700_get_eeprom_len,
 	.get_eeprom	= sr9700_get_eeprom,
 	.nway_reset	= usbnet_nway_reset,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 static void sr9700_set_multicast(struct net_device *netdev)
diff --git a/drivers/net/usb/sr9800.c b/drivers/net/usb/sr9800.c
index da56735d7755..b466d53047c5 100644
--- a/drivers/net/usb/sr9800.c
+++ b/drivers/net/usb/sr9800.c
@@ -527,8 +527,8 @@  static const struct ethtool_ops sr9800_ethtool_ops = {
 	.get_eeprom_len	= sr_get_eeprom_len,
 	.get_eeprom	= sr_get_eeprom,
 	.nway_reset	= usbnet_nway_reset,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 static int sr9800_link_reset(struct usbnet *dev)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 1447da1d5729..e2ca88259b05 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -944,7 +944,10 @@  EXPORT_SYMBOL_GPL(usbnet_open);
  * they'll probably want to use this base set.
  */
 
-int usbnet_get_link_ksettings(struct net_device *net,
+/* These methods are written on the assumption that the device
+ * uses MII
+ */
+int usbnet_get_link_ksettings_mdio(struct net_device *net,
 			      struct ethtool_link_ksettings *cmd)
 {
 	struct usbnet *dev = netdev_priv(net);
@@ -956,9 +959,9 @@  int usbnet_get_link_ksettings(struct net_device *net,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(usbnet_get_link_ksettings);
+EXPORT_SYMBOL_GPL(usbnet_get_link_ksettings_mdio);
 
-int usbnet_set_link_ksettings(struct net_device *net,
+int usbnet_set_link_ksettings_mdio(struct net_device *net,
 			      const struct ethtool_link_ksettings *cmd)
 {
 	struct usbnet *dev = netdev_priv(net);
@@ -978,7 +981,7 @@  int usbnet_set_link_ksettings(struct net_device *net,
 
 	return retval;
 }
-EXPORT_SYMBOL_GPL(usbnet_set_link_ksettings);
+EXPORT_SYMBOL_GPL(usbnet_set_link_ksettings_mdio);
 
 u32 usbnet_get_link (struct net_device *net)
 {
@@ -1043,8 +1046,8 @@  static const struct ethtool_ops usbnet_ethtool_ops = {
 	.get_msglevel		= usbnet_get_msglevel,
 	.set_msglevel		= usbnet_set_msglevel,
 	.get_ts_info		= ethtool_op_get_ts_info,
-	.get_link_ksettings	= usbnet_get_link_ksettings,
-	.set_link_ksettings	= usbnet_set_link_ksettings,
+	.get_link_ksettings	= usbnet_get_link_ksettings_mdio,
+	.set_link_ksettings	= usbnet_set_link_ksettings_mdio,
 };
 
 /*-------------------------------------------------------------------------*/
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index cfbfd6fe01df..fd65b7a5ee15 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -267,9 +267,9 @@  extern void usbnet_pause_rx(struct usbnet *);
 extern void usbnet_resume_rx(struct usbnet *);
 extern void usbnet_purge_paused_rxq(struct usbnet *);
 
-extern int usbnet_get_link_ksettings(struct net_device *net,
+extern int usbnet_get_link_ksettings_mdio(struct net_device *net,
 				     struct ethtool_link_ksettings *cmd);
-extern int usbnet_set_link_ksettings(struct net_device *net,
+extern int usbnet_set_link_ksettings_mdio(struct net_device *net,
 				     const struct ethtool_link_ksettings *cmd);
 extern u32 usbnet_get_link(struct net_device *net);
 extern u32 usbnet_get_msglevel(struct net_device *);