netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] net: qmi_wwan: MDM9x30 support
@ 2015-12-03 18:24 Bjørn Mork
  2015-12-03 18:24 ` [PATCH 1/6] net: qmi_wwan: MDM9x30 specific power management Bjørn Mork
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Bjørn Mork @ 2015-12-03 18:24 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Marcel Holtmann, Oliver Neukum, Aleksander Morgado,
	Dan Williams, Bjørn Mork

We add new device IDs all the time, often without any testing on
actual hardware. This is usually OK as long as the device is similar
to already supported devices, using the same chipset and firmware
basis.  But the Sierra Wireless MC7455 is an example of a new chipset
generation. Adding it based on assumed similarity with its ancestors
proved too optimistic.

This series adds the missing bits and pieces necessary to support LTE
Advanced modems based on the Qualcomm MDM9x30 chipset. A big thanks to
Sierra Wireless for providing MC7455 samples for testing

The most important change is the "raw-ip" support. The series also
adds a necessary control request, removes an unsupported device ID,
and adds a driver specific entry in MAINTAINERS.

A few random notes about "raw-ip":

"I rather have these all running in raw IP mode. The 802.3 framing is
utterly stupid." - Marcel Holtmann in Jan 2012 [1]

Marcel was right.  I should have listened to him. What more can I say?

The 802.3 framing has provided a steady supply of firmware bugs for
many years. We've added driver workarounds for many of these, but
there are still known bugs where the workaround is so yucky that we
have refused to apply it. But all that is over now.  The latest
generation Qualcomm chips no longer supports 802.3 framing at all.

I had two open questions regarding the "raw-ip" userspace API:

1) Should we continue faking an ethernet device, even if we don't use
   the L2 headers on the USB link anymore?

   There was a vote in favour of the "headerless" device. This is the
   honest representation of the hardware/firmware interface.

2) What input should the driver base its framing on?

   Snooping or directly manipulating QMI is considered out of the
   question. We delegated all QMI handling to userspace from the
   beginning.

   We have so far required userspace to configure the firmware for
   "802.3" framing, or fail if that proved impossible.  This
   requirement is now changed.  Userspace must now inform the driver
   if it negotiates "raw-ip" framing.  Two alternative interfaces were
   proposed:
    - ethtool private driver flag, or
    - sysfs file

   The NetworkManager/ModemManager developers were in favour of the
   sysfs alternative.

These questions (or any other you migh have :) are of course still
open.  This patch set presents the solutions I currently prefer,
considering the above.

All comments are appreciated, even simple '+1' ones.


Bjørn

[1] http://www.spinics.net/lists/linux-usb/msg57056.html


Bjørn Mork (6):
  net: qmi_wwan: MDM9x30 specific power management
  net: qmi_wwan: remove 1199:9070 device id
  usbnet: allow mini-drivers to consume L2 headers
  net: qmi_wwan: support "raw IP" mode
  net: qmi_wwan: document the qmi/raw_ip sysfs file
  MAINTAINERS: add qmi_wwan driver entry

 Documentation/ABI/testing/sysfs-class-net-qmi |  23 +++++
 MAINTAINERS                                   |   7 ++
 drivers/net/usb/qmi_wwan.c                    | 138 +++++++++++++++++++++++++-
 drivers/net/usb/usbnet.c                      |   5 +-
 4 files changed, 169 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-qmi

-- 
2.1.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/6] net: qmi_wwan: MDM9x30 specific power management
  2015-12-03 18:24 [PATCH 0/6] net: qmi_wwan: MDM9x30 support Bjørn Mork
@ 2015-12-03 18:24 ` Bjørn Mork
  2015-12-03 18:24 ` [PATCH 2/6] net: qmi_wwan: remove 1199:9070 device id Bjørn Mork
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Bjørn Mork @ 2015-12-03 18:24 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Marcel Holtmann, Oliver Neukum, Aleksander Morgado,
	Dan Williams, Bjørn Mork

MDM9x30 based modems appear to go into a deeper sleep when
suspended without "Remote Wakeup" enabled.  The QMI interface
will not respond unless a "set DTR" control request is sent
on resume. The effect is similar to a QMI_CTL SYNC request,
resetting (some of) the firmware state.

We allow userspace sessions to span multiple character device
open/close sequences.  This means that userspace can depend
on firmware state while both the netdev and the character
device are closed.  We have disabled "needs_remote_wakeup" at
this point to allow devices without remote wakeup support to
be auto-suspended.

To make sure the MDM9x30 keeps firmware state, we need to
keep "needs_remote_wakeup" always set. We also need to
issue a "set DTR" request to enable the QMI interface.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 9a5be8b85186..fc9dd452a3b5 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -223,6 +223,20 @@ err:
 	return rv;
 }
 
+/* Send CDC SetControlLineState request, setting or clearing the DTR.
+ * "Required for Autoconnect and 9x30 to wake up" according to the
+ * GobiNet driver. The requirement has been verified on an MDM9230
+ * based Sierra Wireless MC7455
+ */
+static int qmi_wwan_change_dtr(struct usbnet *dev, bool on)
+{
+	u8 intf = dev->intf->cur_altsetting->desc.bInterfaceNumber;
+
+	return usbnet_write_cmd(dev, USB_CDC_REQ_SET_CONTROL_LINE_STATE,
+				USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+				on ? 0x01 : 0x00, intf, NULL, 0);
+}
+
 static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int status = -1;
@@ -280,6 +294,24 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 		usb_driver_release_interface(driver, info->data);
 	}
 
+	/* disabling remote wakeup on MDM9x30 devices has the same
+	 * effect as clearing DTR. The device will not respond to QMI
+	 * requests until we set DTR again.  This is similar to a
+	 * QMI_CTL SYNC request, clearing a lot of firmware state
+	 * including the client ID allocations.
+	 *
+	 * Our usage model allows a session to span multiple
+	 * open/close events, so we must prevent the firmware from
+	 * clearing out state the clients might need.
+	 *
+	 * MDM9x30 is the first QMI chipset with USB3 support. Abuse
+	 * this fact to enable the quirk.
+	 */
+	if (le16_to_cpu(dev->udev->descriptor.bcdUSB) >= 0x0201) {
+		qmi_wwan_manage_power(dev, 1);
+		qmi_wwan_change_dtr(dev, true);
+	}
+
 	/* Never use the same address on both ends of the link, even if the
 	 * buggy firmware told us to. Or, if device is assigned the well-known
 	 * buggy firmware MAC address, replace it with a random address,
@@ -307,6 +339,12 @@ static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *intf)
 	if (info->subdriver && info->subdriver->disconnect)
 		info->subdriver->disconnect(info->control);
 
+	/* disable MDM9x30 quirk */
+	if (le16_to_cpu(dev->udev->descriptor.bcdUSB) >= 0x0201) {
+		qmi_wwan_change_dtr(dev, false);
+		qmi_wwan_manage_power(dev, 0);
+	}
+
 	/* allow user to unbind using either control or data */
 	if (intf == info->control)
 		other = info->data;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/6] net: qmi_wwan: remove 1199:9070 device id
  2015-12-03 18:24 [PATCH 0/6] net: qmi_wwan: MDM9x30 support Bjørn Mork
  2015-12-03 18:24 ` [PATCH 1/6] net: qmi_wwan: MDM9x30 specific power management Bjørn Mork
