All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-26 17:07 ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Mans Rullgard @ 2019-02-26 17:07 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

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),
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-26 17:07 ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-02-26 17:07 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

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),

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-27  8:33   ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-02-27  8:33 UTC (permalink / raw)
  To: Mans Rullgard; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

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?

Thanks,
Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-27  8:33   ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-02-27  8:33 UTC (permalink / raw)
  To: Mans Rullgard; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

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?

Thanks,
Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-27 11:57     ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-02-27 11:57 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

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:
Bus 001 Device 003: ID 05c6:9000 Qualcomm, Inc. SIMCom SIM5218 modem
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x05c6 Qualcomm, Inc.
  idProduct          0x9000 SIMCom SIM5218 modem
  bcdDevice            0.00
  iManufacturer           3 SimTech, Incorporated
  iProduct                2 SimTech SIM5360
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength          138
    bNumInterfaces          5
    bConfigurationValue     1
    iConfiguration          1 SimTech Configuration
    bmAttributes         0xe0
      Self Powered
      Remote Wakeup
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        3
      bAlternateSetting       0
      bNumEndpoints           3
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x84  EP 4 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               5
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x85  EP 5 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x04  EP 4 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        4
      bAlternateSetting       0
      bNumEndpoints           3
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x86  EP 6 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               5
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x87  EP 7 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x05  EP 5 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass            0 
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  bNumConfigurations      1
can't get debug descriptor: Resource temporarily unavailable
Device Status:     0x0000
  (Bus Powered)

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-27 11:57     ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-02-27 11:57 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

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:
Bus 001 Device 003: ID 05c6:9000 Qualcomm, Inc. SIMCom SIM5218 modem
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x05c6 Qualcomm, Inc.
  idProduct          0x9000 SIMCom SIM5218 modem
  bcdDevice            0.00
  iManufacturer           3 SimTech, Incorporated
  iProduct                2 SimTech SIM5360
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength          138
    bNumInterfaces          5
    bConfigurationValue     1
    iConfiguration          1 SimTech Configuration
    bmAttributes         0xe0
      Self Powered
      Remote Wakeup
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        3
      bAlternateSetting       0
      bNumEndpoints           3
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x84  EP 4 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               5
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x85  EP 5 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x04  EP 4 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        4
      bAlternateSetting       0
      bNumEndpoints           3
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x86  EP 6 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               5
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x87  EP 7 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x05  EP 5 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval              32
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass            0 
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  bNumConfigurations      1
can't get debug descriptor: Resource temporarily unavailable
Device Status:     0x0000
  (Bus Powered)

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-27 13:13       ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-02-27 13:13 UTC (permalink / raw)
  To: Måns Rullgård, Bjørn Mork
  Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

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:
> Bus 001 Device 003: ID 05c6:9000 Qualcomm, Inc. SIMCom SIM5218 modem
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               2.00
>   bDeviceClass            0 
>   bDeviceSubClass         0 
>   bDeviceProtocol         0 
>   bMaxPacketSize0        64
>   idVendor           0x05c6 Qualcomm, Inc.
>   idProduct          0x9000 SIMCom SIM5218 modem
>   bcdDevice            0.00
>   iManufacturer           3 SimTech, Incorporated
>   iProduct                2 SimTech SIM5360
>   iSerial                 0 
>   bNumConfigurations      1
>   Configuration Descriptor:
>     bLength                 9
>     bDescriptorType         2
>     wTotalLength          138
>     bNumInterfaces          5
>     bConfigurationValue     1
>     iConfiguration          1 SimTech Configuration
>     bmAttributes         0xe0
>       Self Powered
>       Remote Wakeup
>     MaxPower              500mA
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        0
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x81  EP 1 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x01  EP 1 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        1
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x82  EP 2 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x02  EP 2 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        2
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x83  EP 3 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x03  EP 3 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        3
>       bAlternateSetting       0
>       bNumEndpoints           3
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x84  EP 4 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               5
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x85  EP 5 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x04  EP 4 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        4
>       bAlternateSetting       0
>       bNumEndpoints           3
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x86  EP 6 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               5
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x87  EP 7 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x05  EP 5 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
> Device Qualifier (for other device speed):
>   bLength                10
>   bDescriptorType         6
>   bcdUSB               2.00
>   bDeviceClass            0 
>   bDeviceSubClass         0 
>   bDeviceProtocol         0 
>   bMaxPacketSize0        64
>   bNumConfigurations      1
> can't get debug descriptor: Resource temporarily unavailable
> Device Status:     0x0000
>   (Bus Powered)

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

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?

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.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-27 13:13       ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-02-27 13:13 UTC (permalink / raw)
  To: Måns Rullgård, Bjørn Mork
  Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

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:
> Bus 001 Device 003: ID 05c6:9000 Qualcomm, Inc. SIMCom SIM5218 modem
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               2.00
>   bDeviceClass            0 
>   bDeviceSubClass         0 
>   bDeviceProtocol         0 
>   bMaxPacketSize0        64
>   idVendor           0x05c6 Qualcomm, Inc.
>   idProduct          0x9000 SIMCom SIM5218 modem
>   bcdDevice            0.00
>   iManufacturer           3 SimTech, Incorporated
>   iProduct                2 SimTech SIM5360
>   iSerial                 0 
>   bNumConfigurations      1
>   Configuration Descriptor:
>     bLength                 9
>     bDescriptorType         2
>     wTotalLength          138
>     bNumInterfaces          5
>     bConfigurationValue     1
>     iConfiguration          1 SimTech Configuration
>     bmAttributes         0xe0
>       Self Powered
>       Remote Wakeup
>     MaxPower              500mA
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        0
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x81  EP 1 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x01  EP 1 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        1
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x82  EP 2 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x02  EP 2 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        2
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x83  EP 3 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x03  EP 3 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        3
>       bAlternateSetting       0
>       bNumEndpoints           3
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x84  EP 4 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               5
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x85  EP 5 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x04  EP 4 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        4
>       bAlternateSetting       0
>       bNumEndpoints           3
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x86  EP 6 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               5
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x87  EP 7 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x05  EP 5 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval              32
> Device Qualifier (for other device speed):
>   bLength                10
>   bDescriptorType         6
>   bcdUSB               2.00
>   bDeviceClass            0 
>   bDeviceSubClass         0 
>   bDeviceProtocol         0 
>   bMaxPacketSize0        64
>   bNumConfigurations      1
> can't get debug descriptor: Resource temporarily unavailable
> Device Status:     0x0000
>   (Bus Powered)

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

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?

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.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-27 14:32         ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-02-27 14:32 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

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.

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-02-27 14:32         ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-02-27 14:32 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

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.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-18 14:32           ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-18 14:32 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-18 14:32           ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-18 14:32 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

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?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 10:28           ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-03-19 10:28 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

