[RFCv3 01/13] net: usb: usbnet: use proper ep number macros

[RFCv3 01/13] net: usb: usbnet: use proper ep number macros

[Oliver Neukum]

We have macros to retrieve endpoint numbers now.
Use them.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/usbnet.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index ed86ba87ca4e..d1edb710a5ed 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -149,10 +149,8 @@ int usbnet_get_endpoints(struct usbnet *dev, struct usb_interface *intf)
 			return tmp;
 	}
 
-	dev->in = usb_rcvbulkpipe(dev->udev,
-				  in->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
-	dev->out = usb_sndbulkpipe(dev->udev,
-				   out->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
+	dev->in = usb_rcvbulkpipe(dev->udev, usb_endpoint_num(&in->desc));
+	dev->out = usb_sndbulkpipe(dev->udev, usb_endpoint_num(&out->desc));
 	dev->status = status;
 	return 0;
 }
@@ -232,9 +230,7 @@ static int init_status(struct usbnet *dev, struct usb_interface *intf)
 	if (!dev->driver_info->status)
 		return 0;
 
-	pipe = usb_rcvintpipe(dev->udev,
-			      dev->status->desc.bEndpointAddress
-			      & USB_ENDPOINT_NUMBER_MASK);
+	pipe = usb_rcvintpipe(dev->udev, usb_endpoint_num(&dev->status->desc));
 	maxp = usb_maxpacket(dev->udev, pipe);
 
 	/* avoid 1 msec chatter:  min 8 msec poll rate */
-- 
2.53.0

[RFCv3 02/13] net: usb: move updating filter and status from cdc to usbnet

[Oliver Neukum]

These helpers are used by multiple drivers and do not depend
on the rest of cdc. Leavin them in a cdc driver means that
more drivers are loaded just for infrastructure, not hardware
support.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/cdc_ether.c | 75 ------------------------------------
 drivers/net/usb/usbnet.c    | 76 +++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 75 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index a032c1ded406..3149fa2b6ac1 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -63,35 +63,6 @@ static const u8 mbm_guid[16] = {
 	0xa6, 0x07, 0xc0, 0xff, 0xcb, 0x7e, 0x39, 0x2a,
 };
 
-void usbnet_cdc_update_filter(struct usbnet *dev)
-{
-	struct net_device	*net = dev->net;
-
-	u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED
-			| USB_CDC_PACKET_TYPE_BROADCAST;
-
-	/* filtering on the device is an optional feature and not worth
-	 * the hassle so we just roughly care about snooping and if any
-	 * multicast is requested, we take every multicast
-	 */
-	if (net->flags & IFF_PROMISC)
-		cdc_filter |= USB_CDC_PACKET_TYPE_PROMISCUOUS;
-	if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI))
-		cdc_filter |= USB_CDC_PACKET_TYPE_ALL_MULTICAST;
-
-	usb_control_msg(dev->udev,
-			usb_sndctrlpipe(dev->udev, 0),
-			USB_CDC_SET_ETHERNET_PACKET_FILTER,
-			USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-			cdc_filter,
-			dev->intf->cur_altsetting->desc.bInterfaceNumber,
-			NULL,
-			0,
-			USB_CTRL_SET_TIMEOUT
-		);
-}
-EXPORT_SYMBOL_GPL(usbnet_cdc_update_filter);
-
 /* We need to override usbnet_*_link_ksettings in bind() */
 static const struct ethtool_ops cdc_ether_ethtool_ops = {
 	.get_link		= usbnet_get_link,
@@ -394,52 +365,6 @@ EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
  * (by Brad Hards) talked with, with more functionality.
  */
 
-static void speed_change(struct usbnet *dev, __le32 *speeds)
-{
-	dev->tx_speed = __le32_to_cpu(speeds[0]);
-	dev->rx_speed = __le32_to_cpu(speeds[1]);
-}
-
-void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
-{
-	struct usb_cdc_notification	*event;
-
-	if (urb->actual_length < sizeof(*event))
-		return;
-
-	/* SPEED_CHANGE can get split into two 8-byte packets */
-	if (test_and_clear_bit(EVENT_STS_SPLIT, &dev->flags)) {
-		speed_change(dev, (__le32 *) urb->transfer_buffer);
-		return;
-	}
-
-	event = urb->transfer_buffer;
-	switch (event->bNotificationType) {
-	case USB_CDC_NOTIFY_NETWORK_CONNECTION:
-		netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
-			  event->wValue ? "on" : "off");
-		if (netif_carrier_ok(dev->net) != !!event->wValue)
-			usbnet_link_change(dev, !!event->wValue, 0);
-		break;
-	case USB_CDC_NOTIFY_SPEED_CHANGE:	/* tx/rx rates */
-		netif_dbg(dev, timer, dev->net, "CDC: speed change (len %d)\n",
-			  urb->actual_length);
-		if (urb->actual_length != (sizeof(*event) + 8))
-			set_bit(EVENT_STS_SPLIT, &dev->flags);
-		else
-			speed_change(dev, (__le32 *) &event[1]);
-		break;
-	/* USB_CDC_NOTIFY_RESPONSE_AVAILABLE can happen too (e.g. RNDIS),
-	 * but there are no standard formats for the response data.
-	 */
-	default:
-		netdev_err(dev->net, "CDC: unexpected notification %02x!\n",
-			   event->bNotificationType);
-		break;
-	}
-}
-EXPORT_SYMBOL_GPL(usbnet_cdc_status);
-
 int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int				status;
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index d1edb710a5ed..906584cd71a4 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -22,6 +22,7 @@
 #include <linux/init.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
+#include <linux/usb/cdc.h>
 #include <linux/ctype.h>
 #include <linux/ethtool.h>
 #include <linux/workqueue.h>
@@ -2270,6 +2271,81 @@ int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
 
 }
 EXPORT_SYMBOL_GPL(usbnet_write_cmd_async);
+
+void usbnet_cdc_update_filter(struct usbnet *dev)
+{
+	struct net_device	*net = dev->net;
+
+	u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED
+			| USB_CDC_PACKET_TYPE_BROADCAST;
+
+	/* filtering on the device is an optional feature and not worth
+	 * the hassle so we just roughly care about snooping and if any
+	 * multicast is requested, we take every multicast
+	 */
+	if (net->flags & IFF_PROMISC)
+		cdc_filter |= USB_CDC_PACKET_TYPE_PROMISCUOUS;
+	if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI))
+		cdc_filter |= USB_CDC_PACKET_TYPE_ALL_MULTICAST;
+
+	usb_control_msg(dev->udev,
+			usb_sndctrlpipe(dev->udev, 0),
+			USB_CDC_SET_ETHERNET_PACKET_FILTER,
+			USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			cdc_filter,
+			dev->intf->cur_altsetting->desc.bInterfaceNumber,
+			NULL,
+			0,
+			USB_CTRL_SET_TIMEOUT
+		);
+}
+EXPORT_SYMBOL_GPL(usbnet_cdc_update_filter);
+
+static void speed_change(struct usbnet *dev, __le32 *speeds)
+{
+	dev->tx_speed = __le32_to_cpu(speeds[0]);
+	dev->rx_speed = __le32_to_cpu(speeds[1]);
+}
+
+void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
+{
+	struct usb_cdc_notification	*event;
+
+	if (urb->actual_length < sizeof(*event))
+		return;
+
+	/* SPEED_CHANGE can get split into two 8-byte packets */
+	if (test_and_clear_bit(EVENT_STS_SPLIT, &dev->flags)) {
+		speed_change(dev, (__le32 *) urb->transfer_buffer);
+		return;
+	}
+
+	event = urb->transfer_buffer;
+	switch (event->bNotificationType) {
+	case USB_CDC_NOTIFY_NETWORK_CONNECTION:
+		netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
+			  event->wValue ? "on" : "off");
+		if (netif_carrier_ok(dev->net) != !!event->wValue)
+			usbnet_link_change(dev, !!event->wValue, 0);
+		break;
+	case USB_CDC_NOTIFY_SPEED_CHANGE:	/* tx/rx rates */
+		netif_dbg(dev, timer, dev->net, "CDC: speed change (len %d)\n",
+			  urb->actual_length);
+		if (urb->actual_length != (sizeof(*event) + 8))
+			set_bit(EVENT_STS_SPLIT, &dev->flags);
+		else
+			speed_change(dev, (__le32 *) &event[1]);
+		break;
+	/* USB_CDC_NOTIFY_RESPONSE_AVAILABLE can happen too (e.g. RNDIS),
+	 * but there are no standard formats for the response data.
+	 */
+	default:
+		netdev_err(dev->net, "CDC: unexpected notification %02x!\n",
+			   event->bNotificationType);
+		break;
+	}
+}
+EXPORT_SYMBOL_GPL(usbnet_cdc_status);
 /*-------------------------------------------------------------------------*/
 
 static int __init usbnet_init(void)
-- 
2.53.0

[RFCv3 03/13] net: usb: centralize usbnet_cdc_zte_rx_fixup in usbnet

[Oliver Neukum]

This helper is used by multiple drivers using usbnet.
It is better to be provided by usbnet than one of them.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/cdc_ether.c | 19 -------------------
 drivers/net/usb/usbnet.c    | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 3149fa2b6ac1..505a653d9482 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -398,25 +398,6 @@ static int usbnet_cdc_zte_bind(struct usbnet *dev, struct usb_interface *intf)
 	return status;
 }
 