@ 2015-12-03 18:24 ` Bjørn Mork
  2015-12-03 18:24 ` [PATCH 3/6] usbnet: allow mini-drivers to consume L2 headers Bjørn Mork
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Bjørn Mork @ 2015-12-03 18:24 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Marcel Holtmann, Oliver Neukum, Aleksander Morgado,
	Dan Williams, Bjørn Mork

This turned out to be a bootloader device ID.  No need for
that in this driver.  It will only provide a single serial
function.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index fc9dd452a3b5..e3727b66d850 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -753,8 +753,6 @@ static const struct usb_device_id products[] = {
 	{QMI_FIXED_INTF(0x1199, 0x9056, 8)},	/* Sierra Wireless Modem */
 	{QMI_FIXED_INTF(0x1199, 0x9057, 8)},
 	{QMI_FIXED_INTF(0x1199, 0x9061, 8)},	/* Sierra Wireless Modem */
-	{QMI_FIXED_INTF(0x1199, 0x9070, 8)},	/* Sierra Wireless MC74xx/EM74xx */
-	{QMI_FIXED_INTF(0x1199, 0x9070, 10)},	/* Sierra Wireless MC74xx/EM74xx */
 	{QMI_FIXED_INTF(0x1199, 0x9071, 8)},	/* Sierra Wireless MC74xx/EM74xx */
 	{QMI_FIXED_INTF(0x1199, 0x9071, 10)},	/* Sierra Wireless MC74xx/EM74xx */
 	{QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},	/* Telekom Speedstick LTE II (Alcatel One Touch L100V LTE) */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/6] usbnet: allow mini-drivers to consume L2 headers
  2015-12-03 18:24 [PATCH 0/6] net: qmi_wwan: MDM9x30 support Bjørn Mork
  2015-12-03 18:24 ` [PATCH 1/6] net: qmi_wwan: MDM9x30 specific power management Bjørn Mork
  2015-12-03 18:24 ` [PATCH 2/6] net: qmi_wwan: remove 1199:9070 device id Bjørn Mork
