Re: [PATCH] USB: serial: option: add UNISOC vendor and TOZED LT70C product

From: Arınç ÜNAL
Date: Mon Apr 17 2023 - 07:02:58 EST


On 13.04.2023 18:14, Johan Hovold wrote:
On Thu, Apr 06, 2023 at 08:50:04AM +0300, arinc9.unal@xxxxxxxxx wrote:
From: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>

Add UNISOC vendor ID and TOZED LT70-C modem which is based from UNISOC
SL8563. The modem supports the NCM mode.

Thanks for the patch. Looks mostly good, but see my comments below.

Thanks a lot for looking at this Johan!


T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 6 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1782 ProdID=4055 Rev=04.04
S: Manufacturer=Unisoc Phone
S: Product=Unisoc Phone
S: SerialNumber=<redacted>
C: #Ifs=14 Cfg#= 1 Atr=c0 MxPwr=500mA
I: If#= 0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0d Prot=00 Driver=cdc_ncm
E: Ad=82(I) Atr=03(Int.) MxPS= 16 Ivl=32ms
I: If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_ncm
E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I: If#=10 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
E: Ad=07(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=8b(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I: If#=11 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
E: Ad=08(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=8c(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I: If#=12 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=option
E: Ad=09(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=8d(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms

This looks like an ADB interface which should be blacklisted (reserved)
so that the driver does not bind to it.

Will do.


I: If#=13 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
E: Ad=0a(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=8e(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I: If#= 2 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0d Prot=00 Driver=cdc_ncm
E: Ad=84(I) Atr=03(Int.) MxPS= 16 Ivl=32ms
I: If#= 3 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_ncm
E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I: If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0d Prot=00 Driver=cdc_ncm
E: Ad=86(I) Atr=03(Int.) MxPS= 16 Ivl=32ms
I: If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_ncm
E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I: If#= 6 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0d Prot=00 Driver=cdc_ncm
E: Ad=88(I) Atr=03(Int.) MxPS= 16 Ivl=32ms
I: If#= 7 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_ncm
E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=87(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I: If#= 8 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=89(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I: If#= 9 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
E: Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=8a(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms

Bus 001 Device 002: ID 1782:4055 Unisoc Phone Unisoc Phone
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x1782
idProduct 0x4055
bcdDevice 4.04
iManufacturer 1 Unisoc Phone
iProduct 2 Unisoc Phone
iSerial 3 <redacted>
bNumConfigurations 1

When resending you can put the verbose lsusb output below the cut-off
line (---) so that we have it in the mail archives if ever needed.

Will do.


Could you also add something what the various serial interfaces are used
for?

Me and Conan only know the first interface which is where the AT commands are run. I will mention this and the ADB interface on v2.


Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
---

I did not receive any comments on RFC so I'm sending this as is.

Arınç

---
drivers/usb/serial/option.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index f31cc3c76329..65a050a9ca39 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -595,6 +595,11 @@ static void option_instat_callback(struct urb *urb);
#define SIERRA_VENDOR_ID 0x1199
#define SIERRA_PRODUCT_EM9191 0x90d3
+/* UNISOC (Spreadtrum) products */
+#define UNISOC_VENDOR_ID 0x1782
+/* TOZED TL70-C based on UNISOC SL8563 uses UNISOC's vendor ID */
+#define TOZED_PRODUCT_LT70C 0x4055
+
/* Device flags */
/* Highest interface number which can be used with NCTRL() and RSVD() */
@@ -2225,6 +2230,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(OPPO_VENDOR_ID, OPPO_PRODUCT_R11, 0xff, 0xff, 0x30) },
{ USB_DEVICE_AND_INTERFACE_INFO(SIERRA_VENDOR_ID, SIERRA_PRODUCT_EM9191, 0xff, 0xff, 0x30) },
{ USB_DEVICE_AND_INTERFACE_INFO(SIERRA_VENDOR_ID, SIERRA_PRODUCT_EM9191, 0xff, 0, 0) },
+ { USB_DEVICE(UNISOC_VENDOR_ID, TOZED_PRODUCT_LT70C) },

You should match also on the interface class so that you don't try to
bind to the cdc interfaces. See USB_DEVICE_INTERFACE_CLASS() (and
RSVD() for the ADB interface).

Thanks, the final result should look like this:

{ USB_DEVICE_INTERFACE_CLASS(UNISOC_VENDOR_ID, TOZED_PRODUCT_LT70C, 0xff),
.driver_info = RSVD(12) },

Arınç