On Wed, Feb 27, 2019 at 02:32:58PM +0000, Måns Rullgård wrote:
> 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.

I didn't say it was correct, just that the vendor proposed binding to it
anyway.

> > 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.

Hmm. You shouldn't see any errors on open (they're not even logged), but
I guess your user space tools complains on receiving -EPROTO instead of
-EINVAL when trying to manage these signals directly?

> > 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.

Well, there are devices that depend on getting these requests, at least
for the QMI interface. But we can always revert if anyone complains.

Now applied, thanks.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 10:28           ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-03-19 10:28 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

On Wed, Feb 27, 2019 at 02:32:58PM +0000, Måns Rullgård wrote:
> 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.

I didn't say it was correct, just that the vendor proposed binding to it
anyway.

> > 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.

Hmm. You shouldn't see any errors on open (they're not even logged), but
I guess your user space tools complains on receiving -EPROTO instead of
-EINVAL when trying to manage these signals directly?

> > 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.

Well, there are devices that depend on getting these requests, at least
for the QMI interface. But we can always revert if anyone complains.

Now applied, thanks.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 10:54             ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-19 10:54 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

Johan Hovold <johan@kernel.org> writes:

> On Wed, Feb 27, 2019 at 02:32:58PM +0000, Måns Rullgård wrote:
>> 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.
>
> I didn't say it was correct, just that the vendor proposed binding to it
> anyway.
>
>> > 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.
>
> Hmm. You shouldn't see any errors on open (they're not even logged), but
> I guess your user space tools complains on receiving -EPROTO instead of
> -EINVAL when trying to manage these signals directly?