@ 2015-12-03 18:24 ` Bjørn Mork
  2015-12-03 20:03   ` Oliver Neukum
  2015-12-03 18:24 ` [PATCH 4/6] net: qmi_wwan: support "raw IP" mode Bjørn Mork
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Bjørn Mork @ 2015-12-03 18:24 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Marcel Holtmann, Oliver Neukum, Aleksander Morgado,
	Dan Williams, Bjørn Mork

Assume the minidriver has taken care of all L2 header parsing
if it sets skb->protocol.  This allows the minidriver to
support non-ethernet L2 headers, and even operate without
any L2 header at all.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/usbnet.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 0744bf2ef2d6..0b0ba7ef14e4 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -324,7 +324,10 @@ void usbnet_skb_return (struct usbnet *dev, struct sk_buff *skb)
 		return;
 	}
 
-	skb->protocol = eth_type_trans (skb, dev->net);
+	/* only update if unset to allow minidriver rx_fixup override */
+	if (skb->protocol == 0)
+		skb->protocol = eth_type_trans (skb, dev->net);
+
 	dev->net->stats.rx_packets++;
 	dev->net->stats.rx_bytes += skb->len;
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/6] net: qmi_wwan: support "raw IP" mode
  2015-12-03 18:24 [PATCH 0/6] net: qmi_wwan: MDM9x30 support Bjørn Mork
                   ` (2 preceding siblings ...)
  2015-12-03 18:24 ` [PATCH 3/6] usbnet: allow mini-drivers to consume L2 headers Bjørn Mork
@ 2015-12-03 18:24 ` Bjørn Mork
  2015-12-03 18:24 ` [PATCH 6/6] MAINTAINERS: add qmi_wwan driver entry Bjørn Mork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Bjørn Mork @ 2015-12-03 18:24 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Marcel Holtmann, Oliver Neukum, Aleksander Morgado,
	Dan Williams, Bjørn Mork

QMI wwan devices have traditionally emulated ethernet devices
by default. But they have always had the capability of operating
without any L2 header at all, transmitting and receiving "raw"
IP packets over the USB link.  This firmware feature used to be
configurable through the QMI management protocol.

Traditionally there was no way to verify the firmware mode
without attempting to change it.  And the firmware would often
disallow changes anyway, i.e. due to a session already being
established.  In some cases, this could be a hidden firmware
internal session, completely outside host control.  For these
reasons, sticking with the "well known" default mode was safest.

But newer generations of QMI hardware and firmware have moved
towards defaulting to "raw IP" mode instead, followed by an
increasing number of bugs in the already buggy "802.3" firmware
implementation. At the same time, the QMI management protocol
gained the ability to detect the current mode.  This has enabled
the userspace QMI management application to verify the current
firmware mode without trying to modify it.

Following this development, the latest QMI hardware and firmware
(the MDM9x30 generation) has dropped support for "802.3" mode
entirely. Support for "raw IP" framing in the driver is therefore
necessary for these devices, and to a certain degree to work
around problems with the previous generation,

This patch adds support for "raw IP" framing for QMI devices,
changing the netdev from an ethernet device to an ARPHRD_NONE
p-t-p device when "raw IP" framing is enabled.

The firmware setup is fully delegated to the QMI userspace
management application, through simple tunneling of the QMI
protocol. The driver will therefore not know which mode has been
"negotiated" between firmware and userspace. Allowing userspace
to inform the driver of the result through a sysfs switch is
considered a better alternative than to change the well established
clean delegation of firmware management to userspace.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c | 98 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index e3727b66d850..98add3bf8821 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -14,6 +14,7 @@
 #include <linux/netdevice.h>
 #include <linux/ethtool.h>
 #include <linux/etherdevice.h>
+#include <linux/if_arp.h>
 #include <linux/mii.h>
 #include <linux/usb.h>
 #include <linux/usb/cdc.h>
@@ -48,11 +49,93 @@
 struct qmi_wwan_state {
 	struct usb_driver *subdriver;
 	atomic_t pmcount;
-	unsigned long unused;
+	unsigned long flags;
 	struct usb_interface *control;
 	struct usb_interface *data;
 };
 
+enum qmi_wwan_flags {
+	QMI_WWAN_FLAG_RAWIP = 1 << 0,
+};
+
+static void qmi_wwan_netdev_setup(struct net_device *net)
+{
+	struct usbnet *dev = netdev_priv(net);
+	struct qmi_wwan_state *info = (void *)&dev->data;
+
+	if (info->flags & QMI_WWAN_FLAG_RAWIP) {
+		net->header_ops      = NULL;  /* No header */
+		net->type            = ARPHRD_NONE;
+		net->hard_header_len = 0;
+		net->addr_len        = 0;
+		net->flags           = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
+		netdev_dbg(net, "mode: raw IP\n");
+	} else if (!net->header_ops) { /* don't bother if already set */
+		ether_setup(net);
+		netdev_dbg(net, "mode: Ethernet\n");
+	}
+
+	/* recalculate buffers after changing hard_header_len */
+	usbnet_change_mtu(net, net->mtu);
+}
+
+static ssize_t raw_ip_show(struct device *d, struct device_attribute *attr, char *buf)
+{
+	struct usbnet *dev = netdev_priv(to_net_dev(d));
+	struct qmi_wwan_state *info = (void *)&dev->data;
+
+	return sprintf(buf, "%c\n", info->flags & QMI_WWAN_FLAG_RAWIP ? 'Y' : 'N');
+}
+
+static ssize_t raw_ip_store(struct device *d,  struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct usbnet *dev = netdev_priv(to_net_dev(d));
+	struct qmi_wwan_state *info = (void *)&dev->data;
+	bool enable;
+	int err;
+
+	if (strtobool(buf, &enable))
+		return -EINVAL;
+
+	/* no change? */
+	if (enable == (info->flags & QMI_WWAN_FLAG_RAWIP))
+		return len;
+
+	/* we don't want to modify a running netdev */
+	if (netif_running(dev->net)) {
+		netdev_err(dev->net, "Cannot change a running device\n");
+		return -EBUSY;
+	}
+
+	/* let other drivers deny the change */
+	err = call_netdevice_notifiers(NETDEV_PRE_TYPE_CHANGE, dev->net);
+	err = notifier_to_errno(err);
+	if (err) {
+		netdev_err(dev->net, "Type change was refused\n");
+		return err;
+	}
+
+	if (enable)
+		info->flags |= QMI_WWAN_FLAG_RAWIP;
+	else
+		info->flags &= ~QMI_WWAN_FLAG_RAWIP;
+	qmi_wwan_netdev_setup(dev->net);
+	call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE, dev->net);
+	return len;
+}
+
+static DEVICE_ATTR_RW(raw_ip);
+
+static struct attribute *qmi_wwan_sysfs_attrs[] = {
+	&dev_attr_raw_ip.attr,
+	NULL,
+};
+
+static struct attribute_group qmi_wwan_sysfs_attr_group = {
+	.name = "qmi",
+	.attrs = qmi_wwan_sysfs_attrs,
+};
+
 /* default ethernet address used by the modem */
 static const u8 default_modem_addr[ETH_ALEN] = {0x02, 0x50, 0xf3};
 
@@ -80,6 +163,8 @@ static const u8 buggy_fw_addr[ETH_ALEN] = {0x00, 0xa0, 0xc6, 0x00, 0x00, 0x00};
  */
 static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
+	struct qmi_wwan_state *info = (void *)&dev->data;
+	bool rawip = info->flags & QMI_WWAN_FLAG_RAWIP;
 	__be16 proto;
 
 	/* This check is no longer done by usbnet */
@@ -94,15 +179,25 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 		proto = htons(ETH_P_IPV6);
 		break;
 	case 0x00:
+		if (rawip)
+			return 0;
 		if (is_multicast_ether_addr(skb->data))
 			return 1;
 		/* possibly bogus destination - rewrite just in case */
 		skb_reset_mac_header(skb);
 		goto fix_dest;
 	default:
+		if (rawip)
+			return 0;
 		/* pass along other packets without modifications */
 		return 1;
 	}
+	if (rawip) {
+		skb->dev = dev->net; /* normally set by eth_type_trans */
+		skb->protocol = proto;
+		return 1;
+	}
+
 	if (skb_headroom(skb) < ETH_HLEN)
 		return 0;
 	skb_push(skb, ETH_HLEN);
@@ -326,6 +421,7 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->net->dev_addr[0] &= 0xbf;	/* clear "IP" bit */
 	}
 	dev->net->netdev_ops = &qmi_wwan_netdev_ops;
+	dev->net->sysfs_groups[0] = &qmi_wwan_sysfs_attr_group;
 err:
 	return status;
 }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/6] net: qmi_wwan: document the qmi/raw_ip sysfs file
       [not found] ` <1449167063-22703-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
