diff mbox series

[net] qmi_wwan: Add support for Quectel EG12/EM12

Message ID 20190302123226.14589-1-kristian.evensen@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series [net] qmi_wwan: Add support for Quectel EG12/EM12 | expand

Commit Message

Kristian Evensen March 2, 2019, 12:32 p.m. UTC
Quectel EG12 (module)/EM12 (M.2 card) is a Cat. 12 LTE modem. The modem
behaves in the same way as the EP06, so the "set DTR"-quirk must be
applied and the diagnostic-interface check performed. Since the
diagnostic-check now applies to more modems, I have renamed the function
from quectel_ep06_diag_detected() to quectel_diag_detected().

Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
---
 drivers/net/usb/qmi_wwan.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

Comments

Bjørn Mork March 2, 2019, 3 p.m. UTC | #1
Kristian Evensen <kristian.evensen@gmail.com> writes:

> Quectel EG12 (module)/EM12 (M.2 card) is a Cat. 12 LTE modem. The modem
> behaves in the same way as the EP06, so the "set DTR"-quirk must be
> applied and the diagnostic-interface check performed. Since the
> diagnostic-check now applies to more modems, I have renamed the function
> from quectel_ep06_diag_detected() to quectel_diag_detected().

I am not too happy about the double device id matching with extra magic
applied. outside the device id table.  It does not scale, and it does
not play well at all with dynamic IDs.  But I guess it's acceptable for
two devices when it was ok for one, so

Acked-by: Bjørn Mork <bjorn@mork.no>

But I assume this isn't the last device we've seen with this issue.
Someone has already mention Quectel EM20. Maybe convert this code to a
quirk for the next device and leave all the device specific magic in the
device id table?  Could even add a macro for the class match + quirk
thing.


Bjørn
Kristian Evensen March 2, 2019, 9:19 p.m. UTC | #2
On Sat, Mar 2, 2019 at 4:00 PM Bjørn Mork <bjorn@mork.no> wrote:
> I am not too happy about the double device id matching with extra magic
> applied. outside the device id table.  It does not scale, and it does
> not play well at all with dynamic IDs.  But I guess it's acceptable for
> two devices when it was ok for one, so
>
> Acked-by: Bjørn Mork <bjorn@mork.no>
>
> But I assume this isn't the last device we've seen with this issue.
> Someone has already mention Quectel EM20. Maybe convert this code to a
> quirk for the next device and leave all the device specific magic in the
> device id table?  Could even add a macro for the class match + quirk
> thing.

(Appologies to Bjørn for receiving multiple copies of this email - I
forgot that the Gmail app sends HTML-encoded emails)

Thanks for your feedback. My reasoning was the same. For one or two
devices, the current approach is (probably) ok. Any more, and a
different approach is needed.

Looking through the documentation I have, it seems like most
Quectel-modems support the command used to change the
USB-configuration. Thus, I wouldn’t be surprised if all or most
existing Quectel-devices should have this quirk applied. I have a
couple of older modules, so I will look at how they behave with
different configurations. I will then either add the quirk soon or
when for example EM20 is available (unless someone else does it
earlier :)).

BR,
Kristian
David Miller March 4, 2019, 5:40 a.m. UTC | #3
From: Kristian Evensen <kristian.evensen@gmail.com>
Date: Sat,  2 Mar 2019 13:32:26 +0100

> Quectel EG12 (module)/EM12 (M.2 card) is a Cat. 12 LTE modem. The modem
> behaves in the same way as the EP06, so the "set DTR"-quirk must be
> applied and the diagnostic-interface check performed. Since the
> diagnostic-check now applies to more modems, I have renamed the function
> from quectel_ep06_diag_detected() to quectel_diag_detected().
> 
> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>