Yes, one specific case is pyserial.  On open, it attempts to set those
signals and, depending on the error returned, either aborts or marks
them as unavailable.

>> > 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.
>
> Well, there are devices that depend on getting these requests, at least
> for the QMI interface. But we can always revert if anyone complains.

The QMI interface doesn't even pretend to be a uart.  The other ones do,
but there isn't actually any real uart behind them.  For instance, it
doesn't matter what baud rate one sets.

> Now applied, thanks.

Thanks.

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 10:54             ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-19 10:54 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

Johan Hovold <johan@kernel.org> writes:

> On Wed, Feb 27, 2019 at 02:32:58PM +0000, Måns Rullgård wrote:
>> 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.
>
> I didn't say it was correct, just that the vendor proposed binding to it
> anyway.
>
>> > 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.
>
> Hmm. You shouldn't see any errors on open (they're not even logged), but
> I guess your user space tools complains on receiving -EPROTO instead of
> -EINVAL when trying to manage these signals directly?

Yes, one specific case is pyserial.  On open, it attempts to set those
signals and, depending on the error returned, either aborts or marks
them as unavailable.

>> > 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.
>
> Well, there are devices that depend on getting these requests, at least
> for the QMI interface. But we can always revert if anyone complains.

The QMI interface doesn't even pretend to be a uart.  The other ones do,
but there isn't actually any real uart behind them.  For instance, it
doesn't matter what baud rate one sets.

> Now applied, thanks.

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 11:08               ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-03-19 11:08 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
> Johan Hovold <johan@kernel.org> writes:

> >> Regardless, setting the NCTRL flag should be harmless.
> >
> > Well, there are devices that depend on getting these requests, at least
> > for the QMI interface. But we can always revert if anyone complains.
> 
> The QMI interface doesn't even pretend to be a uart.  The other ones do,
> but there isn't actually any real uart behind them.  For instance, it
> doesn't matter what baud rate one sets.

Sure, but some devices still require "DTR" to be set for the QMI
interface, so there not being any real uart is no guarantee that there
is no firmware that expects these calls.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 11:08               ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-03-19 11:08 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
> Johan Hovold <johan@kernel.org> writes:

> >> Regardless, setting the NCTRL flag should be harmless.
> >
> > Well, there are devices that depend on getting these requests, at least
> > for the QMI interface. But we can always revert if anyone complains.
> 
> The QMI interface doesn't even pretend to be a uart.  The other ones do,
> but there isn't actually any real uart behind them.  For instance, it
> doesn't matter what baud rate one sets.

Sure, but some devices still require "DTR" to be set for the QMI
interface, so there not being any real uart is no guarantee that there
is no firmware that expects these calls.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 12:25                 ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-19 12:25 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

Johan Hovold <johan@kernel.org> writes:

> On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
>> Johan Hovold <johan@kernel.org> writes:
>
>> >> Regardless, setting the NCTRL flag should be harmless.
>> >
>> > Well, there are devices that depend on getting these requests, at least
>> > for the QMI interface. But we can always revert if anyone complains.
>> 
>> The QMI interface doesn't even pretend to be a uart.  The other ones do,
>> but there isn't actually any real uart behind them.  For instance, it
>> doesn't matter what baud rate one sets.
>
> Sure, but some devices still require "DTR" to be set for the QMI
> interface, so there not being any real uart is no guarantee that there
> is no firmware that expects these calls.

Now I'm thoroughly confused.  The QMI interface has a completely
separate driver that creates a network device (if I'm reading the code
correctly).

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 12:25                 ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-19 12:25 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

Johan Hovold <johan@kernel.org> writes:

> On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
>> Johan Hovold <johan@kernel.org> writes:
>
>> >> Regardless, setting the NCTRL flag should be harmless.
>> >
>> > Well, there are devices that depend on getting these requests, at least
>> > for the QMI interface. But we can always revert if anyone complains.
>> 
>> The QMI interface doesn't even pretend to be a uart.  The other ones do,
>> but there isn't actually any real uart behind them.  For instance, it
>> doesn't matter what baud rate one sets.
>
> Sure, but some devices still require "DTR" to be set for the QMI
> interface, so there not being any real uart is no guarantee that there
> is no firmware that expects these calls.