@ 2015-12-03 18:24   ` Bjørn Mork
  2015-12-04 21:56   ` [PATCH 0/6] net: qmi_wwan: MDM9x30 support David Miller
  1 sibling, 0 replies; 11+ messages in thread
From: Bjørn Mork @ 2015-12-03 18:24 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Marcel Holtmann, Oliver Neukum,
	Aleksander Morgado, Dan Williams, Bjørn Mork

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-class-net-qmi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-qmi

diff --git a/Documentation/ABI/testing/sysfs-class-net-qmi b/Documentation/ABI/testing/sysfs-class-net-qmi
new file mode 100644
index 000000000000..d240b7bf6587
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-qmi
@@ -0,0 +1,23 @@
+What:		/sys/class/net/<iface>/qmi/raw_ip
+Date:		Dec 2015
+KernelVersion:	4.4
+Contact:	Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
+Description:
+		Boolean.  Default: 'N'
+
+		Set this to 'Y' to change the network device link
+		framing from '802.3' to 'raw-ip'.
+
+		The netdev will change to reflect the link framing
+		mode.  The netdev is an ordinary ethernet device in
+		'802.3' mode, and the driver expects to exchange
+		frames with an ethernet header over the USB link. The
+		netdev is a headerless p-t-p device in 'raw-ip' mode,
+		and the driver expects to echange IPv4 or IPv6 packets
+		without any L2 header over the USB link.
+
+		Userspace is in full control of firmware configuration
+		through the delegation of the QMI protocol. Userspace
+		is responsible for coordination of driver and firmware
+		link framing mode, changing this setting to 'Y' if the
+		firmware is configured for 'raw-ip' mode.
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/6] MAINTAINERS: add qmi_wwan driver entry
  2015-12-03 18:24 [PATCH 0/6] net: qmi_wwan: MDM9x30 support Bjørn Mork
                   ` (3 preceding siblings ...)
  2015-12-03 18:24 ` [PATCH 4/6] net: qmi_wwan: support "raw IP" mode Bjørn Mork