-/* Make sure packets have correct destination MAC address
- *
- * A firmware bug observed on some devices (ZTE MF823/831/910) is that the
- * device sends packets with a static, bogus, random MAC address (event if
- * device MAC address has been updated). Always set MAC address to that of the
- * device.
- */
-int usbnet_cdc_zte_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
-{
-	if (skb->len < ETH_HLEN || !(skb->data[0] & 0x02))
-		return 1;
-
-	skb_reset_mac_header(skb);
-	ether_addr_copy(eth_hdr(skb)->h_dest, dev->net->dev_addr);
-
-	return 1;
-}
-EXPORT_SYMBOL_GPL(usbnet_cdc_zte_rx_fixup);
-
 /* Ensure correct link state
  *
  * Some devices (ZTE MF823/831/910) export two carrier on notifications when
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 906584cd71a4..8a9e0d98d9c6 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -2346,6 +2346,25 @@ void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
 	}
 }
 EXPORT_SYMBOL_GPL(usbnet_cdc_status);
+
+/* Make sure packets have correct destination MAC address
+ *
+ * A firmware bug observed on some devices (ZTE MF823/831/910) is that the
+ * device sends packets with a static, bogus, random MAC address (event if
+ * device MAC address has been updated). Always set MAC address to that of the
+ * device.
+ */
+int usbnet_cdc_zte_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+	if (skb->len < ETH_HLEN || !(skb->data[0] & 0x02))
+		return 1;
+
+	skb_reset_mac_header(skb);
+	ether_addr_copy(eth_hdr(skb)->h_dest, dev->net->dev_addr);
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(usbnet_cdc_zte_rx_fixup);
 /*-------------------------------------------------------------------------*/
 
 static int __init usbnet_init(void)
-- 
2.53.0

[RFCv4 04/13] net: usb: usbnet: add access helper for private data

[Oliver Neukum]

Drivers should not need to go for the internals of
usbnet to find their private data.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 include/linux/usb/usbnet.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index b0e84896e6ac..4a97845be62c 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -206,6 +206,11 @@ extern int usbnet_write_cmd_nopm(struct usbnet *dev, u8 cmd, u8 reqtype,
 extern int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
 		    u16 value, u16 index, const void *data, u16 size);
 
+static inline void *usbnet_priv(struct usbnet *dev)
+{
+	return (void *)dev->data;
+}
+
 /* Drivers that reuse some of the standard USB CDC infrastructure
  * (notably, using multiple interfaces according to the CDC
  * union descriptor) get some helper code.
-- 
2.53.0

[RFCv5 05/13] net: usb: use accessor helper for usbnet private data

[Oliver Neukum]

Use the new accessor function.
No functional change intended.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/asix_common.c  |  2 +-
 drivers/net/usb/asix_devices.c | 16 ++++++++--------
 drivers/net/usb/ax88172a.c     |  2 +-
 drivers/net/usb/cdc_ether.c    | 12 +++---------
 drivers/net/usb/cdc_mbim.c     | 18 +++++++++---------
 5 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 4f03f4e57655..add6493d4001 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -441,7 +441,7 @@ int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
 void asix_set_multicast(struct net_device *net)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	u16 rx_ctl = AX_DEFAULT_RX_CTL;
 
 	if (net->flags & IFF_PROMISC) {
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index df0bcfedddbc..979ee0110822 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -118,7 +118,7 @@ static const struct ethtool_ops ax88172_ethtool_ops = {
 static void ax88172_set_multicast(struct net_device *net)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	u8 rx_ctl = 0x8c;
 
 	if (net->flags & IFF_PROMISC) {
@@ -331,7 +331,7 @@ static const struct ethtool_ops ax88772_ethtool_ops = {
 
 static int ax88772_reset(struct usbnet *dev)
 {
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	struct asix_common_private *priv = dev->driver_priv;
 	int ret;
 
@@ -361,7 +361,7 @@ static int ax88772_reset(struct usbnet *dev)
 
 static int ax88772_hw_reset(struct usbnet *dev, int in_pm)
 {
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	struct asix_common_private *priv = dev->driver_priv;
 	u16 rx_ctl;
 	int ret;
@@ -455,7 +455,7 @@ static int ax88772_hw_reset(struct usbnet *dev, int in_pm)
 
 static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
 {
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	struct asix_common_private *priv = dev->driver_priv;
 	u16 rx_ctl, phy14h, phy15h, phy16h;
 	int ret;
@@ -987,7 +987,7 @@ static const struct ethtool_ops ax88178_ethtool_ops = {
 
 static int marvell_phy_init(struct usbnet *dev)
 {
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	u16 reg;
 
 	netdev_dbg(dev->net, "marvell_phy_init()\n");
@@ -1018,7 +1018,7 @@ static int marvell_phy_init(struct usbnet *dev)
 
 static int rtl8211cl_phy_init(struct usbnet *dev)
 {
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 
 	netdev_dbg(dev->net, "rtl8211cl_phy_init()\n");
 
@@ -1065,7 +1065,7 @@ static int marvell_led_status(struct usbnet *dev, u16 speed)
 
 static int ax88178_reset(struct usbnet *dev)
 {
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	int ret;
 	__le16 eeprom;
 	u8 status;
@@ -1163,7 +1163,7 @@ static int ax88178_link_reset(struct usbnet *dev)
 {
 	u16 mode;
 	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	u32 speed;
 
 	netdev_dbg(dev->net, "ax88178_link_reset()\n");
diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index 6b14bce2a552..f9b4364e0a0f 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -260,7 +260,7 @@ static void ax88172a_unbind(struct usbnet *dev, struct usb_interface *intf)
 
 static int ax88172a_reset(struct usbnet *dev)
 {
-	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_data *data = usbnet_priv(dev);
 	struct ax88172a_private *priv = dev->driver_priv;
 	int ret;
 	u16 rx_ctl;
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 505a653d9482..c24c14702f4b 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -85,16 +85,13 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 	u8				*buf = intf->cur_altsetting->extra;
 	int				len = intf->cur_altsetting->extralen;
 	struct usb_interface_descriptor	*d;
-	struct cdc_state		*info = (void *) &dev->data;
+	struct cdc_state		*info = usbnet_priv(dev);
 	int				status;
 	int				rndis;
 	bool				android_rndis_quirk = false;
 	struct usb_driver		*driver = driver_of(intf);
 	struct usb_cdc_parsed_header header;
 
-	if (sizeof(dev->data) < sizeof(*info))
-		return -EDOM;
-
 	/* expect strict spec conformance for the descriptors, but
 	 * cope with firmware which stores them in the wrong place
 	 */
@@ -330,7 +327,7 @@ EXPORT_SYMBOL_GPL(usbnet_ether_cdc_bind);
 
 void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct cdc_state		*info = (void *) &dev->data;
+	struct cdc_state		*info = usbnet_priv(dev);
 	struct usb_driver		*driver = driver_of(intf);
 
 	/* combined interface - nothing  to do */
@@ -368,10 +365,7 @@ EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
 int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int				status;
-	struct cdc_state		*info = (void *) &dev->data;
-
-	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data)
-			< sizeof(struct cdc_state)));
+	struct cdc_state		*info = usbnet_priv(dev);
 
 	status = usbnet_ether_cdc_bind(dev, intf);
 	if (status < 0)
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index dbf01210b0e7..5522dc137d45 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -43,7 +43,7 @@ enum cdc_mbim_flags {
 /* using a counter to merge subdriver requests with our own into a combined state */
 static int cdc_mbim_manage_power(struct usbnet *dev, int on)
 {
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 	int rv = 0;
 
 	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);
@@ -72,7 +72,7 @@ static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status)
 static int cdc_mbim_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid)
 {
 	struct usbnet *dev = netdev_priv(netdev);
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 
 	/* creation of this VLAN is a request to tag IP session 0 */
 	if (vid == MBIM_IPS0_VID)
@@ -86,7 +86,7 @@ static int cdc_mbim_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid)
 static int cdc_mbim_rx_kill_vid(struct net_device *netdev, __be16 proto, u16 vid)
 {
 	struct usbnet *dev = netdev_priv(netdev);
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 
 	/* this is a request for an untagged IP session 0 */
 	if (vid == MBIM_IPS0_VID)
@@ -143,7 +143,7 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
 	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
 	int ret = -ENODEV;
 	u8 data_altsetting = 1;
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 
 	/* should we change control altsetting on a NCM/MBIM function? */
 	if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
@@ -195,7 +195,7 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
 
 static void cdc_mbim_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 	struct cdc_ncm_ctx *ctx = info->ctx;
 
 	/* disconnect subdriver from control interface */
@@ -221,7 +221,7 @@ static bool is_ip_proto(__be16 proto)
 static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
 	struct sk_buff *skb_out;
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 	struct cdc_ncm_ctx *ctx = info->ctx;
 	__le32 sign = cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN);
 	u16 tci = 0;
@@ -411,7 +411,7 @@ static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_
 static int cdc_mbim_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 	struct cdc_ncm_ctx *ctx = info->ctx;
 	int len;
 	int nframes;
@@ -506,7 +506,7 @@ static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	int ret = -ENODEV;
 	struct usbnet *dev = usb_get_intfdata(intf);
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 	struct cdc_ncm_ctx *ctx = info->ctx;
 
 	if (!ctx)