Now I'm thoroughly confused.  The QMI interface has a completely
separate driver that creates a network device (if I'm reading the code
correctly).

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 12:27                   ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-03-19 12:27 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

On Tue, Mar 19, 2019 at 12:25:53PM +0000, Måns Rullgård wrote:
> Johan Hovold <johan@kernel.org> writes:
> 
> > On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
> >> Johan Hovold <johan@kernel.org> writes:
> >
> >> >> Regardless, setting the NCTRL flag should be harmless.
> >> >
> >> > Well, there are devices that depend on getting these requests, at least
> >> > for the QMI interface. But we can always revert if anyone complains.
> >> 
> >> The QMI interface doesn't even pretend to be a uart.  The other ones do,
> >> but there isn't actually any real uart behind them.  For instance, it
> >> doesn't matter what baud rate one sets.
> >
> > Sure, but some devices still require "DTR" to be set for the QMI
> > interface, so there not being any real uart is no guarantee that there
> > is no firmware that expects these calls.
> 
> Now I'm thoroughly confused.  The QMI interface has a completely
> separate driver that creates a network device (if I'm reading the code
> correctly).

I was just giving an example of firmware sometimes doing unexpected
things.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 12:27                   ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-03-19 12:27 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

On Tue, Mar 19, 2019 at 12:25:53PM +0000, Måns Rullgård wrote:
> Johan Hovold <johan@kernel.org> writes:
> 
> > On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
> >> Johan Hovold <johan@kernel.org> writes:
> >
> >> >> Regardless, setting the NCTRL flag should be harmless.
> >> >
> >> > Well, there are devices that depend on getting these requests, at least
> >> > for the QMI interface. But we can always revert if anyone complains.
> >> 
> >> The QMI interface doesn't even pretend to be a uart.  The other ones do,
> >> but there isn't actually any real uart behind them.  For instance, it
> >> doesn't matter what baud rate one sets.
> >
> > Sure, but some devices still require "DTR" to be set for the QMI
> > interface, so there not being any real uart is no guarantee that there
> > is no firmware that expects these calls.
> 
> Now I'm thoroughly confused.  The QMI interface has a completely
> separate driver that creates a network device (if I'm reading the code
> correctly).

I was just giving an example of firmware sometimes doing unexpected
things.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 12:43                     ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-03-19 12:43 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

On Tue, Mar 19, 2019 at 01:27:19PM +0100, Johan Hovold wrote:
> On Tue, Mar 19, 2019 at 12:25:53PM +0000, Måns Rullgård wrote:
> > Johan Hovold <johan@kernel.org> writes:
> > 
> > > On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
> > >> Johan Hovold <johan@kernel.org> writes:
> > >
> > >> >> Regardless, setting the NCTRL flag should be harmless.
> > >> >
> > >> > Well, there are devices that depend on getting these requests, at least
> > >> > for the QMI interface. But we can always revert if anyone complains.
> > >> 
> > >> The QMI interface doesn't even pretend to be a uart.  The other ones do,
> > >> but there isn't actually any real uart behind them.  For instance, it
> > >> doesn't matter what baud rate one sets.
> > >
> > > Sure, but some devices still require "DTR" to be set for the QMI
> > > interface, so there not being any real uart is no guarantee that there
> > > is no firmware that expects these calls.
> > 
> > Now I'm thoroughly confused.  The QMI interface has a completely
> > separate driver that creates a network device (if I'm reading the code
> > correctly).
> 
> I was just giving an example of firmware sometimes doing unexpected
> things.

