diff mbox series

[net-next] qmi_wwan: unconditionally reject 2 ep interfaces

Message ID 20200208155504.30243-1-bjorn@mork.no (mailing list archive)
State Mainlined
Commit 00516d13d4cfa56ce39da144db2dbf08b09b9357
Headers show
Series [net-next] qmi_wwan: unconditionally reject 2 ep interfaces | expand

Commit Message

Bjørn Mork Feb. 8, 2020, 3:55 p.m. UTC
We have been using the fact that the QMI and DIAG functions
usually are the only ones with class/subclass/protocol being
ff/ff/ff on Quectel modems. This has allowed us to match the
QMI function without knowing the exact interface number,
which can vary depending on firmware configuration.

The ability to silently reject the DIAG function, which is
usually handled by the option driver, is important for this
method to work.  This is done based on the knowledge that it
has exactly 2 bulk endpoints.  QMI function control interfaces
will have either 3 or 1 endpoint. This rule is universal so
the quirk condition can be removed.

The fixed layouts known from the Gobi1k and Gobi2k modems
have been gradually replaced by more dynamic layouts, and
many vendors now use configurable layouts without changing
device IDs.  Renaming the class/subclass/protocol matching
macro makes it more obvious that this is now not Quectel
specific anymore.

Cc: Kristian Evensen <kristian.evensen@gmail.com>
Cc: Aleksander Morgado <aleksander@aleksander.es>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
What do you think, Kristian?  There is no real need to limit this
rule to Quectel modems, is there?  And from what I've understood,
it seems that most/all the upcoming X55 modems will have a
completely configurable layout.  Which means that we should
avoid macthing on interface number if we can.  And I believe we
can. I've not yet seen an example where ff/ff/ff would match
anything except QMI and DIAG.


 drivers/net/usb/qmi_wwan.c | 42 ++++++++++++++------------------------
 1 file changed, 15 insertions(+), 27 deletions(-)

Comments

David Miller Feb. 10, 2020, 1:03 p.m. UTC | #1
From: Bjørn Mork <bjorn@mork.no>
Date: Sat,  8 Feb 2020 16:55:04 +0100

> We have been using the fact that the QMI and DIAG functions
> usually are the only ones with class/subclass/protocol being
> ff/ff/ff on Quectel modems. This has allowed us to match the
> QMI function without knowing the exact interface number,
> which can vary depending on firmware configuration.
> 
> The ability to silently reject the DIAG function, which is
> usually handled by the option driver, is important for this
> method to work.  This is done based on the knowledge that it
> has exactly 2 bulk endpoints.  QMI function control interfaces
> will have either 3 or 1 endpoint. This rule is universal so
> the quirk condition can be removed.
> 
> The fixed layouts known from the Gobi1k and Gobi2k modems
> have been gradually replaced by more dynamic layouts, and
> many vendors now use configurable layouts without changing
> device IDs.  Renaming the class/subclass/protocol matching
> macro makes it more obvious that this is now not Quectel
> specific anymore.
> 
> Cc: Kristian Evensen <kristian.evensen@gmail.com>
> Cc: Aleksander Morgado <aleksander@aleksander.es>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>

Applied.
Kristian Evensen Feb. 11, 2020, 11:04 a.m. UTC | #2
Hi Bjørn,

On Sat, Feb 8, 2020 at 4:55 PM Bjørn Mork <bjorn@mork.no> wrote:
>
> We have been using the fact that the QMI and DIAG functions
> usually are the only ones with class/subclass/protocol being
> ff/ff/ff on Quectel modems. This has allowed us to match the
> QMI function without knowing the exact interface number,
> which can vary depending on firmware configuration.
>
> The ability to silently reject the DIAG function, which is
> usually handled by the option driver, is important for this
> method to work.  This is done based on the knowledge that it
> has exactly 2 bulk endpoints.  QMI function control interfaces
> will have either 3 or 1 endpoint. This rule is universal so
> the quirk condition can be removed.
>
> The fixed layouts known from the Gobi1k and Gobi2k modems
> have been gradually replaced by more dynamic layouts, and
> many vendors now use configurable layouts without changing
> device IDs.  Renaming the class/subclass/protocol matching
> macro makes it more obvious that this is now not Quectel
> specific anymore.
>
> Cc: Kristian Evensen <kristian.evensen@gmail.com>
> Cc: Aleksander Morgado <aleksander@aleksander.es>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> What do you think, Kristian?  There is no real need to limit this
> rule to Quectel modems, is there?  And from what I've understood,
> it seems that most/all the upcoming X55 modems will have a
> completely configurable layout.  Which means that we should
> avoid macthing on interface number if we can.  And I believe we
> can. I've not yet seen an example where ff/ff/ff would match
> anything except QMI and DIAG.

I am sorry for my late reply, your email had for some reason ended up
in my spam filter. I agree with you reasoning and I think that making
the Quectel-code generic is a good idea. I went through the modem I
have, and could also not find any modems where the current
Quectel-code would incorrectly match. FWIW:

Acked-by: Kristian Evensen <kristian.evensen@gmail.com>

BR,
Kristian
diff mbox series