@ 2015-12-03 18:24 ` Bjørn Mork
  2015-12-03 19:36 ` [PATCH 0/6] net: qmi_wwan: MDM9x30 support Dan Williams
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Bjørn Mork @ 2015-12-03 18:24 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Marcel Holtmann, Oliver Neukum, Aleksander Morgado,
	Dan Williams, Bjørn Mork

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ea1751283b49..e9d444a88e48 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11159,6 +11159,13 @@ L:	linux-usb@vger.kernel.org
 S:	Supported
 F:	drivers/usb/class/usblp.c
 
+USB QMI WWAN NETWORK DRIVER
+M:	Bjørn Mork <bjorn@mork.no>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	Documentation/ABI/testing/sysfs-class-net-qmi
+F:	drivers/net/usb/qmi_wwan.c
+
 USB RTL8150 DRIVER
 M:	Petko Manolov <petkan@nucleusys.com>
 L:	linux-usb@vger.kernel.org
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/6] net: qmi_wwan: MDM9x30 support
  2015-12-03 18:24 [PATCH 0/6] net: qmi_wwan: MDM9x30 support Bjørn Mork
                   ` (4 preceding siblings ...)
  2015-12-03 18:24 ` [PATCH 6/6] MAINTAINERS: add qmi_wwan driver entry Bjørn Mork