See 93725149794d ("net: qmi_wwan: MDM9x30 specific power management")
for some background.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 12:43                     ` Johan Hovold
  0 siblings, 0 replies; 34+ messages in thread
From: Johan Hovold @ 2019-03-19 12:43 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

On Tue, Mar 19, 2019 at 01:27:19PM +0100, Johan Hovold wrote:
> On Tue, Mar 19, 2019 at 12:25:53PM +0000, Måns Rullgård wrote:
> > Johan Hovold <johan@kernel.org> writes:
> > 
> > > On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
> > >> Johan Hovold <johan@kernel.org> writes:
> > >
> > >> >> Regardless, setting the NCTRL flag should be harmless.
> > >> >
> > >> > Well, there are devices that depend on getting these requests, at least
> > >> > for the QMI interface. But we can always revert if anyone complains.
> > >> 
> > >> The QMI interface doesn't even pretend to be a uart.  The other ones do,
> > >> but there isn't actually any real uart behind them.  For instance, it
> > >> doesn't matter what baud rate one sets.
> > >
> > > Sure, but some devices still require "DTR" to be set for the QMI
> > > interface, so there not being any real uart is no guarantee that there
> > > is no firmware that expects these calls.
> > 
> > Now I'm thoroughly confused.  The QMI interface has a completely
> > separate driver that creates a network device (if I'm reading the code
> > correctly).
> 
> I was just giving an example of firmware sometimes doing unexpected
> things.

See 93725149794d ("net: qmi_wwan: MDM9x30 specific power management")
for some background.

Johan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 14:30                       ` Dan Williams
  0 siblings, 0 replies; 34+ messages in thread
From: Dan Williams @ 2019-03-19 14:30 UTC (permalink / raw)
  To: Johan Hovold, Måns Rullgård
  Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

On Tue, 2019-03-19 at 13:43 +0100, Johan Hovold wrote:
> On Tue, Mar 19, 2019 at 01:27:19PM +0100, Johan Hovold wrote:
> > On Tue, Mar 19, 2019 at 12:25:53PM +0000, Måns Rullgård wrote:
> > > Johan Hovold <johan@kernel.org> writes:
> > > 
> > > > On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
> > > > > Johan Hovold <johan@kernel.org> writes:
> > > > > > > Regardless, setting the NCTRL flag should be harmless.
> > > > > > 
> > > > > > Well, there are devices that depend on getting these
> > > > > > requests, at least
> > > > > > for the QMI interface. But we can always revert if anyone
> > > > > > complains.
> > > > > 
> > > > > The QMI interface doesn't even pretend to be a uart.  The
> > > > > other ones do,
> > > > > but there isn't actually any real uart behind them.  For
> > > > > instance, it
> > > > > doesn't matter what baud rate one sets.
> > > > 
> > > > Sure, but some devices still require "DTR" to be set for the
> > > > QMI
> > > > interface, so there not being any real uart is no guarantee
> > > > that there
> > > > is no firmware that expects these calls.
> > > 
> > > Now I'm thoroughly confused.  The QMI interface has a completely
> > > separate driver that creates a network device (if I'm reading the
> > > code
> > > correctly).
> > 
> > I was just giving an example of firmware sometimes doing unexpected
> > things.
> 
> See 93725149794d ("net: qmi_wwan: MDM9x30 specific power management")
> for some background.

TLDR; some firmware uses the DTR signal as an indicator to come out of
low-power mode. Without doing so you cannot talk to the modem over any
of it's ports, QMI, net, or serial.

Dan