@@ -534,7 +534,7 @@ static int cdc_mbim_resume(struct usb_interface *intf)
 {
 	int  ret = 0;
 	struct usbnet *dev = usb_get_intfdata(intf);
-	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_mbim_state *info = usbnet_priv(dev);
 	struct cdc_ncm_ctx *ctx = info->ctx;
 	bool callsub = (intf == ctx->control && info->subdriver && info->subdriver->resume);
 
-- 
2.53.0

Re: [RFCv5 05/13] net: usb: use accessor helper for usbnet private data

[Bjørn Mork]

Oliver Neukum <oneukum@suse.com> writes:

> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
> index dbf01210b0e7..5522dc137d45 100644
> --- a/drivers/net/usb/cdc_mbim.c
> +++ b/drivers/net/usb/cdc_mbim.c
> @@ -43,7 +43,7 @@ enum cdc_mbim_flags {
>  /* using a counter to merge subdriver requests with our own into a combined state */
>  static int cdc_mbim_manage_power(struct usbnet *dev, int on)
>  {
> -	struct cdc_mbim_state *info = (void *)&dev->data;
> +	struct cdc_mbim_state *info = usbnet_priv(dev);
>  	int rv = 0;
>  
>  	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);


I'm not sure I'm following all the conversions, but you are aware that
cdc_mbim and cdc_ncm share a few functions struct cdc_ncm_ctx state, the
and therefore have to cooperate wrt the private data layout?

The 'struct cdc_mbim_state' has a ctx field which is supposed to be
managed by the cdc_ncm part of the driver:

struct cdc_mbim_state {
	struct cdc_ncm_ctx *ctx;
	atomic_t pmcount;
	struct usb_driver *subdriver;
	unsigned long _unused;
	unsigned long flags;
};

The cdc_mbim driver currently depends on cdc_ncm_bind_common(), which is
called from cdc_mbim_bind(), to allocate the 'struct cdc_ncm_ctx' data.
It will later call other functions, like cdc_ncm_fill_tx_frame(), which
depend on that 'struct cdc_ncm_ctx *ctx' field.

I don't understand how you make the patch above work in cooperation with
this:


@@ -1203,7 +1187,7 @@ static struct usb_cdc_ncm_ndp32 *cdc_ncm_ndp32(struct cdc_ncm_ctx *ctx, struct s
 struct sk_buff *
 cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	union {
 		struct usb_cdc_ncm_nth16 *nth16;
 		struct usb_cdc_ncm_nth32 *nth32;


Looks like the cdc_mbim driver will use the same usbnet_priv(dev) as
both 'struct cdc_ncm_ctx' and 'struct cdc_mbim_state'.

Yes, I know this too magic, and should be rewritten on so many levels.
But it is what it is.



Bjørn

Re: [RFCv5 05/13] net: usb: use accessor helper for usbnet private data

[Oliver Neukum]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

On 09.03.26 18:46, Bjørn Mork wrote:
> Oliver Neukum <oneukum@suse.com> writes:
> 
>> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
>> index dbf01210b0e7..5522dc137d45 100644
>> --- a/drivers/net/usb/cdc_mbim.c
>> +++ b/drivers/net/usb/cdc_mbim.c
>> @@ -43,7 +43,7 @@ enum cdc_mbim_flags {
>>   /* using a counter to merge subdriver requests with our own into a combined state */
>>   static int cdc_mbim_manage_power(struct usbnet *dev, int on)
>>   {
>> -	struct cdc_mbim_state *info = (void *)&dev->data;
>> +	struct cdc_mbim_state *info = usbnet_priv(dev);
>>   	int rv = 0;
>>   
>>   	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);
> 
> 
> I'm not sure I'm following all the conversions, but you are aware that
> cdc_mbim and cdc_ncm share a few functions struct cdc_ncm_ctx state, the
> and therefore have to cooperate wrt the private data layout?

Hi,

I was not aware of that. What do you think about the approach taken
in the attachment? It just makes sure that the first part of the
descriptors match.

	Regards
		Oliver


[-- Attachment #2: 0001-net-usb-cdc_ncm-use-dynamic-private-allocation.patch --]
[-- Type: text/x-patch, Size: 18463 bytes --]

From a3989540cffbe770254de0ce2abfe308a53a2028 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Tue, 24 Feb 2026 16:45:33 +0100
Subject: [PATCH] net: usb: cdc_ncm: use dynamic private allocation

The old style private room was too small for cdc_ncm.
With dynamic allocation that factor is gone.
Remove handling private allocations in the driver.

As cdc-mbim uses parts of cdc-ncm this necessitates
a change to mbim making cdc_nmcm's descriptor
the very first part of cdc_nmbim's descriptor to allow
a cast.

Signed-off-by: Oliver Neukum <oneukum@suse.com>

net: usb: including ctx as first element in mbim
---
 drivers/net/usb/cdc_mbim.c | 32 ++++++++--------
 drivers/net/usb/cdc_ncm.c  | 78 +++++++++++++++++---------------------
 2 files changed, 50 insertions(+), 60 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 5522dc137d45..4530332d2812 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -26,9 +26,13 @@
 /* alternative VLAN for IP session 0 if not untagged */
 #define MBIM_IPS0_VID	4094
 
-/* driver specific data - must match cdc_ncm usage */
+/*
+ * cdc-mbim shares code with cdc-ncm
+ */
+
 struct cdc_mbim_state {
-	struct cdc_ncm_ctx *ctx;
+	/* this must be the very first element */
+	struct cdc_ncm_ctx ctx;
 	atomic_t pmcount;
 	struct usb_driver *subdriver;
 	unsigned long _unused;
@@ -139,11 +143,11 @@ static int cdc_mbim_set_ctrlalt(struct usbnet *dev, struct usb_interface *intf,
 
 static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct cdc_ncm_ctx *ctx;
 	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
 	int ret = -ENODEV;
 	u8 data_altsetting = 1;
 	struct cdc_mbim_state *info = usbnet_priv(dev);
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)info;
 
 	/* should we change control altsetting on a NCM/MBIM function? */
 	if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
@@ -162,8 +166,6 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
 	if (ret)
 		goto err;
 
-	ctx = info->ctx;
-
 	/* The MBIM descriptor and the status endpoint are required */
 	if (ctx->mbim_desc && dev->status)
 		subdriver = usb_cdc_wdm_register(ctx->control,
@@ -196,7 +198,7 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
 static void cdc_mbim_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
 	struct cdc_mbim_state *info = usbnet_priv(dev);
-	struct cdc_ncm_ctx *ctx = info->ctx;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)info;
 
 	/* disconnect subdriver from control interface */
 	if (info->subdriver && info->subdriver->disconnect)
@@ -222,15 +224,12 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
 {
 	struct sk_buff *skb_out;
 	struct cdc_mbim_state *info = usbnet_priv(dev);
-	struct cdc_ncm_ctx *ctx = info->ctx;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)info;
 	__le32 sign = cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN);
 	u16 tci = 0;
 	bool is_ip;
 	u8 *c;
 
-	if (!ctx)
-		goto error;
-
 	if (skb) {
 		if (skb->len <= ETH_HLEN)
 			goto error;
@@ -412,7 +411,7 @@ static int cdc_mbim_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
 	struct cdc_mbim_state *info = usbnet_priv(dev);
-	struct cdc_ncm_ctx *ctx = info->ctx;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)info;
 	int len;
 	int nframes;
 	int x;
@@ -507,10 +506,7 @@ static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
 	int ret = -ENODEV;
 	struct usbnet *dev = usb_get_intfdata(intf);
 	struct cdc_mbim_state *info = usbnet_priv(dev);
-	struct cdc_ncm_ctx *ctx = info->ctx;
-
-	if (!ctx)
-		goto error;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)info;
 
 	/*
 	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
@@ -535,7 +531,7 @@ static int cdc_mbim_resume(struct usb_interface *intf)
 	int  ret = 0;
 	struct usbnet *dev = usb_get_intfdata(intf);
 	struct cdc_mbim_state *info = usbnet_priv(dev);
-	struct cdc_ncm_ctx *ctx = info->ctx;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)info;
 	bool callsub = (intf == ctx->control && info->subdriver && info->subdriver->resume);
 
 	if (callsub)
@@ -557,6 +553,7 @@ static const struct driver_info cdc_mbim_info = {
 	.manage_power = cdc_mbim_manage_power,
 	.rx_fixup = cdc_mbim_rx_fixup,
 	.tx_fixup = cdc_mbim_tx_fixup,
+	.required_room = sizeof(struct cdc_mbim_state),
 };
 
 /* MBIM and NCM devices should not need a ZLP after NTBs with
@@ -581,6 +578,7 @@ static const struct driver_info cdc_mbim_info_zlp = {
 	.manage_power = cdc_mbim_manage_power,
 	.rx_fixup = cdc_mbim_rx_fixup,
 	.tx_fixup = cdc_mbim_tx_fixup,
+	.required_room = sizeof(struct cdc_mbim_state),
 };
 
 /* The spefication explicitly allows NDPs to be placed anywhere in the
@@ -601,6 +599,7 @@ static const struct driver_info cdc_mbim_info_ndp_to_end = {
 	.rx_fixup = cdc_mbim_rx_fixup,
 	.tx_fixup = cdc_mbim_tx_fixup,
 	.data = CDC_NCM_FLAG_NDP_TO_END,
+	.required_room = sizeof(struct cdc_mbim_state),
 };
 
 /* Some modems (e.g. Telit LE922A6) do not work properly with altsetting
@@ -616,6 +615,7 @@ static const struct driver_info cdc_mbim_info_avoid_altsetting_toggle = {
 	.rx_fixup = cdc_mbim_rx_fixup,
 	.tx_fixup = cdc_mbim_tx_fixup,
 	.data = CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE,
+	.required_room = sizeof(struct cdc_mbim_state),
 };
 
 static const struct usb_device_id mbim_devs[] = {
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 7057c6c0cfc6..91da42a778f4 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -107,7 +107,7 @@ static void cdc_ncm_get_ethtool_stats(struct net_device *netdev,
 				    u64 *data)
 {
 	struct usbnet *dev = netdev_priv(netdev);
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	int i;
 	char *p = NULL;
 
@@ -149,7 +149,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
 
 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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u32 val, max, min;
 
 	/* clamp new_rx to sane values */
@@ -172,7 +172,7 @@ static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx)
 
 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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u32 val, max, min;
 
 	/* clamp new_tx to sane values */
@@ -202,7 +202,7 @@ static ssize_t min_tx_pkt_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%u\n", ctx->min_tx_pkt);
 }
@@ -211,7 +211,7 @@ static ssize_t rx_max_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%u\n", ctx->rx_max);
 }
@@ -220,7 +220,7 @@ static ssize_t tx_max_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%u\n", ctx->tx_max);
 }
@@ -229,7 +229,7 @@ static ssize_t tx_timer_usecs_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%u\n", ctx->timer_interval / (u32)NSEC_PER_USEC);
 }