@ 2015-12-03 19:36 ` Dan Williams
  2015-12-04  9:56 ` Aleksander Morgado
       [not found] ` <1449167063-22703-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
  7 siblings, 0 replies; 11+ messages in thread
From: Dan Williams @ 2015-12-03 19:36 UTC (permalink / raw)
  To: Bjørn Mork, netdev
  Cc: linux-usb, Marcel Holtmann, Oliver Neukum, Aleksander Morgado

On Thu, 2015-12-03 at 19:24 +0100, Bjørn Mork wrote:
> We add new device IDs all the time, often without any testing on
> actual hardware. This is usually OK as long as the device is similar
> to already supported devices, using the same chipset and firmware
> basis.  But the Sierra Wireless MC7455 is an example of a new chipset
> generation. Adding it based on assumed similarity with its ancestors
> proved too optimistic.
> 
> This series adds the missing bits and pieces necessary to support LTE
> Advanced modems based on the Qualcomm MDM9x30 chipset. A big thanks
> to
> Sierra Wireless for providing MC7455 samples for testing
> 
> The most important change is the "raw-ip" support. The series also
> adds a necessary control request, removes an unsupported device ID,
> and adds a driver specific entry in MAINTAINERS.
> 
> A few random notes about "raw-ip":
> 
> "I rather have these all running in raw IP mode. The 802.3 framing is
> utterly stupid." - Marcel Holtmann in Jan 2012 [1]
> 
> Marcel was right.  I should have listened to him. What more can I
> say?

The decision was less clear-cut at the time, since all the devices did
support 802.3 framing and DHCP.  And people wanted easy-1-2-3 DHCP and
bridging capability too.  We still get a lot of people asking about
issues with DHCP and even bridging.  802.3 makes it *look* simple but
of course we know it's not that simple...

> The 802.3 framing has provided a steady supply of firmware bugs for
> many years. We've added driver workarounds for many of these, but
> there are still known bugs where the workaround is so yucky that we
> have refused to apply it. But all that is over now.  The latest
> generation Qualcomm chips no longer supports 802.3 framing at all.
> 
> I had two open questions regarding the "raw-ip" userspace API:
> 
> 1) Should we continue faking an ethernet device, even if we don't use
>    the L2 headers on the USB link anymore?
> 
>    There was a vote in favour of the "headerless" device. This is the
>    honest representation of the hardware/firmware interface.

I like the approach of the current patchset where it's more like a tun
device.  Simple.

> 2) What input should the driver base its framing on?
> 
>    Snooping or directly manipulating QMI is considered out of the
>    question. We delegated all QMI handling to userspace from the
>    beginning.
> 
>    We have so far required userspace to configure the firmware for
>    "802.3" framing, or fail if that proved impossible.  This
>    requirement is now changed.  Userspace must now inform the driver
>    if it negotiates "raw-ip" framing.  Two alternative interfaces
> were
>    proposed:
>     - ethtool private driver flag, or
>     - sysfs file
> 
>    The NetworkManager/ModemManager developers were in favour of the
>    sysfs alternative.

Sysfs is the easiest for most things to touch; ethtool requires being
able to do ioctls and bit operations or shell out to ethtool.  Just
stating the reasons for my above vote.

Dan

> These questions (or any other you migh have :) are of course still
> open.  This patch set presents the solutions I currently prefer,
> considering the above.
> 
> All comments are appreciated, even simple '+1' ones.
> 
> 
> Bjørn
> 
> [1] http://www.spinics.net/lists/linux-usb/msg57056.html
> 
> 
> Bjørn Mork (6):
>   net: qmi_wwan: MDM9x30 specific power management
>   net: qmi_wwan: remove 1199:9070 device id
>   usbnet: allow mini-drivers to consume L2 headers
>   net: qmi_wwan: support "raw IP" mode
>   net: qmi_wwan: document the qmi/raw_ip sysfs file
>   MAINTAINERS: add qmi_wwan driver entry
> 
>  Documentation/ABI/testing/sysfs-class-net-qmi |  23 +++++
>  MAINTAINERS                                   |   7 ++
>  drivers/net/usb/qmi_wwan.c                    | 138
> +++++++++++++++++++++++++-
>  drivers/net/usb/usbnet.c                      |   5 +-
>  4 files changed, 169 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-net-qmi
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/6] usbnet: allow mini-drivers to consume L2 headers
  2015-12-03 18:24 ` [PATCH 3/6] usbnet: allow mini-drivers to consume L2 headers Bjørn Mork
