netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn-yOkvZcmFvRU@public.gmane.org>
To: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Alexey Orishko"
	<alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"Oliver Neukum" <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>,
	"Enrico Mioso"
	<mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"David Laight"
	<David.Laight-ZS65k/vG3HxXrIkS9f7CXA@public.gmane.org>,
	"Lars Melin" <larsm17-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"Peter Stuge" <peter-Y+HMSxxDrH8@public.gmane.org>,
	"Greg Suarez" <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>,
	"Bjørn Mork" <bjorn-yOkvZcmFvRU@public.gmane.org>
Subject: [PATCH v2 net-next 4/8] net: cdc_ncm: use sysfs for rx/tx aggregation tuning
Date: Fri, 30 May 2014 09:31:06 +0200	[thread overview]
Message-ID: <1401435070-26721-5-git-send-email-bjorn@mork.no> (raw)
In-Reply-To: <1401435070-26721-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>

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-Y+HMSxxDrH8@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c | 144 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 125 insertions(+), 19 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 5bce86a0d063..631741c8ff22 100644
--- 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);

  parent reply	other threads:[~2014-05-30  7:31 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-30  7:31 [PATCH v2 net-next 0/8] cdc_ncm: fixes and conversion to sysfs API Bjørn Mork
2014-05-30  7:31 ` [PATCH v2 net-next 1/8] net: cdc_ncm: reduce skb truesize in rx path Bjørn Mork
     [not found] ` <1401435070-26721-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
2014-05-30  7:31   ` [PATCH v2 net-next 2/8] net: cdc_ncm: always reallocate tx_curr_skb when tx_max increases Bjørn Mork
2014-05-30  7:31   ` Bjørn Mork [this message]
2014-05-30  7:31 ` [PATCH v2 net-next 3/8] net: cdc_ncm: inform usbnet when rx buffers are reduced Bjørn Mork
     [not found]   ` <1401435070-26721-4-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
2014-05-30  9:18     ` David Laight
2014-05-30 11:39       ` Bjørn Mork
2014-05-30 12:39         ` David Laight
2014-05-30 13:35           ` Bjørn Mork
     [not found]         ` <87d2eva41t.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2014-05-30 12:42           ` David Laight
2014-05-30 13:39             ` Bjørn Mork
2014-05-30  7:31 ` [PATCH v2 net-next 5/8] net: cdc_ncm: drop ethtool coalesce support Bjørn Mork
2014-05-30  7:31 ` [PATCH v2 net-next 6/8] net: cdc_ncm: export NCM Transfer Block (NTB) parameters Bjørn Mork
2014-05-30  7:31 ` [PATCH v2 net-next 7/8] net: cdc_ncm: allow tuning min_tx_pkt Bjørn Mork
2014-05-30  7:31 ` [PATCH v2 net-next 8/8] net: cdc_ncm: document the sysfs API Bjørn Mork
2014-06-02 23:02 ` [PATCH v2 net-next 0/8] cdc_ncm: fixes and conversion to " David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1401435070-26721-5-git-send-email-bjorn@mork.no \
    --to=bjorn-yokvzcmfvru@public.gmane.org \
    --cc=David.Laight-ZS65k/vG3HxXrIkS9f7CXA@public.gmane.org \
    --cc=alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org \
    --cc=larsm17-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org \
    --cc=peter-Y+HMSxxDrH8@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).