^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 14:30                       ` Dan Williams
  0 siblings, 0 replies; 34+ messages in thread
From: Dan Williams @ 2019-03-19 14:30 UTC (permalink / raw)
  To: Johan Hovold, Måns Rullgård
  Cc: Bjørn Mork, Greg Kroah-Hartman, linux-usb, linux-kernel

On Tue, 2019-03-19 at 13:43 +0100, Johan Hovold wrote:
> On Tue, Mar 19, 2019 at 01:27:19PM +0100, Johan Hovold wrote:
> > On Tue, Mar 19, 2019 at 12:25:53PM +0000, Måns Rullgård wrote:
> > > Johan Hovold <johan@kernel.org> writes:
> > > 
> > > > On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
> > > > > Johan Hovold <johan@kernel.org> writes:
> > > > > > > Regardless, setting the NCTRL flag should be harmless.
> > > > > > 
> > > > > > Well, there are devices that depend on getting these
> > > > > > requests, at least
> > > > > > for the QMI interface. But we can always revert if anyone
> > > > > > complains.
> > > > > 
> > > > > The QMI interface doesn't even pretend to be a uart.  The
> > > > > other ones do,
> > > > > but there isn't actually any real uart behind them.  For
> > > > > instance, it
> > > > > doesn't matter what baud rate one sets.
> > > > 
> > > > Sure, but some devices still require "DTR" to be set for the
> > > > QMI
> > > > interface, so there not being any real uart is no guarantee
> > > > that there
> > > > is no firmware that expects these calls.
> > > 
> > > Now I'm thoroughly confused.  The QMI interface has a completely
> > > separate driver that creates a network device (if I'm reading the
> > > code
> > > correctly).
> > 
> > I was just giving an example of firmware sometimes doing unexpected
> > things.
> 
> See 93725149794d ("net: qmi_wwan: MDM9x30 specific power management")
> for some background.

TLDR; some firmware uses the DTR signal as an indicator to come out of
low-power mode. Without doing so you cannot talk to the modem over any
of it's ports, QMI, net, or serial.

Dan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 14:35                         ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-19 14:35 UTC (permalink / raw)
  To: Dan Williams
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

Dan Williams <dcbw@redhat.com> writes:

> On Tue, 2019-03-19 at 13:43 +0100, Johan Hovold wrote:
>> On Tue, Mar 19, 2019 at 01:27:19PM +0100, Johan Hovold wrote:
>> > On Tue, Mar 19, 2019 at 12:25:53PM +0000, Måns Rullgård wrote:
>> > > Johan Hovold <johan@kernel.org> writes:
>> > > 
>> > > > On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
>> > > > > Johan Hovold <johan@kernel.org> writes:
>> > > > > > > Regardless, setting the NCTRL flag should be harmless.
>> > > > > > 
>> > > > > > Well, there are devices that depend on getting these
>> > > > > > requests, at least
>> > > > > > for the QMI interface. But we can always revert if anyone
>> > > > > > complains.
>> > > > > 
>> > > > > The QMI interface doesn't even pretend to be a uart.  The
>> > > > > other ones do,
>> > > > > but there isn't actually any real uart behind them.  For
>> > > > > instance, it
>> > > > > doesn't matter what baud rate one sets.
>> > > > 
>> > > > Sure, but some devices still require "DTR" to be set for the
>> > > > QMI
>> > > > interface, so there not being any real uart is no guarantee
>> > > > that there
>> > > > is no firmware that expects these calls.
>> > > 
>> > > Now I'm thoroughly confused.  The QMI interface has a completely
>> > > separate driver that creates a network device (if I'm reading the
>> > > code
>> > > correctly).
>> > 
>> > I was just giving an example of firmware sometimes doing unexpected
>> > things.
>> 
>> See 93725149794d ("net: qmi_wwan: MDM9x30 specific power management")
>> for some background.
>
> TLDR; some firmware uses the DTR signal as an indicator to come out of
> low-power mode. Without doing so you cannot talk to the modem over any
> of it's ports, QMI, net, or serial.

I must be missing something, but how does a network interface have a DTR
signal?

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 14:35                         ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-19 14:35 UTC (permalink / raw)
  To: Dan Williams
  Cc: Johan Hovold, Bjørn Mork, Greg Kroah-Hartman, linux-usb,
	linux-kernel

Dan Williams <dcbw@redhat.com> writes:

> On Tue, 2019-03-19 at 13:43 +0100, Johan Hovold wrote:
>> On Tue, Mar 19, 2019 at 01:27:19PM +0100, Johan Hovold wrote:
>> > On Tue, Mar 19, 2019 at 12:25:53PM +0000, Måns Rullgård wrote:
>> > > Johan Hovold <johan@kernel.org> writes:
>> > > 
>> > > > On Tue, Mar 19, 2019 at 10:54:00AM +0000, Måns Rullgård wrote:
>> > > > > Johan Hovold <johan@kernel.org> writes:
>> > > > > > > Regardless, setting the NCTRL flag should be harmless.
>> > > > > > 
>> > > > > > Well, there are devices that depend on getting these
>> > > > > > requests, at least
>> > > > > > for the QMI interface. But we can always revert if anyone
>> > > > > > complains.
>> > > > > 
>> > > > > The QMI interface doesn't even pretend to be a uart.  The
>> > > > > other ones do,
>> > > > > but there isn't actually any real uart behind them.  For
>> > > > > instance, it
>> > > > > doesn't matter what baud rate one sets.
>> > > > 
>> > > > Sure, but some devices still require "DTR" to be set for the
>> > > > QMI
>> > > > interface, so there not being any real uart is no guarantee
>> > > > that there
>> > > > is no firmware that expects these calls.
>> > > 
>> > > Now I'm thoroughly confused.  The QMI interface has a completely
>> > > separate driver that creates a network device (if I'm reading the
>> > > code
>> > > correctly).
>> > 
>> > I was just giving an example of firmware sometimes doing unexpected
>> > things.
>> 
>> See 93725149794d ("net: qmi_wwan: MDM9x30 specific power management")
>> for some background.
>
> TLDR; some firmware uses the DTR signal as an indicator to come out of
> low-power mode. Without doing so you cannot talk to the modem over any
> of it's ports, QMI, net, or serial.

I must be missing something, but how does a network interface have a DTR
signal?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 14:59                           ` Bjørn Mork
  0 siblings, 0 replies; 34+ messages in thread