Applied, thanks.
Dan Williams March 5, 2019, 5:28 p.m. UTC | #4
On Sat, 2019-03-02 at 22:19 +0100, Kristian Evensen wrote:
> On Sat, Mar 2, 2019 at 4:00 PM Bjørn Mork <bjorn@mork.no> wrote:
> > I am not too happy about the double device id matching with extra
> > magic
> > applied. outside the device id table.  It does not scale, and it
> > does
> > not play well at all with dynamic IDs.  But I guess it's acceptable
> > for
> > two devices when it was ok for one, so
> > 
> > Acked-by: Bjørn Mork <bjorn@mork.no>
> > 
> > But I assume this isn't the last device we've seen with this issue.
> > Someone has already mention Quectel EM20. Maybe convert this code
> > to a
> > quirk for the next device and leave all the device specific magic
> > in the
> > device id table?  Could even add a macro for the class match +
> > quirk
> > thing.
> 
> (Appologies to Bjørn for receiving multiple copies of this email - I
> forgot that the Gmail app sends HTML-encoded emails)
> 
> Thanks for your feedback. My reasoning was the same. For one or two
> devices, the current approach is (probably) ok. Any more, and a
> different approach is needed.
> 
> Looking through the documentation I have, it seems like most
> Quectel-modems support the command used to change the
> USB-configuration. Thus, I wouldn’t be surprised if all or most
> existing Quectel-devices should have this quirk applied. I have a
> couple of older modules, so I will look at how they behave with
> different configurations. I will then either add the quirk soon or
> when for example EM20 is available (unless someone else does it
> earlier :)).

Yeah, this should really be a new quirk from here on.

Dan
diff mbox series

Patch

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 18af2f8eee96..74bebbdb4b15 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -976,6 +976,13 @@  static const struct usb_device_id products[] = {
 					      0xff),
 		.driver_info	    = (unsigned long)&qmi_wwan_info_quirk_dtr,
 	},
+	{	/* Quectel EG12/EM12 */
+		USB_DEVICE_AND_INTERFACE_INFO(0x2c7c, 0x0512,
+					      USB_CLASS_VENDOR_SPEC,
+					      USB_SUBCLASS_VENDOR_SPEC,
+					      0xff),
+		.driver_info	    = (unsigned long)&qmi_wwan_info_quirk_dtr,
+	},
 
 	/* 3. Combined interface devices matching on interface number */
 	{QMI_FIXED_INTF(0x0408, 0xea42, 4)},	/* Yota / Megafon M100-1 */
@@ -1343,17 +1350,20 @@  static bool quectel_ec20_detected(struct usb_interface *intf)
 	return false;
 }
 
-static bool quectel_ep06_diag_detected(struct usb_interface *intf)
+static bool quectel_diag_detected(struct usb_interface *intf)
 {
 	struct usb_device *dev = interface_to_usbdev(intf);
 	struct usb_interface_descriptor intf_desc = intf->cur_altsetting->desc;
+	u16 id_vendor = le16_to_cpu(dev->descriptor.idVendor);
+	u16 id_product = le16_to_cpu(dev->descriptor.idProduct);
 
-	if (le16_to_cpu(dev->descriptor.idVendor) == 0x2c7c &&
-	    le16_to_cpu(dev->descriptor.idProduct) == 0x0306 &&
-	    intf_desc.bNumEndpoints == 2)
-		return true;
+	if (id_vendor != 0x2c7c || intf_desc.bNumEndpoints != 2)
+		return false;
 
-	return false;
+	if (id_product == 0x0306 || id_product == 0x0512)
+		return true;
+	else
+		return false;
 }
 
 static int qmi_wwan_probe(struct usb_interface *intf,
@@ -1390,13 +1400,13 @@  static int qmi_wwan_probe(struct usb_interface *intf,
 		return -ENODEV;
 	}
 
-	/* Quectel EP06/EM06/EG06 supports dynamic interface configuration, so
+	/* Several Quectel modems supports dynamic interface configuration, so
 	 * we need to match on class/subclass/protocol. These values are
 	 * identical for the diagnostic- and QMI-interface, but bNumEndpoints is
 	 * different. Ignore the current interface if the number of endpoints
 	 * the number for the diag interface (two).
 	 */
-	if (quectel_ep06_diag_detected(intf))
+	if (quectel_diag_detected(intf))
 		return -ENODEV;
 
 	return usbnet_probe(intf, id);