mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Victor Fragoso <victorffs@hotmail.com>
To: "larsm17@gmail.com" <larsm17@gmail.com>,
	"johan@kernel.org" <johan@kernel.org>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] USB: serial: option: add Fibocom L7xx modules
Date: Fri, 27 Oct 2023 17:55:21 +0000	[thread overview]
Message-ID: <94477736e69cc76eaaef8584d7e1aa5078a0611e.camel@hotmail.com> (raw)
In-Reply-To: <84a78bb8-fd85-4ee5-9c92-859e8450a587@gmail.com>

On Thu, 2023-10-26 at 20:13 +0700, Lars Melin wrote:
> On 10/26/2023 8:24, Victor Fragoso wrote:
> > Add support for Fibocom L7xx module series and variants.
> > 
> > L716-EU-60 (ECM):
> > T:  Bus=03 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 17 Spd=480  MxCh= 0
> > D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> > P:  Vendor=19d2 ProdID=0579 Rev= 1.00
> > S:  Manufacturer=Fibocom,Incorporated
> > S:  Product=Fibocom Mobile Boardband
> > S:  SerialNumber=1234567890ABCDEF
> > C:* #Ifs= 7 Cfg#= 1 Atr=e0 MxPwr=500mA
> > A:  FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=06 Prot=00
> > I:* If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=06 Prot=00 Driver=cdc_ether
> > E:  Ad=87(I) Atr=03(Int.) MxPS=  16 Ivl=32ms
> > I:  If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
> > I:* If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
> > E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 4 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 6 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
> > E:  Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > 
> > L716-EU-60 (RNDIS):
> > T:  Bus=03 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 21 Spd=480  MxCh= 0
> > D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> > P:  Vendor=2cb7 ProdID=0001 Rev= 1.00
> > S:  Manufacturer=Fibocom,Incorporated
> > S:  Product=Fibocom Mobile Boardband
> > S:  SerialNumber=1234567890ABCDEF
> > C:* #Ifs= 7 Cfg#= 1 Atr=e0 MxPwr=500mA
> > A:  FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=06 Prot=00
> > I:* If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=06 Prot=00 Driver=cdc_ether
> > E:  Ad=87(I) Atr=03(Int.) MxPS=  16 Ivl=32ms
> > I:  If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
> > I:* If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
> > E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 4 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 6 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
> > E:  Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > 
> > L716-EU-10 (ECM):
> > T:  Bus=03 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 21 Spd=480  MxCh= 0
> > D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> > P:  Vendor=2cb7 ProdID=0001 Rev= 1.00
> > S:  Manufacturer=Fibocom,Incorporated
> > S:  Product=Fibocom Mobile Boardband
> > S:  SerialNumber=1234567890ABCDEF
> > C:* #Ifs= 7 Cfg#= 1 Atr=e0 MxPwr=500mA
> > A:  FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=06 Prot=00
> > I:* If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=06 Prot=00 Driver=cdc_ether
> > E:  Ad=87(I) Atr=03(Int.) MxPS=  16 Ivl=32ms
> > I:  If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
> > I:* If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
> > E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 4 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > E:  Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > I:* If#= 6 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
> > E:  Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > E:  Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > 
> > Signed-off-by: Victor Fragoso <victorffs@hotmail.com>
> > ---
> >   drivers/usb/serial/option.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> > index 45dcfaadaf98..4ba3dc352d65 100644
> > --- a/drivers/usb/serial/option.c
> > +++ b/drivers/usb/serial/option.c
> > @@ -2262,6 +2262,11 @@ static const struct usb_device_id option_ids[] =
> > {
> >   	{ USB_DEVICE_INTERFACE_CLASS(0x2cb7, 0x01a2, 0xff)
> > },			/* Fibocom FM101-GL (laptop MBIM) */
> >   	{ USB_DEVICE_INTERFACE_CLASS(0x2cb7, 0x01a4,
> > 0xff),			/* Fibocom FM101-GL (laptop MBIM) */
> >   	  .driver_info = RSVD(4) },
> > +	{ USB_DEVICE_AND_INTERFACE_INFO(0x2cb7, 0x0001, 0xff, 0xff,
> > 0xff) },	/* Fibocom L71x */
> > +	{ USB_DEVICE_AND_INTERFACE_INFO(0x2cb7, 0x0001, 0x0a, 0x00,
> > 0xff) },	/* Fibocom L71x */
> > +	{ USB_DEVICE_AND_INTERFACE_INFO(0x2cb7, 0x0100, 0xff, 0xff,
> > 0xff) },	/* Fibocom L71x */
> > +	{ USB_DEVICE_AND_INTERFACE_INFO(0x19d2, 0x0256, 0xff, 0xff,
> > 0xff) },	/* Fibocom L71x */
> > +	{ USB_DEVICE_AND_INTERFACE_INFO(0x19d2, 0x0579, 0xff, 0xff,
> > 0xff) },	/* Fibocom L71x */
> >   	{ USB_DEVICE_INTERFACE_CLASS(0x2df3, 0x9d03, 0xff)
> > },			/* LongSung M5710 */
> >   	{ USB_DEVICE_INTERFACE_CLASS(0x305a, 0x1404, 0xff)
> > },			/* GosunCn GM500 RNDIS */
> >   	{ USB_DEVICE_INTERFACE_CLASS(0x305a, 0x1405, 0xff)
> > },			/* GosunCn GM500 MBIM */
> 
> 
> Hi Victor, thanks for the patch, there is unfortunately the following 
> errors in it:
> The device list is sorted in ascending order based on vid:pid, you have 
> inserted all of your added Id's in the wrong place.
> 
> 19d2:0579 is a ZTE device Id and should be placed among the other 19d2 
> devices.
> 
> You have not included usb-devices output for 19d2:0256 and 2cb7:0100, 
> and I have strong reasons to believe that they should not be included in 
> the option driver.
> If you are of another opinion then please show the usb-devices output 
> for them, otherwise remove them from the patch.
> 
> You have added support for an interface with the attributes 0a/00/ff , 
> there is no such interface in your provided usb-devices listing, 
> interface Class 0a does not even belong to the option driver.
> 
> Thanks
> Lars

Hi Lars, sorry about the wrong order, I will correct it.

But about the ZTE device ID, I belive that we should insert among
Fibocom drivers because we are talking about a Fibocom module that is
using an ZTE Chipset.
So, this is exactly the same situation from Fibocom L610 IDs (0x1782,
0x4d10 / 0x1782, 0x4d11) that is using the Unisoc Chipset but were
inserted among Fibocom drivers.

And about the usb-devices output, let me explain better:
I am a Field Application Enginner at Fibocom Brazil and I am using the
IDs from our internal and official documentation.
On this documents its suggested to add all this IDs because it will
guarantee that can be used on all the variants devices from L71x series
(that can change according to different part number, region support or
network protocol).
Unfortunately, I don't have all the modules variations available with
me right now to test and share all the outputs.

Can we continue with all the IDs that I have inserted?
Or do you prefer to keep just the devices that I tested by myself until
now?

Victor Fragoso

  reply	other threads:[~2023-10-27 17:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-26  1:24 Victor Fragoso
2023-10-26 13:13 ` Lars Melin
2023-10-27 17:55   ` Victor Fragoso [this message]
2023-10-27 19:59     ` Lars Melin
2023-11-07 22:02       ` Victor Fragoso
2023-11-08 10:15         ` Johan Hovold
2023-10-26 13:49 ` Dan Williams
2023-10-27 17:55   ` Victor Fragoso
2023-11-08 10:23     ` Johan Hovold

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=94477736e69cc76eaaef8584d7e1aa5078a0611e.camel@hotmail.com \
    --to=victorffs@hotmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=johan@kernel.org \
    --cc=larsm17@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®