diff mbox series

[PATCHv2,3/3] CDC-NCM: record speed in status method

Message ID 20210121125731.19425-4-oneukum@suse.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series usbnet: speed reporting for devices without MDIO | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers warning 2 maintainers not CCed: kuba@kernel.org oliver@neukum.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 41 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Oliver Neukum Jan. 21, 2021, 12:57 p.m. UTC
The driver has a status method for receiving speed updates.
The framework, however, had support functions only for devices
that reported their speed upon an explicit query over a MDIO
interface.
CDC_NCM however gets direct notifications from the device.
As new support functions have become available, we shall now
record such notifications and tell the usbnet framework
to make direct use of them without going through the PHY layer.

v2: adjusted to recent changes

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Tested-by: Roland Dreier <roland@kernel.org>
---
 drivers/net/usb/cdc_ncm.c | 29 ++++-------------------------
 1 file changed, 4 insertions(+), 25 deletions(-)

Comments

Grant Grundler Jan. 22, 2021, 3:02 a.m. UTC | #1
On Thu, Jan 21, 2021 at 12:57 PM Oliver Neukum <oneukum@suse.com> wrote:
>
> The driver has a status method for receiving speed updates.
> The framework, however, had support functions only for devices
> that reported their speed upon an explicit query over a MDIO
> interface.
> CDC_NCM however gets direct notifications from the device.
> As new support functions have become available, we shall now
> record such notifications and tell the usbnet framework
> to make direct use of them without going through the PHY layer.
>
> v2: adjusted to recent changes
>
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Tested-by: Roland Dreier <roland@kernel.org>
> ---
>  drivers/net/usb/cdc_ncm.c | 29 ++++-------------------------
>  1 file changed, 4 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 04174704bf7c..9b5bb8ae5eb8 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -142,7 +142,7 @@ 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_mdio,
> +       .get_link_ksettings      = usbnet_get_link_ksettings_internal,
>         .set_link_ksettings      = usbnet_set_link_ksettings_mdio,
>  };
>
> @@ -1827,30 +1827,9 @@ cdc_ncm_speed_change(struct usbnet *dev,
>         uint32_t rx_speed = le32_to_cpu(data->DLBitRRate);
>         uint32_t tx_speed = le32_to_cpu(data->ULBitRate);
>
> -       /* if the speed hasn't changed, don't report it.
> -        * RTL8156 shipped before 2021 sends notification about every 32ms.
> -        */
> -       if (dev->rx_speed == rx_speed && dev->tx_speed == tx_speed)
> -               return;
> -
> -       dev->rx_speed = rx_speed;
> -       dev->tx_speed = tx_speed;

Oliver,
This patch removes the use of "rx_speed" field but doesn't remove the
field from struct usbnet (usbnet.h).

It might be better to revert my patch (make that the first patch in
your series) and use your original patches.
OR use "rx_speed" as the field name in the entire series and move them
to the preferred location in struct usbnet (patch 2/3).

cheers,
grant

> -
> -       /*
> -        * Currently the USB-NET API does not support reporting the actual
> -        * device speed. Do print it instead.
> -        */
> -       if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
> -               netif_info(dev, link, dev->net,
> -                          "%u mbit/s downlink %u mbit/s uplink\n",
> -                          (unsigned int)(rx_speed / 1000000U),
> -                          (unsigned int)(tx_speed / 1000000U));
> -       } else {
> -               netif_info(dev, link, dev->net,
> -                          "%u kbit/s downlink %u kbit/s uplink\n",
> -                          (unsigned int)(rx_speed / 1000U),
> -                          (unsigned int)(tx_speed / 1000U));
> -       }
> +        /* RTL8156 shipped before 2021 sends notification about every 32ms. */
> +       dev->rxspeed = rx_speed;
> +       dev->txspeed = tx_speed;
>  }
>
>  static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
> --
> 2.26.2
>
diff mbox series

Patch

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 04174704bf7c..9b5bb8ae5eb8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -142,7 +142,7 @@  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_mdio,
+	.get_link_ksettings      = usbnet_get_link_ksettings_internal,
 	.set_link_ksettings      = usbnet_set_link_ksettings_mdio,
 };
 
@@ -1827,30 +1827,9 @@  cdc_ncm_speed_change(struct usbnet *dev,
 	uint32_t rx_speed = le32_to_cpu(data->DLBitRRate);
 	uint32_t tx_speed = le32_to_cpu(data->ULBitRate);
 
-	/* if the speed hasn't changed, don't report it.
-	 * RTL8156 shipped before 2021 sends notification about every 32ms.
-	 */
-	if (dev->rx_speed == rx_speed && dev->tx_speed == tx_speed)
-		return;
-
-	dev->rx_speed = rx_speed;
-	dev->tx_speed = tx_speed;
-
-	/*
-	 * Currently the USB-NET API does not support reporting the actual
-	 * device speed. Do print it instead.
-	 */
-	if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
-		netif_info(dev, link, dev->net,
-			   "%u mbit/s downlink %u mbit/s uplink\n",
-			   (unsigned int)(rx_speed / 1000000U),
-			   (unsigned int)(tx_speed / 1000000U));
-	} else {
-		netif_info(dev, link, dev->net,
-			   "%u kbit/s downlink %u kbit/s uplink\n",
-			   (unsigned int)(rx_speed / 1000U),
-			   (unsigned int)(tx_speed / 1000U));
-	}
+	 /* RTL8156 shipped before 2021 sends notification about every 32ms. */
+	dev->rxspeed = rx_speed;
+	dev->txspeed = tx_speed;
 }
 
 static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)