@@ -239,7 +239,7 @@ static ssize_t min_tx_pkt_store(struct device *d,
 				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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	unsigned long val;
 
 	/* no need to restrict values - anything from 0 to infinity is OK */
@@ -255,7 +255,7 @@ static ssize_t rx_max_store(struct device *d,
 			    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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	unsigned long val;
 
 	if (kstrtoul(buf, 0, &val) || cdc_ncm_check_rx_max(dev, val) != val)
@@ -270,7 +270,7 @@ static ssize_t tx_max_store(struct device *d,
 			    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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	unsigned long val;
 
 	if (kstrtoul(buf, 0, &val) || cdc_ncm_check_tx_max(dev, val) != val)
@@ -285,7 +285,7 @@ static ssize_t tx_timer_usecs_store(struct device *d,
 				    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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	ssize_t ret;
 	unsigned long val;
 
@@ -311,7 +311,7 @@ static DEVICE_ATTR_RW(tx_timer_usecs);
 static ssize_t ndp_to_end_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%c\n", ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END ? 'Y' : 'N');
 }
@@ -319,7 +319,7 @@ static ssize_t ndp_to_end_show(struct device *d, struct device_attribute *attr,
 static ssize_t ndp_to_end_store(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	bool enable;
 
 	if (kstrtobool(buf, &enable))
@@ -361,7 +361,7 @@ static DEVICE_ATTR_RW(ndp_to_end);
 static ssize_t cdc_ncm_show_##name(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]; \
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev); \
 	return sprintf(buf, format "\n", tocpu(ctx->ncm_parm.name));	\
 } \
 static DEVICE_ATTR(name, 0444, cdc_ncm_show_##name, NULL)
@@ -404,7 +404,7 @@ static const struct attribute_group cdc_ncm_sysfs_attr_group = {
 /* 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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 	u32 val;
 
@@ -476,7 +476,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
 /* helpers for NCM and MBIM differences */
 static u8 cdc_ncm_flags(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	if (cdc_ncm_comm_intf_is_mbim(dev->intf->cur_altsetting) && ctx->mbim_desc)
 		return ctx->mbim_desc->bmNetworkCapabilities;
@@ -501,7 +501,7 @@ static u32 cdc_ncm_min_dgram_size(struct usbnet *dev)
 
 static u32 cdc_ncm_max_dgram_size(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	if (cdc_ncm_comm_intf_is_mbim(dev->intf->cur_altsetting) && ctx->mbim_desc)
 		return le16_to_cpu(ctx->mbim_desc->wMaxSegmentSize);
@@ -515,7 +515,7 @@ static u32 cdc_ncm_max_dgram_size(struct usbnet *dev)
  */
 static int cdc_ncm_init(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 	int err;
 
@@ -608,7 +608,7 @@ static int cdc_ncm_init(struct usbnet *dev)
 /* set a new max datagram size */
 static void cdc_ncm_set_dgram_size(struct usbnet *dev, int new_size)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 	__le16 max_datagram_size;
 	u16 mbim_mtu;
@@ -656,7 +656,7 @@ static void cdc_ncm_set_dgram_size(struct usbnet *dev, int new_size)
 
 static void cdc_ncm_fix_modulus(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u32 val;
 
 	/*
@@ -700,7 +700,7 @@ static void cdc_ncm_fix_modulus(struct usbnet *dev)
 
 static int cdc_ncm_setup(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u32 def_rx, def_tx;
 
 	/* be conservative when selecting initial buffer size to
@@ -769,9 +769,6 @@ cdc_ncm_find_endpoints(struct usbnet *dev, struct usb_interface *intf)
 
 static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
 {
-	if (ctx == NULL)
-		return;
-
 	if (ctx->tx_rem_skb != NULL) {
 		dev_kfree_skb_any(ctx->tx_rem_skb);
 		ctx->tx_rem_skb = NULL;
@@ -786,8 +783,6 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
 		kfree(ctx->delayed_ndp16);
 	else
 		kfree(ctx->delayed_ndp32);
-
-	kfree(ctx);
 }
 
 /* we need to override the usbnet change_mtu ndo for two reasons:
@@ -819,7 +814,7 @@ static const struct net_device_ops cdc_ncm_netdev_ops = {
 
 int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting, int drvflags)
 {
-	struct cdc_ncm_ctx *ctx;
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	struct usb_driver *driver;
 	u8 *buf;
 	int len;
@@ -827,10 +822,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	u8 iface_no;
 	struct usb_cdc_parsed_header hdr;
 
-	ctx = kzalloc_obj(*ctx);
-	if (!ctx)
-		return -ENOMEM;
-
 	ctx->dev = dev;
 
 	hrtimer_setup(&ctx->tx_timer, &cdc_ncm_tx_timer_cb, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
@@ -838,9 +829,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	atomic_set(&ctx->stop, 0);
 	spin_lock_init(&ctx->mtx);
 
-	/* store ctx pointer in device data field */
-	dev->data[0] = (unsigned long)ctx;
-
 	/* only the control interface can be successfully probed */
 	ctx->control = intf;
 
@@ -988,8 +976,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	if (ctx->data != ctx->control)
 		usb_driver_release_interface(driver, ctx->data);
 error:
-	cdc_ncm_free((struct cdc_ncm_ctx *)dev->data[0]);
-	dev->data[0] = 0;
+	cdc_ncm_free(usbnet_priv(dev));
 	dev_info(&intf->dev, "bind() failure\n");
 	return -ENODEV;
 }
@@ -997,12 +984,9 @@ EXPORT_SYMBOL_GPL(cdc_ncm_bind_common);
 
 void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	struct usb_driver *driver = driver_of(intf);
 
-	if (ctx == NULL)
-		return;		/* no setup */
-
 	atomic_set(&ctx->stop, 1);
 
 	hrtimer_cancel(&ctx->tx_timer);
@@ -1203,7 +1187,7 @@ static struct usb_cdc_ncm_ndp32 *cdc_ncm_ndp32(struct cdc_ncm_ctx *ctx, struct s
 struct sk_buff *
 cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	union {
 		struct usb_cdc_ncm_nth16 *nth16;
 		struct usb_cdc_ncm_nth32 *nth32;
@@ -1523,7 +1507,7 @@ struct sk_buff *
 cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
 	struct sk_buff *skb_out;
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	/*
 	 * The Ethernet API we are using does not support transmitting
@@ -1725,7 +1709,7 @@ EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp32);
 int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	unsigned int len;
 	int nframes;
 	int x;
@@ -1904,7 +1888,7 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 
 static void cdc_ncm_update_filter(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	if (ctx->filtering_supported)
 		usbnet_cdc_update_filter(dev);
@@ -1921,6 +1905,7 @@ static const struct driver_info cdc_ncm_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = cdc_ncm_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as cdc_ncm_info, but with FLAG_SEND_ZLP  */
@@ -1935,6 +1920,7 @@ static const struct driver_info cdc_ncm_zlp_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = cdc_ncm_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as cdc_ncm_info, but with FLAG_SEND_ZLP */
@@ -1949,6 +1935,7 @@ static const struct driver_info apple_tethering_interface_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = usbnet_cdc_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as apple_tethering_interface_info, but without FLAG_LINK_INTR */
@@ -1963,6 +1950,7 @@ static const struct driver_info apple_private_interface_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = usbnet_cdc_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as cdc_ncm_info, but with FLAG_WWAN */
@@ -1977,6 +1965,7 @@ static const struct driver_info wwan_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = cdc_ncm_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as wwan_info, but with FLAG_NOARP  */
@@ -1991,6 +1980,7 @@ static const struct driver_info wwan_noarp_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = cdc_ncm_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 static const struct usb_device_id cdc_devs[] = {
-- 
2.53.0

Re: [RFCv5 05/13] net: usb: use accessor helper for usbnet private data

[Bjørn Mork]

Oliver Neukum <oneukum@suse.com> writes:
> On 09.03.26 18:46, Bjørn Mork wrote:
>> Oliver Neukum <oneukum@suse.com> writes:
>> 
>>> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
>>> index dbf01210b0e7..5522dc137d45 100644
>>> --- a/drivers/net/usb/cdc_mbim.c
>>> +++ b/drivers/net/usb/cdc_mbim.c
>>> @@ -43,7 +43,7 @@ enum cdc_mbim_flags {
>>>   /* using a counter to merge subdriver requests with our own into a combined state */
>>>   static int cdc_mbim_manage_power(struct usbnet *dev, int on)
>>>   {
>>> -	struct cdc_mbim_state *info = (void *)&dev->data;
>>> +	struct cdc_mbim_state *info = usbnet_priv(dev);
>>>   	int rv = 0;
>>>     	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__,
>>> atomic_read(&info->pmcount), on);
>> I'm not sure I'm following all the conversions, but you are aware
>> that
>> cdc_mbim and cdc_ncm share a few functions struct cdc_ncm_ctx state, the
>> and therefore have to cooperate wrt the private data layout?
>
> Hi,
>
> I was not aware of that. What do you think about the approach taken
> in the attachment? It just makes sure that the first part of the
> descriptors match.

Looks reasonable to me.  I haven't actually tested it, though.


Bjørn

[RFCv3 06/13] net: usb: usbnet: add cdc_state to struct usbnet

[Oliver Neukum]

This allows centralisation of code using cdc_state in usbnet, reducing
code duplication. No functional change intended.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 include/linux/usb/usbnet.h | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 4a97845be62c..766862427620 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -16,6 +16,14 @@
 #include <linux/usb.h>
 #include <linux/spinlock.h>
 
+struct cdc_state {
+	struct usb_cdc_header_desc      *header;
+	struct usb_cdc_union_desc       *u;
+	struct usb_cdc_ether_desc       *ether;
+	struct usb_interface            *control;
+	struct usb_interface            *data;
+};
+
 /* interface from usbnet core to each USB networking link we handle */
 struct usbnet {
 	/* housekeeping */
@@ -41,6 +49,7 @@ struct usbnet {
 	/* protocol/interface state */
 	struct net_device	*net;
 	int			msg_enable;
+	struct cdc_state	cdc;		/* too common to leave out*/
 	unsigned long		data[5];
 	u32			xid;
 	u32			hard_mtu;	/* count any extra framing */
@@ -215,13 +224,6 @@ static inline void *usbnet_priv(struct usbnet *dev)
  * (notably, using multiple interfaces according to the CDC
  * union descriptor) get some helper code.
  */
-struct cdc_state {
-	struct usb_cdc_header_desc	*header;
-	struct usb_cdc_union_desc	*u;
-	struct usb_cdc_ether_desc	*ether;
-	struct usb_interface		*control;
-	struct usb_interface		*data;
-};
 
 extern void usbnet_cdc_update_filter(struct usbnet *dev);
 extern int usbnet_generic_cdc_bind(struct usbnet *, struct usb_interface *);
-- 
2.53.0

[RFCv3 07/13] net: usb: use cdc_state in struct usbnet

[Oliver Neukum]

Remove private copies as now a central state can be used.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/cdc_ether.c  | 6 +++---
 drivers/net/usb/rndis_host.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index c24c14702f4b..dafc2462be98 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -85,7 +85,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 	u8				*buf = intf->cur_altsetting->extra;
 	int				len = intf->cur_altsetting->extralen;
 	struct usb_interface_descriptor	*d;
-	struct cdc_state		*info = usbnet_priv(dev);
+	struct cdc_state		*info = &dev->cdc;
 	int				status;
 	int				rndis;
 	bool				android_rndis_quirk = false;
@@ -327,7 +327,7 @@ EXPORT_SYMBOL_GPL(usbnet_ether_cdc_bind);
 
 void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct cdc_state		*info = usbnet_priv(dev);
+	struct cdc_state		*info = &dev->cdc;
 	struct usb_driver		*driver = driver_of(intf);
 
 	/* combined interface - nothing  to do */
@@ -365,7 +365,7 @@ EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
 int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int				status;
-	struct cdc_state		*info = usbnet_priv(dev);
+	struct cdc_state		*info = &dev->cdc;
 
 	status = usbnet_ether_cdc_bind(dev, intf);
 	if (status < 0)
diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index 7b3739b29c8f..6aed29dfa021 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -57,7 +57,7 @@ EXPORT_SYMBOL_GPL(rndis_status);
 static void rndis_msg_indicate(struct usbnet *dev, struct rndis_indicate *msg,
 				int buflen)
 {
-	struct cdc_state *info = (void *)&dev->data;
+	struct cdc_state *info = &dev->cdc;
 	struct device *udev = &info->control->dev;
 
 	if (dev->driver_info->indication) {
@@ -90,7 +90,7 @@ static void rndis_msg_indicate(struct usbnet *dev, struct rndis_indicate *msg,
  */
 int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int buflen)
 {
-	struct cdc_state	*info = (void *) &dev->data;
+	struct cdc_state	*info = &dev->cdc;
 	struct usb_cdc_notification notification;
 	int			master_ifnum;
 	int			retval;
@@ -290,7 +290,7 @@ generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags)
 {
 	int			retval;
 	struct net_device	*net = dev->net;
-	struct cdc_state	*info = (void *) &dev->data;
+	struct cdc_state	*info = &dev->cdc;
 	union {
 		void			*buf;
 		struct rndis_msg_hdr	*header;
-- 
2.53.0

[RFCv3 08/13] net: usb: usbnet: allow drivers to specify private space needed

[Oliver Neukum]

usbnet has been providing a fixed space of unsigned long[5]
for minidrivers. This has led to various constructions in drivers
this is not enough space for.
Allow minidrivers to tell usbnet how much they need by means
of struct driver_info. For backwards compatibility zero
is taken to mean enough space for unsigned long[5]

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/qmi_wwan.c   | 3 ---
 drivers/net/usb/sierra_net.c | 3 ---
 drivers/net/usb/usbnet.c     | 5 ++++-
 include/linux/usb/usbnet.h   | 6 +++++-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 3a4985b582cb..fc2591ccb682 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -753,9 +753,6 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 	struct qmi_wwan_state *info = (void *)&dev->data;
 	struct usb_cdc_parsed_header hdr;
 
-	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) <
-		      sizeof(struct qmi_wwan_state)));
-
 	/* set up initial state */
 	info->control = intf;
 	info->data = intf;
diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index 4d3ed642b3e7..5a32796a7a89 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -855,9 +855,6 @@ static struct sk_buff *sierra_net_tx_fixup(struct usbnet *dev,
 	u16 len;
 	bool need_tail;
 
-	BUILD_BUG_ON(sizeof_field(struct usbnet, data)
-				< sizeof(struct cdc_state));
-
 	dev_dbg(&dev->udev->dev, "%s", __func__);
 	if (priv->link_up && check_ethip_packet(skb, dev) && is_ip(skb)) {
 		/* enough head room as is? */
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 8a9e0d98d9c6..b3cc3428b342 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1734,6 +1734,7 @@ usbnet_probe(struct usb_interface *udev, const struct usb_device_id *prod)
 	int				status;
 	const char			*name;
 	struct usb_driver 	*driver = to_usb_driver(udev->dev.driver);
+	unsigned int extra;
 
 	/* usbnet already took usb runtime pm, so have to enable the feature
 	 * for usb interface, otherwise usb_autopm_get_interface may return
@@ -1756,7 +1757,9 @@ usbnet_probe(struct usb_interface *udev, const struct usb_device_id *prod)
 	status = -ENOMEM;
 
 	// set up our own records
-	net = alloc_etherdev(sizeof(*dev));
+	extra = info->required_room ?
+		info->required_room : 5 * sizeof(unsigned long);
+	net = alloc_etherdev(sizeof(*dev) + extra);
 	if (!net)
 		goto out;
 
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 766862427620..41582346360f 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -50,7 +50,6 @@ struct usbnet {
 	struct net_device	*net;
 	int			msg_enable;
 	struct cdc_state	cdc;		/* too common to leave out*/
-	unsigned long		data[5];
 	u32			xid;
 	u32			hard_mtu;	/* count any extra framing */
 	size_t			rx_urb_size;	/* size for rx urbs */
@@ -93,6 +92,8 @@ struct usbnet {
  * that must be broken
  */
 #		define EVENT_UNPLUG		31
+
+	DECLARE_FLEX_ARRAY(unsigned long, data);
 };
 
 static inline bool usbnet_going_away(struct usbnet *ubn)
@@ -142,6 +143,9 @@ struct driver_info {
 #define FLAG_RX_ASSEMBLE	0x4000	/* rx packets may span >1 frames */
 #define FLAG_NOARP		0x8000	/* device can't do ARP */
 
+	/* tells us how much private space is need in the descriptor */
+	unsigned int	required_room;
+
 	/* init device ... can sleep, or cause probe() failure */
 	int	(*bind)(struct usbnet *, struct usb_interface *);
 
-- 
2.53.0

[RFCv3 09/13] net: usb: use allocation on demand for usbnet

[Oliver Neukum]

This converts the simple cases for allocation of
private room for mnidrivers to dynamic allocation.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/ax88172a.c | 1 +
 drivers/net/usb/cdc_mbim.c | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index f9b4364e0a0f..489a09759593 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -355,4 +355,5 @@ const struct driver_info ax88172a_info = {
 		 FLAG_MULTI_PACKET,
 	.rx_fixup = ax88172a_rx_fixup,
 	.tx_fixup = asix_tx_fixup,
+	.required_room = sizeof(struct ax88172a_private),
 };
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 5522dc137d45..e319fe37986f 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -557,6 +557,7 @@ static const struct driver_info cdc_mbim_info = {
 	.manage_power = cdc_mbim_manage_power,
 	.rx_fixup = cdc_mbim_rx_fixup,
 	.tx_fixup = cdc_mbim_tx_fixup,
+	.required_room = sizeof(struct cdc_mbim_state),
 };
 
 /* MBIM and NCM devices should not need a ZLP after NTBs with
@@ -581,6 +582,7 @@ static const struct driver_info cdc_mbim_info_zlp = {
 	.manage_power = cdc_mbim_manage_power,
 	.rx_fixup = cdc_mbim_rx_fixup,
 	.tx_fixup = cdc_mbim_tx_fixup,
+	.required_room = sizeof(struct cdc_mbim_state),
 };
 
 /* The spefication explicitly allows NDPs to be placed anywhere in the
@@ -601,6 +603,7 @@ static const struct driver_info cdc_mbim_info_ndp_to_end = {
 	.rx_fixup = cdc_mbim_rx_fixup,
 	.tx_fixup = cdc_mbim_tx_fixup,
 	.data = CDC_NCM_FLAG_NDP_TO_END,
+	.required_room = sizeof(struct cdc_mbim_state),
 };
 
 /* Some modems (e.g. Telit LE922A6) do not work properly with altsetting
@@ -616,6 +619,7 @@ static const struct driver_info cdc_mbim_info_avoid_altsetting_toggle = {
 	.rx_fixup = cdc_mbim_rx_fixup,
 	.tx_fixup = cdc_mbim_tx_fixup,
 	.data = CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE,
+	.required_room = sizeof(struct cdc_mbim_state),
 };
 
 static const struct usb_device_id mbim_devs[] = {
-- 
2.53.0

[RFCv3 10/13] net: usb: sierra_net: use dynamic private allocation

[Oliver Neukum]

The old style private room was too small for sierra_net.
With dynamic allocation that factor is gone.
Remove handling private allocations in the driver.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/sierra_net.c | 42 +++++++++++-------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index 5a32796a7a89..9a721413bdf4 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -187,19 +187,6 @@ static const struct net_device_ops sierra_net_device_ops = {
 	.ndo_validate_addr      = eth_validate_addr,
 };
 
-/* get private data associated with passed in usbnet device */
-static inline struct sierra_net_data *sierra_net_get_private(struct usbnet *dev)
-{
-	return (struct sierra_net_data *)dev->data[0];
-}
-
-/* set private data associated with passed in usbnet device */
-static inline void sierra_net_set_private(struct usbnet *dev,
-			struct sierra_net_data *priv)
-{
-	dev->data[0] = (unsigned long)priv;
-}
-
 /* is packet IPv4/IPv6 */
 static inline int is_ip(struct sk_buff *skb)
 {
@@ -317,7 +304,7 @@ static void build_hip(u8 *buf, const u16 payloadlen,
 static int sierra_net_send_cmd(struct usbnet *dev,
 		u8 *cmd, int cmdlen, const char * cmd_name)
 {
-	struct sierra_net_data *priv = sierra_net_get_private(dev);
+	struct sierra_net_data *priv = usbnet_priv(dev);
 	int  status;
 
 	status = usbnet_write_cmd(dev, USB_CDC_SEND_ENCAPSULATED_COMMAND,
@@ -333,7 +320,7 @@ static int sierra_net_send_cmd(struct usbnet *dev,
 static int sierra_net_send_sync(struct usbnet *dev)
 {
 	int  status;
-	struct sierra_net_data *priv = sierra_net_get_private(dev);
+	struct sierra_net_data *priv = usbnet_priv(dev);
 
 	dev_dbg(&dev->udev->dev, "%s", __func__);
 
@@ -410,7 +397,7 @@ static int sierra_net_parse_lsi(struct usbnet *dev, char *data, int datalen)
 static void sierra_net_handle_lsi(struct usbnet *dev, char *data,
 		struct hip_hdr	*hh)
 {
-	struct sierra_net_data *priv = sierra_net_get_private(dev);
+	struct sierra_net_data *priv = usbnet_priv(dev);
 	int link_up;
 
 	link_up = sierra_net_parse_lsi(dev, data + hh->hdrlen,
@@ -431,7 +418,7 @@ static void sierra_net_handle_lsi(struct usbnet *dev, char *data,
 static void sierra_net_dosync(struct usbnet *dev)
 {
 	int status;
-	struct sierra_net_data *priv = sierra_net_get_private(dev);
+	struct sierra_net_data *priv = usbnet_priv(dev);
 
 	dev_dbg(&dev->udev->dev, "%s", __func__);
 
@@ -560,7 +547,7 @@ static void sierra_net_kevent(struct work_struct *work)
 
 static void sierra_net_defer_kevent(struct usbnet *dev, int work)
 {
-	struct sierra_net_data *priv = sierra_net_get_private(dev);
+	struct sierra_net_data *priv = usbnet_priv(dev);
 
 	set_bit(work, &priv->kevent_flags);
 	schedule_work(&priv->sierra_net_kevent);
@@ -608,8 +595,10 @@ static void sierra_net_status(struct usbnet *dev, struct urb *urb)
 static u32 sierra_net_get_link(struct net_device *net)
 {
 	struct usbnet *dev = netdev_priv(net);
+	struct sierra_net_data *priv = usbnet_priv(dev);
+
 	/* Report link is down whenever the interface is down */
-	return sierra_net_get_private(dev)->link_up && netif_running(net);
+	return priv->link_up && netif_running(net);
 }
 
 static const struct ethtool_ops sierra_net_ethtool_ops = {
@@ -653,7 +642,7 @@ static int sierra_net_bind(struct usbnet *dev, struct usb_interface *intf)
 	u8	numendpoints;
 	u16	fwattr = 0;
 	int	status;
-	struct sierra_net_data *priv;
+	struct sierra_net_data *priv = usbnet_priv(dev);
 	static const u8 sync_tmplate[sizeof(priv->sync_msg)] = {
 		0x00, 0x00, SIERRA_NET_HIP_MSYNC_ID, 0x00};
 	static const u8 shdwn_tmplate[sizeof(priv->shdwn_msg)] = {
@@ -682,11 +671,8 @@ static int sierra_net_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev_err(&dev->udev->dev, "No status endpoint found");
 		return -ENODEV;
 	}
-	/* Initialize sierra private data */
-	priv = kzalloc_obj(*priv);
-	if (!priv)
-		return -ENOMEM;
 
+	/* Initialize sierra private data */
 	priv->usbnet = dev;
 	priv->ifnum = ifacenum;
 	dev->net->netdev_ops = &sierra_net_device_ops;
@@ -718,8 +704,6 @@ static int sierra_net_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->net->ethtool_ops = &sierra_net_ethtool_ops;
 	netif_carrier_off(dev->net);
 
-	sierra_net_set_private(dev, priv);
-
 	priv->kevent_flags = 0;
 
 	/* Use the shared workqueue */
@@ -747,7 +731,7 @@ static int sierra_net_bind(struct usbnet *dev, struct usb_interface *intf)
 static void sierra_net_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int status;
-	struct sierra_net_data *priv = sierra_net_get_private(dev);
+	struct sierra_net_data *priv = usbnet_priv(dev);
 
 	dev_dbg(&dev->udev->dev, "%s", __func__);
 
@@ -764,7 +748,6 @@ static void sierra_net_unbind(struct usbnet *dev, struct usb_interface *intf)
 
 	usbnet_status_stop(dev);
 
-	sierra_net_set_private(dev, NULL);
 	kfree(priv);
 }
 
@@ -851,7 +834,7 @@ static int sierra_net_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 static struct sk_buff *sierra_net_tx_fixup(struct usbnet *dev,
 					   struct sk_buff *skb, gfp_t flags)
 {
-	struct sierra_net_data *priv = sierra_net_get_private(dev);
+	struct sierra_net_data *priv = usbnet_priv(dev);
 	u16 len;
 	bool need_tail;
 
@@ -905,6 +888,7 @@ static const struct driver_info sierra_net_info_direct_ip = {
 	.status = sierra_net_status,
 	.rx_fixup = sierra_net_rx_fixup,
 	.tx_fixup = sierra_net_tx_fixup,
+	.required_room = sizeof(struct sierra_net_data),
 };
 
 static int
-- 
2.53.0

[RFCv3 11/13] net: usb: cdc_ncm: use dynamic private allocation

[Oliver Neukum]

The old style private room was too small for cdc_ncm.
With dynamic allocation that factor is gone.
Remove handling private allocations in the driver.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/cdc_ncm.c | 78 +++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 44 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 7057c6c0cfc6..91da42a778f4 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -107,7 +107,7 @@ static void cdc_ncm_get_ethtool_stats(struct net_device *netdev,
 				    u64 *data)
 {
 	struct usbnet *dev = netdev_priv(netdev);
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	int i;
 	char *p = NULL;
 
@@ -149,7 +149,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
 
 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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u32 val, max, min;
 
 	/* clamp new_rx to sane values */
@@ -172,7 +172,7 @@ static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx)
 
 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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u32 val, max, min;
 
 	/* clamp new_tx to sane values */
@@ -202,7 +202,7 @@ static ssize_t min_tx_pkt_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%u\n", ctx->min_tx_pkt);
 }
@@ -211,7 +211,7 @@ static ssize_t rx_max_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%u\n", ctx->rx_max);
 }
@@ -220,7 +220,7 @@ static ssize_t tx_max_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%u\n", ctx->tx_max);
 }
@@ -229,7 +229,7 @@ static ssize_t tx_timer_usecs_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%u\n", ctx->timer_interval / (u32)NSEC_PER_USEC);
 }
@@ -239,7 +239,7 @@ static ssize_t min_tx_pkt_store(struct device *d,
 				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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	unsigned long val;
 
 	/* no need to restrict values - anything from 0 to infinity is OK */
@@ -255,7 +255,7 @@ static ssize_t rx_max_store(struct device *d,
 			    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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	unsigned long val;
 
 	if (kstrtoul(buf, 0, &val) || cdc_ncm_check_rx_max(dev, val) != val)
@@ -270,7 +270,7 @@ static ssize_t tx_max_store(struct device *d,
 			    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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	unsigned long val;
 
 	if (kstrtoul(buf, 0, &val) || cdc_ncm_check_tx_max(dev, val) != val)
@@ -285,7 +285,7 @@ static ssize_t tx_timer_usecs_store(struct device *d,
 				    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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	ssize_t ret;
 	unsigned long val;
 
@@ -311,7 +311,7 @@ static DEVICE_ATTR_RW(tx_timer_usecs);
 static ssize_t ndp_to_end_show(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	return sprintf(buf, "%c\n", ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END ? 'Y' : 'N');
 }
@@ -319,7 +319,7 @@ static ssize_t ndp_to_end_show(struct device *d, struct device_attribute *attr,
 static ssize_t ndp_to_end_store(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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	bool enable;
 
 	if (kstrtobool(buf, &enable))
@@ -361,7 +361,7 @@ static DEVICE_ATTR_RW(ndp_to_end);
 static ssize_t cdc_ncm_show_##name(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]; \
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev); \
 	return sprintf(buf, format "\n", tocpu(ctx->ncm_parm.name));	\
 } \
 static DEVICE_ATTR(name, 0444, cdc_ncm_show_##name, NULL)
@@ -404,7 +404,7 @@ static const struct attribute_group cdc_ncm_sysfs_attr_group = {
 /* 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];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 	u32 val;
 
@@ -476,7 +476,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
 /* helpers for NCM and MBIM differences */
 static u8 cdc_ncm_flags(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	if (cdc_ncm_comm_intf_is_mbim(dev->intf->cur_altsetting) && ctx->mbim_desc)
 		return ctx->mbim_desc->bmNetworkCapabilities;
@@ -501,7 +501,7 @@ static u32 cdc_ncm_min_dgram_size(struct usbnet *dev)
 
 static u32 cdc_ncm_max_dgram_size(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	if (cdc_ncm_comm_intf_is_mbim(dev->intf->cur_altsetting) && ctx->mbim_desc)
 		return le16_to_cpu(ctx->mbim_desc->wMaxSegmentSize);
@@ -515,7 +515,7 @@ static u32 cdc_ncm_max_dgram_size(struct usbnet *dev)
  */
 static int cdc_ncm_init(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 	int err;
 
@@ -608,7 +608,7 @@ static int cdc_ncm_init(struct usbnet *dev)
 /* set a new max datagram size */
 static void cdc_ncm_set_dgram_size(struct usbnet *dev, int new_size)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 	__le16 max_datagram_size;
 	u16 mbim_mtu;
@@ -656,7 +656,7 @@ static void cdc_ncm_set_dgram_size(struct usbnet *dev, int new_size)
 
 static void cdc_ncm_fix_modulus(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u32 val;
 
 	/*
@@ -700,7 +700,7 @@ static void cdc_ncm_fix_modulus(struct usbnet *dev)
 
 static int cdc_ncm_setup(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	u32 def_rx, def_tx;
 
 	/* be conservative when selecting initial buffer size to
@@ -769,9 +769,6 @@ cdc_ncm_find_endpoints(struct usbnet *dev, struct usb_interface *intf)
 
 static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
 {
-	if (ctx == NULL)
-		return;
-
 	if (ctx->tx_rem_skb != NULL) {
 		dev_kfree_skb_any(ctx->tx_rem_skb);
 		ctx->tx_rem_skb = NULL;
@@ -786,8 +783,6 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
 		kfree(ctx->delayed_ndp16);
 	else
 		kfree(ctx->delayed_ndp32);
-
-	kfree(ctx);
 }
 
 /* we need to override the usbnet change_mtu ndo for two reasons:
@@ -819,7 +814,7 @@ static const struct net_device_ops cdc_ncm_netdev_ops = {
 
 int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting, int drvflags)
 {
-	struct cdc_ncm_ctx *ctx;
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	struct usb_driver *driver;
 	u8 *buf;
 	int len;
@@ -827,10 +822,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	u8 iface_no;
 	struct usb_cdc_parsed_header hdr;
 
-	ctx = kzalloc_obj(*ctx);
-	if (!ctx)
-		return -ENOMEM;
-
 	ctx->dev = dev;
 
 	hrtimer_setup(&ctx->tx_timer, &cdc_ncm_tx_timer_cb, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
@@ -838,9 +829,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	atomic_set(&ctx->stop, 0);
 	spin_lock_init(&ctx->mtx);
 
-	/* store ctx pointer in device data field */
-	dev->data[0] = (unsigned long)ctx;
-
 	/* only the control interface can be successfully probed */
 	ctx->control = intf;
 
@@ -988,8 +976,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	if (ctx->data != ctx->control)
 		usb_driver_release_interface(driver, ctx->data);
 error:
-	cdc_ncm_free((struct cdc_ncm_ctx *)dev->data[0]);
-	dev->data[0] = 0;
+	cdc_ncm_free(usbnet_priv(dev));
 	dev_info(&intf->dev, "bind() failure\n");
 	return -ENODEV;
 }
@@ -997,12 +984,9 @@ EXPORT_SYMBOL_GPL(cdc_ncm_bind_common);
 
 void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	struct usb_driver *driver = driver_of(intf);
 
-	if (ctx == NULL)
-		return;		/* no setup */
-
 	atomic_set(&ctx->stop, 1);
 
 	hrtimer_cancel(&ctx->tx_timer);
@@ -1203,7 +1187,7 @@ static struct usb_cdc_ncm_ndp32 *cdc_ncm_ndp32(struct cdc_ncm_ctx *ctx, struct s
 struct sk_buff *
 cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	union {
 		struct usb_cdc_ncm_nth16 *nth16;
 		struct usb_cdc_ncm_nth32 *nth32;
@@ -1523,7 +1507,7 @@ struct sk_buff *
 cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
 	struct sk_buff *skb_out;
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	/*
 	 * The Ethernet API we are using does not support transmitting
@@ -1725,7 +1709,7 @@ EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp32);
 int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	unsigned int len;
 	int nframes;
 	int x;
@@ -1904,7 +1888,7 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 
 static void cdc_ncm_update_filter(struct usbnet *dev)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 
 	if (ctx->filtering_supported)
 		usbnet_cdc_update_filter(dev);
@@ -1921,6 +1905,7 @@ static const struct driver_info cdc_ncm_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = cdc_ncm_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as cdc_ncm_info, but with FLAG_SEND_ZLP  */
@@ -1935,6 +1920,7 @@ static const struct driver_info cdc_ncm_zlp_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = cdc_ncm_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as cdc_ncm_info, but with FLAG_SEND_ZLP */
@@ -1949,6 +1935,7 @@ static const struct driver_info apple_tethering_interface_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = usbnet_cdc_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as apple_tethering_interface_info, but without FLAG_LINK_INTR */
@@ -1963,6 +1950,7 @@ static const struct driver_info apple_private_interface_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = usbnet_cdc_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as cdc_ncm_info, but with FLAG_WWAN */
@@ -1977,6 +1965,7 @@ static const struct driver_info wwan_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = cdc_ncm_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 /* Same as wwan_info, but with FLAG_NOARP  */
@@ -1991,6 +1980,7 @@ static const struct driver_info wwan_noarp_info = {
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
 	.set_rx_mode = cdc_ncm_update_filter,
+	.required_room = sizeof(struct cdc_ncm_ctx),
 };
 
 static const struct usb_device_id cdc_devs[] = {
-- 
2.53.0

[RFCv3 12/13] net: usb: smsc95xx: use dynamic private allocation

[Oliver Neukum]

The old style private room was too small for smsc95xx
With dynamic allocation that factor is gone.
Remove handling private allocations in the driver.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/smsc95xx.c | 67 +++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 38 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 42e4048b574b..b1d66290fe17 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -84,7 +84,7 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
 					  u32 *data)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	u32 buf;
 	int ret;
 	int (*fn)(struct usbnet *, u8, u8, u16, u16, void *, u16);
@@ -115,7 +115,7 @@ static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
 static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
 					   u32 data)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	u32 buf;
 	int ret;
 	int (*fn)(struct usbnet *, u8, u8, u16, u16, const void *, u16);
@@ -265,7 +265,7 @@ static int smsc95xx_mdiobus_reset(struct mii_bus *bus)
 	int ret;
 
 	dev = bus->priv;
-	pdata = dev->driver_priv;
+	pdata = usbnet_priv(dev);
 
 	if (pdata->is_internal_phy)
 		return 0;
@@ -481,7 +481,7 @@ static unsigned int smsc95xx_hash(char addr[ETH_ALEN])
 static void smsc95xx_set_multicast(struct net_device *netdev)
 {
 	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	unsigned long flags;
 	int ret;
 
@@ -539,7 +539,7 @@ static void smsc95xx_set_multicast(struct net_device *netdev)
 
 static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	u32 flow = 0, afc_cfg;
 
 	int ret = smsc95xx_read_reg(dev, AFC_CFG, &afc_cfg);
@@ -584,7 +584,7 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev)
 
 static void smsc95xx_mac_update_fullduplex(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	unsigned long flags;
 	int ret;
 
@@ -613,7 +613,7 @@ static void smsc95xx_mac_update_fullduplex(struct usbnet *dev)
 
 static void smsc95xx_status(struct usbnet *dev, struct urb *urb)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	unsigned long flags;
 	u32 intdata;
 
@@ -730,7 +730,7 @@ static void smsc95xx_ethtool_get_wol(struct net_device *net,
 				     struct ethtool_wolinfo *wolinfo)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 
 	wolinfo->supported = SUPPORTED_WAKE;
 	wolinfo->wolopts = pdata->wolopts;
@@ -740,7 +740,7 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
 				    struct ethtool_wolinfo *wolinfo)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	int ret;
 
 	if (wolinfo->wolopts & ~SUPPORTED_WAKE)
@@ -788,7 +788,7 @@ static void smsc95xx_get_pauseparam(struct net_device *ndev,
 	struct usbnet *dev;
 
 	dev = netdev_priv(ndev);
-	pdata = dev->driver_priv;
+	pdata = usbnet_priv(dev);
 
 	pause->autoneg = pdata->pause_autoneg;
 	pause->rx_pause = pdata->pause_rx;
@@ -804,7 +804,7 @@ static int smsc95xx_set_pauseparam(struct net_device *ndev,
 	struct usbnet *dev;
 
 	dev = netdev_priv(ndev);
-	pdata = dev->driver_priv;
+	pdata = usbnet_priv(dev);
 	phydev = ndev->phydev;
 
 	if (!phydev)
@@ -898,7 +898,7 @@ static int smsc95xx_set_mac_address(struct usbnet *dev)
 /* starts the TX path */
 static int smsc95xx_start_tx_path(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	unsigned long flags;
 	int ret;
 
@@ -918,7 +918,7 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
 /* Starts the Receive path */
 static int smsc95xx_start_rx_path(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	unsigned long flags;
 
 	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
@@ -930,7 +930,7 @@ static int smsc95xx_start_rx_path(struct usbnet *dev)
 
 static int smsc95xx_reset(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	u32 read_buf, burst_cap;
 	int ret = 0, timeout;
 
@@ -1146,7 +1146,7 @@ static void smsc95xx_handle_link_change(struct net_device *net)
 
 static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct smsc95xx_priv *pdata;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	char usb_path[64];
 	int ret, phy_irq;
 	u32 val;
@@ -1157,12 +1157,6 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 		return ret;
 	}
 
-	pdata = kzalloc_obj(*pdata);
-	if (!pdata)
-		return -ENOMEM;
-
-	dev->driver_priv = pdata;
-
 	spin_lock_init(&pdata->mac_cr_lock);
 
 	/* LAN95xx devices do not alter the computed checksum of 0 to 0xffff.
@@ -1185,14 +1179,14 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Init all registers */
 	ret = smsc95xx_reset(dev);
 	if (ret)
-		goto free_pdata;
+		goto bail_out;
 
 	/* create irq domain for use by PHY driver and GPIO consumers */
 	usb_make_path(dev->udev, usb_path, sizeof(usb_path));
 	pdata->irqfwnode = irq_domain_alloc_named_fwnode(usb_path);
 	if (!pdata->irqfwnode) {
 		ret = -ENOMEM;
-		goto free_pdata;
+		goto bail_out;
 	}
 
 	pdata->irqdomain = irq_domain_create_linear(pdata->irqfwnode,
@@ -1309,14 +1303,13 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 free_irqfwnode:
 	irq_domain_free_fwnode(pdata->irqfwnode);
 
-free_pdata:
-	kfree(pdata);
+bail_out:
 	return ret;
 }
 
 static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 
 	phy_disconnect(dev->net->phydev);
 	mdiobus_unregister(pdata->mdiobus);
@@ -1324,8 +1317,6 @@ static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 	irq_dispose_mapping(irq_find_mapping(pdata->irqdomain, PHY_HWIRQ));
 	irq_domain_remove(pdata->irqdomain);
 	irq_domain_free_fwnode(pdata->irqfwnode);
-	netif_dbg(dev, ifdown, dev->net, "free pdata\n");
-	kfree(pdata);
 }
 
 static int smsc95xx_start_phy(struct usbnet *dev)
@@ -1350,7 +1341,7 @@ static u32 smsc_crc(const u8 *buffer, size_t len, int filter)
 
 static int smsc95xx_link_ok(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	int ret;
 
 	/* first, a dummy read, needed to latch some MII phys */
@@ -1367,7 +1358,7 @@ static int smsc95xx_link_ok(struct usbnet *dev)
 
 static int smsc95xx_enter_suspend0(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	u32 val;
 	int ret;
 
@@ -1406,7 +1397,7 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 
 static int smsc95xx_enter_suspend1(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	int ret, phy_id = pdata->phydev->mdio.addr;
 	u32 val;
 
@@ -1453,7 +1444,7 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
 
 static int smsc95xx_enter_suspend2(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	u32 val;
 	int ret;
 
@@ -1475,7 +1466,7 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
 
 static int smsc95xx_enter_suspend3(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	u32 val;
 	int ret;
 
@@ -1514,7 +1505,7 @@ static int smsc95xx_enter_suspend3(struct usbnet *dev)
 
 static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 
 	if (!netif_running(dev->net)) {
 		/* interface is ifconfig down so fully power down hw */
@@ -1544,7 +1535,7 @@ static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
 static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	u32 val, link_up;
 	int ret;
 
@@ -1805,7 +1796,7 @@ static int smsc95xx_resume(struct usb_interface *intf)
 	u32 val;
 
 	BUG_ON(!dev);
-	pdata = dev->driver_priv;
+	pdata = usbnet_priv(dev);
 	suspend_flags = pdata->suspend_flags;
 
 	netdev_dbg(dev->net, "resume suspend_flags=0x%02x\n", suspend_flags);
@@ -1854,7 +1845,7 @@ static int smsc95xx_resume(struct usb_interface *intf)
 static int smsc95xx_reset_resume(struct usb_interface *intf)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 	int ret;
 
 	pdata->pm_task = current;
@@ -2038,7 +2029,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 
 static int smsc95xx_manage_power(struct usbnet *dev, int on)
 {
-	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct smsc95xx_priv *pdata = usbnet_priv(dev);
 
 	dev->intf->needs_remote_wakeup = on;
 
-- 
2.53.0

[RFCv3 13/13] net: usb: lg-vl600: use dynamic private allocation

[Oliver Neukum]

The old style private room was too small for lg-vl600
With dynamic allocation that factor is gone.
Remove handling private allocations in the driver.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/lg-vl600.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/lg-vl600.c b/drivers/net/usb/lg-vl600.c
index c4ad2d9f6f4f..02949e78e5d6 100644
--- a/drivers/net/usb/lg-vl600.c
+++ b/drivers/net/usb/lg-vl600.c
@@ -56,18 +56,10 @@ struct vl600_state {
 static int vl600_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int ret;
-	struct vl600_state *s = kzalloc_obj(struct vl600_state);
-
-	if (!s)
-		return -ENOMEM;
 
 	ret = usbnet_cdc_bind(dev, intf);
-	if (ret) {
-		kfree(s);
+	if (ret)
 		return ret;
-	}
-
-	dev->driver_priv = s;
 
 	/* ARP packets don't go through, but they're also of no use.  The
 	 * subnet has only two hosts anyway: us and the gateway / DHCP
@@ -85,10 +77,9 @@ static int vl600_bind(struct usbnet *dev, struct usb_interface *intf)
 
 static void vl600_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct vl600_state *s = dev->driver_priv;
+	struct vl600_state *s = usbnet_priv(dev);
 
 	dev_kfree_skb(s->current_rx_buf);
-	kfree(s);
 
 	return usbnet_cdc_unbind(dev, intf);
 }
@@ -101,7 +92,7 @@ static int vl600_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	int packet_len, count;
 	struct sk_buff *buf = skb;
 	struct sk_buff *clone;
-	struct vl600_state *s = dev->driver_priv;
+	struct vl600_state *s = usbnet_priv(dev);
 
 	/* Frame lengths are generally 4B multiplies but every couple of
 	 * hours there's an odd number of bytes sized yet correct frame,
@@ -306,6 +297,7 @@ static const struct driver_info	vl600_info = {
 	.status		= usbnet_cdc_status,
 	.rx_fixup	= vl600_rx_fixup,
 	.tx_fixup	= vl600_tx_fixup,
+	.required_room	= sizeof(struct vl600_state),
 };
 
 static const struct usb_device_id products[] = {
-- 
2.53.0