From: Bjørn Mork @ 2019-03-19 14:59 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Dan Williams, Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

Måns Rullgård <mans@mansr.com> writes:
> Dan Williams <dcbw@redhat.com> writes:
>
>> TLDR; some firmware uses the DTR signal as an indicator to come out of
>> low-power mode. Without doing so you cannot talk to the modem over any
>> of it's ports, QMI, net, or serial.
>
> I must be missing something, but how does a network interface have a DTR
> signal?

It does not.  But the network function is (ab)using the Comm class USB
control message "SetControlLineState" to signal "wake up" from the host
to the device.  Which is perfectly fine since the USB function in
question is vendor specific.  The vendor can define any USB control
message to mean anything they want. Reusing a Comm class message
probably made sense to the firmware engineer designing this.  We can
only note and adapt.

It doesn't have anything to do with DTR.


Bjørn

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 14:59                           ` Bjørn Mork
  0 siblings, 0 replies; 34+ messages in thread
From: Bjørn Mork @ 2019-03-19 14:59 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Dan Williams, Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

Måns Rullgård <mans@mansr.com> writes:
> Dan Williams <dcbw@redhat.com> writes:
>
>> TLDR; some firmware uses the DTR signal as an indicator to come out of
>> low-power mode. Without doing so you cannot talk to the modem over any
>> of it's ports, QMI, net, or serial.
>
> I must be missing something, but how does a network interface have a DTR
> signal?

It does not.  But the network function is (ab)using the Comm class USB
control message "SetControlLineState" to signal "wake up" from the host
to the device.  Which is perfectly fine since the USB function in
question is vendor specific.  The vendor can define any USB control
message to mean anything they want. Reusing a Comm class message
probably made sense to the firmware engineer designing this.  We can
only note and adapt.

It doesn't have anything to do with DTR.


Bjørn

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 16:26                             ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-19 16:26 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Dan Williams, Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

Bjørn Mork <bjorn@mork.no> writes:

> Måns Rullgård <mans@mansr.com> writes:
>> Dan Williams <dcbw@redhat.com> writes:
>>
>>> TLDR; some firmware uses the DTR signal as an indicator to come out of
>>> low-power mode. Without doing so you cannot talk to the modem over any
>>> of it's ports, QMI, net, or serial.
>>
>> I must be missing something, but how does a network interface have a DTR
>> signal?
>
> It does not.  But the network function is (ab)using the Comm class USB
> control message "SetControlLineState" to signal "wake up" from the host
> to the device.  Which is perfectly fine since the USB function in
> question is vendor specific.  The vendor can define any USB control
> message to mean anything they want. Reusing a Comm class message
> probably made sense to the firmware engineer designing this.  We can
> only note and adapt.
>
> It doesn't have anything to do with DTR.

Every time you think USB can't get any worse, it does.

-- 
Måns Rullgård

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 16:26                             ` Måns Rullgård
  0 siblings, 0 replies; 34+ messages in thread
From: Måns Rullgård @ 2019-03-19 16:26 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Dan Williams, Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

Bjørn Mork <bjorn@mork.no> writes:

> Måns Rullgård <mans@mansr.com> writes:
>> Dan Williams <dcbw@redhat.com> writes:
>>
>>> TLDR; some firmware uses the DTR signal as an indicator to come out of
>>> low-power mode. Without doing so you cannot talk to the modem over any
>>> of it's ports, QMI, net, or serial.
>>
>> I must be missing something, but how does a network interface have a DTR
>> signal?
>
> It does not.  But the network function is (ab)using the Comm class USB
> control message "SetControlLineState" to signal "wake up" from the host
> to the device.  Which is perfectly fine since the USB function in
> question is vendor specific.  The vendor can define any USB control
> message to mean anything they want. Reusing a Comm class message
> probably made sense to the firmware engineer designing this.  We can
> only note and adapt.
>
> It doesn't have anything to do with DTR.

Every time you think USB can't get any worse, it does.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 17:15                               ` Bjørn Mork
  0 siblings, 0 replies; 34+ messages in thread
From: Bjørn Mork @ 2019-03-19 17:15 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Dan Williams, Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

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

> Every time you think USB can't get any worse, it does.

You can't blame vendor creativity on the bus spec.  Look at what the
same vendors do to e.g. SCSI to make these firmwares appear as different
USB devices.  The fact that some of then use "eject" as a signal to
reboot the firmware into some other mode is not a SCSI problem.

But I do agree that USB should have dropped "vendor specific" and
mandated class functions for everything.  In theory. In practice, that
would probably have casued USB to lose to some other competing spec with
vendor specific as an option...



Bjørn

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: serial: option: set driver_info for SIM5218 and compatibles
@ 2019-03-19 17:15                               ` Bjørn Mork
  0 siblings, 0 replies; 34+ messages in thread
From: Bjørn Mork @ 2019-03-19 17:15 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Dan Williams, Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel

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

> Every time you think USB can't get any worse, it does.

You can't blame vendor creativity on the bus spec.  Look at what the
same vendors do to e.g. SCSI to make these firmwares appear as different
USB devices.  The fact that some of then use "eject" as a signal to
reboot the firmware into some other mode is not a SCSI problem.

But I do agree that USB should have dropped "vendor specific" and
mandated class functions for everything.  In theory. In practice, that
would probably have casued USB to lose to some other competing spec with
vendor specific as an option...



Bjørn

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2019-03-19 17:15 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-26 17:07 [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles Mans Rullgard
2019-02-26 17:07 ` Måns Rullgård
2019-02-27  8:33 ` [PATCH] " Johan Hovold
2019-02-27  8:33   ` Johan Hovold
2019-02-27 11:57   ` [PATCH] " Måns Rullgård
2019-02-27 11:57     ` Måns Rullgård
2019-02-27 13:13     ` [PATCH] " Johan Hovold
2019-02-27 13:13       ` Johan Hovold
2019-02-27 14:32       ` [PATCH] " Måns Rullgård
2019-02-27 14:32         ` Måns Rullgård
2019-03-18 14:32         ` [PATCH] " Måns Rullgård
2019-03-18 14:32           ` Måns Rullgård
2019-03-19 10:28         ` [PATCH] " Johan Hovold
2019-03-19 10:28           ` Johan Hovold
2019-03-19 10:54           ` [PATCH] " Måns Rullgård
2019-03-19 10:54             ` Måns Rullgård
2019-03-19 11:08             ` [PATCH] " Johan Hovold
2019-03-19 11:08               ` Johan Hovold
2019-03-19 12:25               ` [PATCH] " Måns Rullgård
2019-03-19 12:25                 ` Måns Rullgård
2019-03-19 12:27                 ` [PATCH] " Johan Hovold
2019-03-19 12:27                   ` Johan Hovold
2019-03-19 12:43                   ` [PATCH] " Johan Hovold
2019-03-19 12:43                     ` Johan Hovold
2019-03-19 14:30                     ` [PATCH] " Dan Williams
2019-03-19 14:30                       ` Dan Williams
2019-03-19 14:35                       ` [PATCH] " Måns Rullgård
2019-03-19 14:35                         ` Måns Rullgård
2019-03-19 14:59                         ` [PATCH] " Bjørn Mork
2019-03-19 14:59                           ` Bjørn Mork
2019-03-19 16:26                           ` [PATCH] " Måns Rullgård
2019-03-19 16:26                             ` Måns Rullgård
2019-03-19 17:15                             ` [PATCH] " Bjørn Mork
2019-03-19 17:15                               ` Bjørn Mork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.