Patch

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 839cef720cf6..3b7a3b8a5e06 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -61,7 +61,6 @@  enum qmi_wwan_flags {
 
 enum qmi_wwan_quirks {
 	QMI_WWAN_QUIRK_DTR = 1 << 0,	/* needs "set DTR" request */
-	QMI_WWAN_QUIRK_QUECTEL_DYNCFG = 1 << 1,	/* check num. endpoints */
 };
 
 struct qmimux_hdr {
@@ -916,16 +915,6 @@  static const struct driver_info	qmi_wwan_info_quirk_dtr = {
 	.data           = QMI_WWAN_QUIRK_DTR,
 };
 
-static const struct driver_info	qmi_wwan_info_quirk_quectel_dyncfg = {
-	.description	= "WWAN/QMI device",
-	.flags		= FLAG_WWAN | FLAG_SEND_ZLP,
-	.bind		= qmi_wwan_bind,
-	.unbind		= qmi_wwan_unbind,
-	.manage_power	= qmi_wwan_manage_power,
-	.rx_fixup       = qmi_wwan_rx_fixup,
-	.data           = QMI_WWAN_QUIRK_DTR | QMI_WWAN_QUIRK_QUECTEL_DYNCFG,
-};
-
 #define HUAWEI_VENDOR_ID	0x12D1
 
 /* map QMI/wwan function by a fixed interface number */
@@ -946,14 +935,18 @@  static const struct driver_info	qmi_wwan_info_quirk_quectel_dyncfg = {
 #define QMI_GOBI_DEVICE(vend, prod) \
 	QMI_FIXED_INTF(vend, prod, 0)
 
-/* Quectel does not use fixed interface numbers on at least some of their
- * devices. We need to check the number of endpoints to ensure that we bind to
- * the correct interface.
+/* Many devices have QMI and DIAG functions which are distinguishable
+ * from other vendor specific functions by class, subclass and
+ * protocol all being 0xff. The DIAG function has exactly 2 endpoints
+ * and is silently rejected when probed.
+ *
+ * This makes it possible to match dynamically numbered QMI functions
+ * as seen on e.g. many Quectel modems.
  */
-#define QMI_QUIRK_QUECTEL_DYNCFG(vend, prod) \
+#define QMI_MATCH_FF_FF_FF(vend, prod) \
 	USB_DEVICE_AND_INTERFACE_INFO(vend, prod, USB_CLASS_VENDOR_SPEC, \
 				      USB_SUBCLASS_VENDOR_SPEC, 0xff), \
-	.driver_info = (unsigned long)&qmi_wwan_info_quirk_quectel_dyncfg
+	.driver_info = (unsigned long)&qmi_wwan_info_quirk_dtr
 
 static const struct usb_device_id products[] = {
 	/* 1. CDC ECM like devices match on the control interface */
@@ -1059,10 +1052,10 @@  static const struct usb_device_id products[] = {
 		USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x581d, USB_CLASS_VENDOR_SPEC, 1, 7),
 		.driver_info = (unsigned long)&qmi_wwan_info,
 	},
-	{QMI_QUIRK_QUECTEL_DYNCFG(0x2c7c, 0x0125)},	/* Quectel EC25, EC20 R2.0  Mini PCIe */
-	{QMI_QUIRK_QUECTEL_DYNCFG(0x2c7c, 0x0306)},	/* Quectel EP06/EG06/EM06 */
-	{QMI_QUIRK_QUECTEL_DYNCFG(0x2c7c, 0x0512)},	/* Quectel EG12/EM12 */
-	{QMI_QUIRK_QUECTEL_DYNCFG(0x2c7c, 0x0800)},	/* Quectel RM500Q-GL */
+	{QMI_MATCH_FF_FF_FF(0x2c7c, 0x0125)},	/* Quectel EC25, EC20 R2.0  Mini PCIe */
+	{QMI_MATCH_FF_FF_FF(0x2c7c, 0x0306)},	/* Quectel EP06/EG06/EM06 */
+	{QMI_MATCH_FF_FF_FF(0x2c7c, 0x0512)},	/* Quectel EG12/EM12 */
+	{QMI_MATCH_FF_FF_FF(0x2c7c, 0x0800)},	/* Quectel RM500Q-GL */
 
 	/* 3. Combined interface devices matching on interface number */
 	{QMI_FIXED_INTF(0x0408, 0xea42, 4)},	/* Yota / Megafon M100-1 */
@@ -1455,7 +1448,6 @@  static int qmi_wwan_probe(struct usb_interface *intf,
 {
 	struct usb_device_id *id = (struct usb_device_id *)prod;
 	struct usb_interface_descriptor *desc = &intf->cur_altsetting->desc;
-	const struct driver_info *info;
 
 	/* Workaround to enable dynamic IDs.  This disables usbnet
 	 * blacklisting functionality.  Which, if required, can be
@@ -1491,12 +1483,8 @@  static int qmi_wwan_probe(struct usb_interface *intf,
 	 * different. Ignore the current interface if the number of endpoints
 	 * equals the number for the diag interface (two).
 	 */
-	info = (void *)id->driver_info;
-
-	if (info->data & QMI_WWAN_QUIRK_QUECTEL_DYNCFG) {
-		if (desc->bNumEndpoints == 2)
-			return -ENODEV;
-	}
+	if (desc->bNumEndpoints == 2)
+		return -ENODEV;
 
 	return usbnet_probe(intf, id);
 }