@ 2015-12-03 20:03   ` Oliver Neukum
  0 siblings, 0 replies; 11+ messages in thread
From: Oliver Neukum @ 2015-12-03 20:03 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: netdev, Aleksander Morgado, Marcel Holtmann, Dan Williams,
	linux-usb

On Thu, 2015-12-03 at 19:24 +0100, Bjørn Mork wrote:
> Assume the minidriver has taken care of all L2 header parsing
> if it sets skb->protocol.  This allows the minidriver to
> support non-ethernet L2 headers, and even operate without
> any L2 header at all.
> 
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
Acked-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/net/usb/usbnet.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 0744bf2ef2d6..0b0ba7ef14e4 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -324,7 +324,10 @@ void usbnet_skb_return (struct usbnet *dev, struct sk_buff *skb)
>  		return;
>  	}
>  
> -	skb->protocol = eth_type_trans (skb, dev->net);
> +	/* only update if unset to allow minidriver rx_fixup override */
> +	if (skb->protocol == 0)
> +		skb->protocol = eth_type_trans (skb, dev->net);
> +
>  	dev->net->stats.rx_packets++;
>  	dev->net->stats.rx_bytes += skb->len;
>  

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/6] net: qmi_wwan: MDM9x30 support
  2015-12-03 18:24 [PATCH 0/6] net: qmi_wwan: MDM9x30 support Bjørn Mork
                   ` (5 preceding siblings ...)
  2015-12-03 19:36 ` [PATCH 0/6] net: qmi_wwan: MDM9x30 support Dan Williams
@ 2015-12-04  9:56 ` Aleksander Morgado
       [not found] ` <1449167063-22703-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
  7 siblings, 0 replies; 11+ messages in thread
From: Aleksander Morgado @ 2015-12-04  9:56 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: netdev@vger.kernel.org, linux-usb, Marcel Holtmann, Oliver Neukum,
	Dan Williams

On Thu, Dec 3, 2015 at 7:24 PM, Bjørn Mork <bjorn@mork.no> wrote:
> We add new device IDs all the time, often without any testing on
> actual hardware. This is usually OK as long as the device is similar
> to already supported devices, using the same chipset and firmware
> basis.  But the Sierra Wireless MC7455 is an example of a new chipset
> generation. Adding it based on assumed similarity with its ancestors
> proved too optimistic.
>
> This series adds the missing bits and pieces necessary to support LTE
> Advanced modems based on the Qualcomm MDM9x30 chipset. A big thanks to
> Sierra Wireless for providing MC7455 samples for testing
>
> The most important change is the "raw-ip" support. The series also
> adds a necessary control request, removes an unsupported device ID,
> and adds a driver specific entry in MAINTAINERS.
>
> A few random notes about "raw-ip":
>
> "I rather have these all running in raw IP mode. The 802.3 framing is
> utterly stupid." - Marcel Holtmann in Jan 2012 [1]
>
> Marcel was right.  I should have listened to him. What more can I say?
>
> The 802.3 framing has provided a steady supply of firmware bugs for
> many years. We've added driver workarounds for many of these, but
> there are still known bugs where the workaround is so yucky that we
> have refused to apply it. But all that is over now.  The latest
> generation Qualcomm chips no longer supports 802.3 framing at all.
>
> I had two open questions regarding the "raw-ip" userspace API:
>
> 1) Should we continue faking an ethernet device, even if we don't use
>    the L2 headers on the USB link anymore?
>
>    There was a vote in favour of the "headerless" device. This is the
>    honest representation of the hardware/firmware interface.
>
> 2) What input should the driver base its framing on?
>
>    Snooping or directly manipulating QMI is considered out of the
>    question. We delegated all QMI handling to userspace from the
>    beginning.
>
>    We have so far required userspace to configure the firmware for
>    "802.3" framing, or fail if that proved impossible.  This
>    requirement is now changed.  Userspace must now inform the driver
>    if it negotiates "raw-ip" framing.  Two alternative interfaces were
>    proposed:
>     - ethtool private driver flag, or
>     - sysfs file
>
>    The NetworkManager/ModemManager developers were in favour of the
>    sysfs alternative.
>
> These questions (or any other you migh have :) are of course still
> open.  This patch set presents the solutions I currently prefer,
> considering the above.
>
> All comments are appreciated, even simple '+1' ones.
>

+1 from me as well.

We still need to decide how to manage this in userspace once the
kernel part is ready. For all pre-WDA devices, I'd use 802-3 as that
is what we've all been using. For WDA capable devices, we should
likely query what the current data link layer mode is, and ask the
kernel to use that one via sysfs; or, default to raw-ip in those,
don't know.

-- 
Aleksander
https://aleksander.es

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/6] net: qmi_wwan: MDM9x30 support
       [not found] ` <1449167063-22703-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
  2015-12-03 18:24   ` [PATCH 5/6] net: qmi_wwan: document the qmi/raw_ip sysfs file Bjørn Mork
@ 2015-12-04 21:56   ` David Miller
  1 sibling, 0 replies; 11+ messages in thread
