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

["Måns Rullgård" <mans@mansr.com>]

Måns Rullgård <mans@mansr.com> writes:

> Johan Hovold <johan@kernel.org> writes:
>
>> Adding Bjørn.
>>
>> On Wed, Feb 27, 2019 at 11:57:16AM +0000, Måns Rullgård wrote:
>>> Johan Hovold <johan@kernel.org> 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@mansr.com>
>>> >> ---
>>> >>  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.

Any further comments on this?

-- 
Måns Rullgård