net: cdc_ncm: use sysfs for rx/tx aggregation tuning

Attach a driver specific sysfs group to the netdev, and use it
for the rx/tx aggregation variables.

The datagram aggregation defined by the CDC NCM specification is
specific to this device class (including CDC MBIM). Using the
ethtool interrupt coalesce API as an interface to the aggregation
parameters redefined that API in a driver specific and confusing
way.  A sysfs group
 - makes it clear that this is a driver specific userspace API, and
 - allows us to export the real values instead of some translated
   version, and
 - lets us include more aggregation variables which were impossible
   to force into the ethtool API.

Additionally, using sysfs allows tuning the driver on space
constrained hosts where userspace tools like ethtool are undesired.

Suggested-by: Peter Stuge <peter@stuge.se>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 5bce86a0d06392f3617ff19ba498a9f73d190475..631741c8ff2261eaf3422381c048ea7810657dc0 100644 (file)
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -191,11 +191,9 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
        .set_coalesce      = cdc_ncm_set_coalesce,
 };
 
-/* handle rx_max and tx_max changes */
-static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
+static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx)
 {
        struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
-       u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
        u32 val, max, min;
 
        /* clamp new_rx to sane values */
@@ -210,10 +208,126 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
        }
 
        val = clamp_t(u32, new_rx, min, max);
-       if (val != new_rx) {
-               dev_dbg(&dev->intf->dev, "rx_max must be in the [%u, %u] range. Using %u\n",
-                       min, max, val);
-       }
+       if (val != new_rx)
+               dev_dbg(&dev->intf->dev, "rx_max must be in the [%u, %u] range\n", min, max);
+
+       return val;
+}
+
+static u32 cdc_ncm_check_tx_max(struct usbnet *dev, u32 new_tx)
+{
+       struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+       u32 val, max, min;
+
+       /* clamp new_tx to sane values */
+       min = ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct usb_cdc_ncm_nth16);
+       max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize));
+
+       /* some devices set dwNtbOutMaxSize too low for the above default */
+       min = min(min, max);
+
+       val = clamp_t(u32, new_tx, min, max);
+       if (val != new_tx)
+               dev_dbg(&dev->intf->dev, "tx_max must be in the [%u, %u] range\n", min, max);
+
+       return val;
+}
+
+static ssize_t cdc_ncm_show_rx_max(struct device *d, struct device_attribute *attr, char *buf)
+{
+       struct usbnet *dev = netdev_priv(to_net_dev(d));
+       struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+
+       return sprintf(buf, "%u\n", ctx->rx_max);
+}
+
+static ssize_t cdc_ncm_show_tx_max(struct device *d, struct device_attribute *attr, char *buf)
+{
+       struct usbnet *dev = netdev_priv(to_net_dev(d));
+       struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+
+       return sprintf(buf, "%u\n", ctx->tx_max);
+}
+
+static ssize_t cdc_ncm_show_tx_timer_usecs(struct device *d, struct device_attribute *attr, char *buf)
+{
+       struct usbnet *dev = netdev_priv(to_net_dev(d));
+       struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+
+       return sprintf(buf, "%u\n", ctx->timer_interval / (u32)NSEC_PER_USEC);
+}
+
+static ssize_t cdc_ncm_store_rx_max(struct device *d,  struct device_attribute *attr, const char *buf, size_t len)
+{
+       struct usbnet *dev = netdev_priv(to_net_dev(d));
+       struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+       unsigned long val;
+
+       if (kstrtoul(buf, 0, &val) || cdc_ncm_check_rx_max(dev, val) != val)
+               return -EINVAL;
+
+       cdc_ncm_update_rxtx_max(dev, val, ctx->tx_max);
+       return len;
+}
+
+static ssize_t cdc_ncm_store_tx_max(struct device *d,  struct device_attribute *attr, const char *buf, size_t len)
+{
+       struct usbnet *dev = netdev_priv(to_net_dev(d));
+       struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+       unsigned long val;
+
+       if (kstrtoul(buf, 0, &val) || cdc_ncm_check_tx_max(dev, val) != val)
+               return -EINVAL;
+
+       cdc_ncm_update_rxtx_max(dev, ctx->rx_max, val);
+       return len;
+}
+
+static ssize_t cdc_ncm_store_tx_timer_usecs(struct device *d,  struct device_attribute *attr, const char *buf, size_t len)
+{
+       struct usbnet *dev = netdev_priv(to_net_dev(d));
+       struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+       ssize_t ret;
+       unsigned long val;
+
+       ret = kstrtoul(buf, 0, &val);
+       if (ret)
+               return ret;
+       if (val && (val < CDC_NCM_TIMER_INTERVAL_MIN || val > CDC_NCM_TIMER_INTERVAL_MAX))
+               return -EINVAL;
+
+       spin_lock_bh(&ctx->mtx);
+       ctx->timer_interval = val * NSEC_PER_USEC;
+       if (!ctx->timer_interval)
+               ctx->tx_timer_pending = 0;
+       spin_unlock_bh(&ctx->mtx);
+       return len;
+}
+
+static DEVICE_ATTR(rx_max, S_IRUGO | S_IWUSR, cdc_ncm_show_rx_max, cdc_ncm_store_rx_max);
+static DEVICE_ATTR(tx_max, S_IRUGO | S_IWUSR, cdc_ncm_show_tx_max, cdc_ncm_store_tx_max);
+static DEVICE_ATTR(tx_timer_usecs, S_IRUGO | S_IWUSR, cdc_ncm_show_tx_timer_usecs, cdc_ncm_store_tx_timer_usecs);
+
+static struct attribute *cdc_ncm_sysfs_attrs[] = {
+       &dev_attr_rx_max.attr,
+       &dev_attr_tx_max.attr,
+       &dev_attr_tx_timer_usecs.attr,
+       NULL,
+};
+
+static struct attribute_group cdc_ncm_sysfs_attr_group = {
+       .name = "cdc_ncm",
+       .attrs = cdc_ncm_sysfs_attrs,
+};
+
+/* handle rx_max and tx_max changes */
+static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
+{
+       struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+       u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
+       u32 val;
+
+       val = cdc_ncm_check_rx_max(dev, new_rx);
 
        /* inform device about NTB input size changes */
        if (val != ctx->rx_max) {
@@ -238,18 +352,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
                        usbnet_unlink_rx_urbs(dev);
        }
 
-       /* clamp new_tx to sane values */
-       min = ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct usb_cdc_ncm_nth16);
-       max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize));
-
-       /* some devices set dwNtbOutMaxSize too low for the above default */
-       min = min(min, max);
-
-       val = clamp_t(u32, new_tx, min, max);
-       if (val != new_tx) {
-               dev_dbg(&dev->intf->dev, "tx_max must be in the [%u, %u] range. Using %u\n",
-                       min, max, val);
-       }
+       val = cdc_ncm_check_tx_max(dev, new_tx);
        if (val != ctx->tx_max)
                dev_info(&dev->intf->dev, "setting tx_max = %u\n", val);
 
@@ -749,6 +852,9 @@ advance:
        /* override ethtool_ops */
        dev->net->ethtool_ops = &cdc_ncm_ethtool_ops;
 
+       /* add our sysfs attrs */
+       dev->net->sysfs_groups[0] = &cdc_ncm_sysfs_attr_group;
+
        return 0;
 
 error2: