Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles

[Måns Rullgård]

Johan Hovold <johan@xxxxxxxxxx> writes:

> Adding Bjørn.
>
> On Wed, Feb 27, 2019 at 11:57:16AM +0000, Måns Rullgård wrote:
>> Johan Hovold <johan@xxxxxxxxxx> writes:
>> 
>> > On Tue, Feb 26, 2019 at 05:07:10PM +0000, Mans Rullgard wrote:
>> >> The SIMCom SIM5218 and compatible devices have 5 USB interfaces, only 4
>> >> of which are serial ports.  The fifth is a network interface supported
>> >> by the qmi-wwan driver.  Furthermore, the serial ports do not support
>> >> modem control signals.  Add driver_info flags to reflect this.
>> >> 
>> >> Signed-off-by: Mans Rullgard <mans@xxxxxxxxx>
>> >> ---
>> >>  drivers/usb/serial/option.c | 3 ++-
>> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
>> >> index fb544340888b..af4cbfecc3ff 100644
>> >> --- a/drivers/usb/serial/option.c
>> >> +++ b/drivers/usb/serial/option.c
>> >> @@ -1066,7 +1066,8 @@ static const struct usb_device_id option_ids[] = {
>> >>  	  .driver_info = RSVD(3) },
>> >>  	{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x6613)}, /* Onda H600/ZTE MF330 */
>> >>  	{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x0023)}, /* ONYX 3G device */
>> >> -	{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x9000)}, /* SIMCom SIM5218 */
>> >> +	{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x9000), /* SIMCom SIM5218 */
>> >> +	  .driver_info = NCTRL(0) | NCTRL(1) | NCTRL(2) | NCTRL(3) | RSVD(4) },
>> >>  	/* Quectel products using Qualcomm vendor ID */
>> >>  	{ USB_DEVICE(QUALCOMM_VENDOR_ID, QUECTEL_PRODUCT_UC15)},
>> >>  	{ USB_DEVICE(QUALCOMM_VENDOR_ID, QUECTEL_PRODUCT_UC20),
>> >
>> > Could you please provide the output of usb-devices (or lsusb -v) for
>> > this device?
>> 
>> lsusb -v:
>> [...]

> So the patch looks fine to me. The fifth interface is QMI, but hasn't
> been available for use until now then, and this seems to have been the
> vendors idea from the start:
>
> 	http://www.microchip.ua/simcom/WCDMA/APPNOTES/SIMCom_3G_Linux_driver_Application%20Note_V1.00.pdf

That document predates the qmi-wwan driver in the kernel.  Note that
this driver has an ID table entry for interface 4 of this device.  Right
now, whichever driver is probed first claims that interface.  I haven't
actually tried using the QMI interface, though.

> And you're seeing errors when opening ports 0-3 due to the DTR calls
> which I guess no one noticed or cared about before?

Right, some userspace tools complain about this.

> Before you sent me the lsusb I searched for it and came across the below
> thread where Bjørn's having a go at SIMCom. In it there's output from a
> second device using the same id but with entirely different descriptors.
>
> 	https://forum.openwrt.org/t/lte-wireless-module-support-by-openwrt-led-on-tplink/13586?page=3
>
> If this is a common theme with this vendor we may need to be extra
> careful when making changes.

Isn't this a common theme with most USB vendors, especially wireless things?

Regardless, setting the NCTRL flag should be harmless.

-- 
Måns Rullgård