diff mbox series

[RFC,net-next,1/2] usb: class: cdc-wdm: add control type

Message ID 1619777783-24116-1-git-send-email-loic.poulain@linaro.org (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series [RFC,net-next,1/2] usb: class: cdc-wdm: add control type | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 5 maintainers not CCed: lee.jones@linaro.org gregkh@linuxfoundation.org colin.king@canonical.com davem@davemloft.net penguin-kernel@i-love.sakura.ne.jp
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Loic Poulain April 30, 2021, 10:16 a.m. UTC 
Add type parameter to usb_cdc_wdm_register function in order to
specify which control protocol the cdc-wdm channel is transporting.
It will be required for exposing the channel(s) through WWAN framework.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/net/usb/cdc_mbim.c       |  1 +
 drivers/net/usb/huawei_cdc_ncm.c |  1 +
 drivers/net/usb/qmi_wwan.c       |  3 ++-
 drivers/usb/class/cdc-wdm.c      | 13 +++++++++----
 include/linux/usb/cdc-wdm.h      | 16 +++++++++++++++-
 5 files changed, 28 insertions(+), 6 deletions(-)

Comments

Oliver Neukum April 30, 2021, 10:32 a.m. UTC | #1 
Am Freitag, den 30.04.2021, 12:16 +0200 schrieb Loic Poulain:
> Add type parameter to usb_cdc_wdm_register function in order to
> specify which control protocol the cdc-wdm channel is transporting.
> It will be required for exposing the channel(s) through WWAN framework.

Hi,

that is an interesting framework.
Some issues though.

	Regards
		Oliver

> +/**
> + * enum usb_cdc_wdm_type - CDC WDM endpoint type
> + * @USB_CDC_WDM_UNKNOWN: Unknown type
> + * @USB_CDC_WDM_MBIM: Mobile Broadband Interface Model control
> + * @USB_CDC_WDM_QMI: Qualcomm Modem Interface for modem control
> + * @USB_CDC_WDM_AT: AT commands interface
> + */
> +enum usb_cdc_wdm_type {
> +	USB_CDC_WDM_UNKNOWN,
> +	USB_CDC_WDM_MBIM,
> +	USB_CDC_WDM_QMI,
> +	USB_CDC_WDM_AT
> +};


If this is supposed to integrate CDC-WDM into a larger subsystem, what
use are private types here? If you do this the protocols need to come
from the common framework.
diff mbox series

Patch

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 5db6627..63b134b 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -168,6 +168,7 @@  static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
 		subdriver = usb_cdc_wdm_register(ctx->control,
 						 &dev->status->desc,
 						 le16_to_cpu(ctx->mbim_desc->wMaxControlMessage),
+						 USB_CDC_WDM_MBIM,
 						 cdc_mbim_wdm_manage_power);
 	if (IS_ERR(subdriver)) {
 		ret = PTR_ERR(subdriver);
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index a87f0da..388a46b 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -96,6 +96,7 @@  static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
 		subdriver = usb_cdc_wdm_register(ctx->control,
 						 &usbnet_dev->status->desc,
 						 1024, /* wMaxCommand */
+						 USB_CDC_WDM_AT,
 						 huawei_cdc_ncm_wdm_manage_power);
 	if (IS_ERR(subdriver)) {
 		ret = PTR_ERR(subdriver);
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 17a0505..fa38471 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -724,7 +724,8 @@  static int qmi_wwan_register_subdriver(struct usbnet *dev)
 
 	/* register subdriver */
 	subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc,
-					 4096, &qmi_wwan_cdc_wdm_manage_power);
+					 4096, USB_CDC_WDM_QMI,
+					 &qmi_wwan_cdc_wdm_manage_power);
 	if (IS_ERR(subdriver)) {
 		dev_err(&info->control->dev, "subdriver registration failed\n");
 		rv = PTR_ERR(subdriver);
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 508b1c3..b59f146 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -106,6 +106,8 @@  struct wdm_device {
 
 	struct list_head	device_list;
 	int			(*manage_power)(struct usb_interface *, int);
+
+	enum usb_cdc_wdm_type	type;
 };
 
 static struct usb_driver wdm_driver;
@@ -836,7 +838,8 @@  static void service_interrupt_work(struct work_struct *work)
 /* --- hotplug --- */
 
 static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor *ep,
-		u16 bufsize, int (*manage_power)(struct usb_interface *, int))
+		      u16 bufsize, enum usb_cdc_wdm_type type,
+		      int (*manage_power)(struct usb_interface *, int))
 {
 	int rv = -ENOMEM;
 	struct wdm_device *desc;
@@ -853,6 +856,7 @@  static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor
 	/* this will be expanded and needed in hardware endianness */
 	desc->inum = cpu_to_le16((u16)intf->cur_altsetting->desc.bInterfaceNumber);
 	desc->intf = intf;
+	desc->type = type;
 	INIT_WORK(&desc->rxwork, wdm_rxwork);
 	INIT_WORK(&desc->service_outs_intr, service_interrupt_work);
 
@@ -977,7 +981,7 @@  static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id)
 		goto err;
 	ep = &iface->endpoint[0].desc;
 