From: David Miller @ 2015-12-04 21:56 UTC (permalink / raw)
  To: bjorn-yOkvZcmFvRU
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	marcel-kz+m5ild9QBg9hUCZPvPmw, oneukum-IBi9RG/b67k,
	aleksander-Dvg4H30XQSRVIjRurl1/8g, dcbw-H+wXaHxf7aLQT0dZR+AlfA

From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Thu,  3 Dec 2015 19:24:17 +0100

> We add new device IDs all the time, often without any testing on
> actual hardware. This is usually OK as long as the device is similar
> to already supported devices, using the same chipset and firmware
> basis.  But the Sierra Wireless MC7455 is an example of a new chipset
> generation. Adding it based on assumed similarity with its ancestors
> proved too optimistic.
> 
> This series adds the missing bits and pieces necessary to support LTE
> Advanced modems based on the Qualcomm MDM9x30 chipset. A big thanks to
> Sierra Wireless for providing MC7455 samples for testing

Series applied to net-next, thanks a lot.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-12-04 21:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-03 18:24 [PATCH 0/6] net: qmi_wwan: MDM9x30 support Bjørn Mork
2015-12-03 18:24 ` [PATCH 1/6] net: qmi_wwan: MDM9x30 specific power management Bjørn Mork
2015-12-03 18:24 ` [PATCH 2/6] net: qmi_wwan: remove 1199:9070 device id Bjørn Mork
2015-12-03 18:24 ` [PATCH 3/6] usbnet: allow mini-drivers to consume L2 headers Bjørn Mork
2015-12-03 20:03   ` Oliver Neukum
2015-12-03 18:24 ` [PATCH 4/6] net: qmi_wwan: support "raw IP" mode Bjørn Mork
2015-12-03 18:24 ` [PATCH 6/6] MAINTAINERS: add qmi_wwan driver entry Bjørn Mork
2015-12-03 19:36 ` [PATCH 0/6] net: qmi_wwan: MDM9x30 support Dan Williams
2015-12-04  9:56 ` Aleksander Morgado
     [not found] ` <1449167063-22703-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
2015-12-03 18:24   ` [PATCH 5/6] net: qmi_wwan: document the qmi/raw_ip sysfs file Bjørn Mork
2015-12-04 21:56   ` [PATCH 0/6] net: qmi_wwan: MDM9x30 support David Miller

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).