-	rv = wdm_create(intf, ep, maxcom, &wdm_manage_power);
+	rv = wdm_create(intf, ep, maxcom, USB_CDC_WDM_UNKNOWN, &wdm_manage_power);
 
 err:
 	return rv;
@@ -988,6 +992,7 @@  static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id)
  * @intf: usb interface the subdriver will associate with
  * @ep: interrupt endpoint to monitor for notifications
  * @bufsize: maximum message size to support for read/write
+ * @type: Type/protocol of the transported data (MBIM, QMI...)
  * @manage_power: call-back invoked during open and release to
  *                manage the device's power
  * Create WDM usb class character device and associate it with intf
@@ -1005,12 +1010,12 @@  static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id)
  */
 struct usb_driver *usb_cdc_wdm_register(struct usb_interface *intf,
 					struct usb_endpoint_descriptor *ep,
-					int bufsize,
+					int bufsize, enum usb_cdc_wdm_type type,
 					int (*manage_power)(struct usb_interface *, int))
 {
 	int rv;
 
-	rv = wdm_create(intf, ep, bufsize, manage_power);
+	rv = wdm_create(intf, ep, bufsize, type, manage_power);
 	if (rv < 0)
 		goto err;
 
diff --git a/include/linux/usb/cdc-wdm.h b/include/linux/usb/cdc-wdm.h
index 9b895f9..ba9702d 100644
--- a/include/linux/usb/cdc-wdm.h
+++ b/include/linux/usb/cdc-wdm.h
@@ -14,9 +14,23 @@ 
 
 #include <uapi/linux/usb/cdc-wdm.h>
 
+/**
+ * enum usb_cdc_wdm_type - CDC WDM endpoint type
+ * @USB_CDC_WDM_UNKNOWN: Unknown type
+ * @USB_CDC_WDM_MBIM: Mobile Broadband Interface Model control
+ * @USB_CDC_WDM_QMI: Qualcomm Modem Interface for modem control
+ * @USB_CDC_WDM_AT: AT commands interface
+ */
+enum usb_cdc_wdm_type {
+	USB_CDC_WDM_UNKNOWN,
+	USB_CDC_WDM_MBIM,
+	USB_CDC_WDM_QMI,
+	USB_CDC_WDM_AT
+};
+
 extern struct usb_driver *usb_cdc_wdm_register(struct usb_interface *intf,
 					struct usb_endpoint_descriptor *ep,
-					int bufsize,
+					int bufsize, enum usb_cdc_wdm_type type,
 					int (*manage_power)(struct usb_interface *, int));
 
 #endif /* __LINUX_USB_CDC_WDM_H */