[PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[Slark Xiao]

There are 2 models for MV32 serials. MV32-W-A is designed
based on Qualcomm SDX62 chip, and MV32-W-B is designed based
on Qualcomm SDX65 chip. So we use 2 different PID to separate it.

Test evidence as below:
T:  Bus=03 Lev=01 Prnt=01 Port=02 Cnt=03 Dev#=  3 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=1e2d ProdID=00f3 Rev=05.04
S:  Manufacturer=Cinterion
S:  Product=Cinterion PID 0x00F3 USB Mobile Broadband
S:  SerialNumber=d7b4be8d
C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option

T:  Bus=03 Lev=01 Prnt=01 Port=02 Cnt=03 Dev#= 10 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=1e2d ProdID=00f4 Rev=05.04
S:  Manufacturer=Cinterion
S:  Product=Cinterion PID 0x00F4 USB Mobile Broadband
S:  SerialNumber=d095087d
C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option

Signed-off-by: Slark Xiao <slark_xiao@163.com>
---
 drivers/net/usb/qmi_wwan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 571a399c195d..709e3c59e340 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1390,6 +1390,8 @@ static const struct usb_device_id products[] = {
 	{QMI_QUIRK_SET_DTR(0x1e2d, 0x00b0, 4)},	/* Cinterion CLS8 */
 	{QMI_FIXED_INTF(0x1e2d, 0x00b7, 0)},	/* Cinterion MV31 RmNet */
 	{QMI_FIXED_INTF(0x1e2d, 0x00b9, 0)},	/* Cinterion MV31 RmNet based on new baseline */
+	{QMI_FIXED_INTF(0x1e2d, 0x00f3, 0)},	/* Cinterion MV32-W-A RmNet */
+	{QMI_FIXED_INTF(0x1e2d, 0x00f4, 0)},	/* Cinterion MV32-W-B RmNet */
 	{QMI_FIXED_INTF(0x413c, 0x81a2, 8)},	/* Dell Wireless 5806 Gobi(TM) 4G LTE Mobile Broadband Card */
 	{QMI_FIXED_INTF(0x413c, 0x81a3, 8)},	/* Dell Wireless 5570 HSPA+ (42Mbps) Mobile Broadband Card */
 	{QMI_FIXED_INTF(0x413c, 0x81a4, 8)},	/* Dell Wireless 5570e HSPA+ (42Mbps) Mobile Broadband Card */
-- 
2.25.1

Re: [PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[Bjørn Mork]

Slark Xiao <slark_xiao@163.com> writes:

> There are 2 models for MV32 serials. MV32-W-A is designed
> based on Qualcomm SDX62 chip, and MV32-W-B is designed based
> on Qualcomm SDX65 chip. So we use 2 different PID to separate it.
>
> Test evidence as below:
> T:  Bus=03 Lev=01 Prnt=01 Port=02 Cnt=03 Dev#=  3 Spd=480 MxCh= 0
> D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=1e2d ProdID=00f3 Rev=05.04
> S:  Manufacturer=Cinterion
> S:  Product=Cinterion PID 0x00F3 USB Mobile Broadband
> S:  SerialNumber=d7b4be8d
> C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
> I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
> I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option

The patch looks nice, but I have a couple of questions since you're one
of the first pushing one of these SDX6x modems.

Is that protocol pattern fixed on this generation of Qualcomm chips?  It
looks like an extension of what they started with the SDX55 generation,
where the DIAG port was identified by ff/ff/30 across multiple vendors.

Specifically wrt this driver and patch, I wonder if we can/should match
on ff/ff/50 instead of interface number here?  I note that the interface
numbers are allocated sequentionally. Probably in the order these
function are enabled by the firmware? If so, are we sure this is static?
Or could we risk config variants where the RMNET/QMI function have a
different interface number for the same PIDs?

And another possibility you might consider.  Assuming that ff/ff/50
uniquely identifies RMNET/QMI functions regardless of PID, would you
consider a VID+class match to catch all of them?  This would not only
support both the PIDs of this patch in one go, but also any future PIDs
without the need for further driver patches.


Bjørn

Re:Re: [PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[Slark Xiao]

At 2022-08-10 14:55:42, "Bjørn Mork" <bjorn@mork.no> wrote:
>Slark Xiao <slark_xiao@163.com> writes:
>
>> There are 2 models for MV32 serials. MV32-W-A is designed
>> based on Qualcomm SDX62 chip, and MV32-W-B is designed based
>> on Qualcomm SDX65 chip. So we use 2 different PID to separate it.
>>
>> Test evidence as below:
>> T:  Bus=03 Lev=01 Prnt=01 Port=02 Cnt=03 Dev#=  3 Spd=480 MxCh= 0
>> D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
>> P:  Vendor=1e2d ProdID=00f3 Rev=05.04
>> S:  Manufacturer=Cinterion
>> S:  Product=Cinterion PID 0x00F3 USB Mobile Broadband
>> S:  SerialNumber=d7b4be8d
>> C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
>> I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
>> I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
>> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
>> I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
>
>The patch looks nice, but I have a couple of questions since you're one
>of the first pushing one of these SDX6x modems.
>
>Is that protocol pattern fixed on this generation of Qualcomm chips?  It
>looks like an extension of what they started with the SDX55 generation,
>where the DIAG port was identified by ff/ff/30 across multiple vendors.
>
 Seems yes. I checked some different usb_compositions and found that
 diag port is using protocol '30' always.

>Specifically wrt this driver and patch, I wonder if we can/should match
>on ff/ff/50 instead of interface number here?  I note that the interface

I checked all our edited usb_compositions and all QC default usb 
compositions(9025, 90db, 9067,90d5,9084,9091,90ad,90b8,90e5), 
ff/ff/50 is rmnet used only. 

>numbers are allocated sequentionally. Probably in the order these
>function are enabled by the firmware? If so, are we sure this is static?

This needs more time to confirm. I will keep you updated.

>Or could we risk config variants where the RMNET/QMI function have a
>different interface number for the same PIDs?
>
>And another possibility you might consider.  Assuming that ff/ff/50
>uniquely identifies RMNET/QMI functions regardless of PID, would you
>consider a VID+class match to catch all of them?  This would not only
>support both the PIDs of this patch in one go, but also any future PIDs
>without the need for further driver patches.
>
>
>Bjørn

I have a concern, if Cinterion or other Vendors, like Quectel, use other 
chip (such as intel, mediateck and so on), this methods may won't work,
because  they share a same VID. Also this may be changed once Qualcomm 
update the protocol patterns for future chip.

Re:Re:Re: [PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[Slark Xiao]

At 2022-08-10 17:28:51, "Slark Xiao" <slark_xiao@163.com> wrote:
>At 2022-08-10 14:55:42, "Bjørn Mork" <bjorn@mork.no> wrote:
>>Slark Xiao <slark_xiao@163.com> writes:
>>
>>> There are 2 models for MV32 serials. MV32-W-A is designed
>>> based on Qualcomm SDX62 chip, and MV32-W-B is designed based
>>> on Qualcomm SDX65 chip. So we use 2 different PID to separate it.
>>>
>>> Test evidence as below:
>>> T:  Bus=03 Lev=01 Prnt=01 Port=02 Cnt=03 Dev#=  3 Spd=480 MxCh= 0
>>> D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
>>> P:  Vendor=1e2d ProdID=00f3 Rev=05.04
>>> S:  Manufacturer=Cinterion
>>> S:  Product=Cinterion PID 0x00F3 USB Mobile Broadband
>>> S:  SerialNumber=d7b4be8d
>>> C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
>>> I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
>>> I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
>>> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
>>> I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
>>
>>The patch looks nice, but I have a couple of questions since you're one
>>of the first pushing one of these SDX6x modems.
>>
>>Is that protocol pattern fixed on this generation of Qualcomm chips?  It
>>looks like an extension of what they started with the SDX55 generation,
>>where the DIAG port was identified by ff/ff/30 across multiple vendors.
>>
> Seems yes. I checked some different usb_compositions and found that
> diag port is using protocol '30' always.
>
>>Specifically wrt this driver and patch, I wonder if we can/should match
>>on ff/ff/50 instead of interface number here?  I note that the interface
>
>I checked all our edited usb_compositions and all QC default usb 
>compositions(9025, 90db, 9067,90d5,9084,9091,90ad,90b8,90e5), 
>ff/ff/50 is rmnet used only. 
>
>>numbers are allocated sequentionally. Probably in the order these
>>function are enabled by the firmware? If so, are we sure this is static?
>
>This needs more time to confirm. I will keep you updated.
>
>>Or could we risk config variants where the RMNET/QMI function have a
>>different interface number for the same PIDs?
>>
>>And another possibility you might consider.  Assuming that ff/ff/50
>>uniquely identifies RMNET/QMI functions regardless of PID, would you
>>consider a VID+class match to catch all of them?  This would not only
>>support both the PIDs of this patch in one go, but also any future PIDs
>>without the need for further driver patches.
>>
>>
>>Bjørn
>
>I have a concern, if Cinterion or other Vendors, like Quectel, use other 
>chip (such as intel, mediateck and so on), this methods may won't work,

My bad. QMI_WWAN driver is designed for Qualcomm based chips only,
 right? 

>because  they share a same VID. Also this may be changed once Qualcomm 
>update the protocol patterns for future chip.

Re: [PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[Bjørn Mork]

"Slark Xiao" <slark_xiao@163.com> writes:
> At 2022-08-10 17:28:51, "Slark Xiao" <slark_xiao@163.com> wrote:
>
>>I have a concern, if Cinterion or other Vendors, like Quectel, use other 
>>chip (such as intel, mediateck and so on), this methods may won't work,
>
> My bad. QMI_WWAN driver is designed for Qualcomm based chips only,
>  right? 

Yes, but your concern is still valid if any of them re-use ff/ff/50 for
something which is not RMNET/QMI.  We do not want this driver to start
matching a non-Qualcomm based device.

>>because  they share a same VID. Also this may be changed once Qualcomm 
>>update the protocol patterns for future chip.

Yes, that' a risk since we have no knowledge of Qualcomm's plans or
thoughts around this. It's all pure guesswork from my side.  But as
such, it doesn't differ from the rest of this driver :-) Qualcomm can
change whatever they want and we'll just have to follow up with whatever
is required. Like what happened when raw-ip became mandatory.

I do find it unlikely that Qualcomm will ever change the meaning of this
pattern now that they've started using it.  That would not make any
sense. If they need to create a new vendor specific function type, then
they can just use one of the "free" protocol numbers (and also subclass
if they run out of protocol numbers).

But it's your call.  If you want to play it safe and keep the VID+PID
matching, then I'm fine with that too.


Bjørn

Re:Re: [PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[Slark Xiao]

At 2022-08-10 20:35:24, "Bjørn Mork" <bjorn@mork.no> wrote:
>"Slark Xiao" <slark_xiao@163.com> writes:
>> At 2022-08-10 17:28:51, "Slark Xiao" <slark_xiao@163.com> wrote:
>>
>>>I have a concern, if Cinterion or other Vendors, like Quectel, use other 
>>>chip (such as intel, mediateck and so on), this methods may won't work,
>>
>> My bad. QMI_WWAN driver is designed for Qualcomm based chips only,
>>  right? 
>
>Yes, but your concern is still valid if any of them re-use ff/ff/50 for
>something which is not RMNET/QMI.  We do not want this driver to start
>matching a non-Qualcomm based device.
>
>>>because  they share a same VID. Also this may be changed once Qualcomm 
>>>update the protocol patterns for future chip.
>
>Yes, that' a risk since we have no knowledge of Qualcomm's plans or
>thoughts around this. It's all pure guesswork from my side.  But as
>such, it doesn't differ from the rest of this driver :-) Qualcomm can
>change whatever they want and we'll just have to follow up with whatever
>is required. Like what happened when raw-ip became mandatory.
>
>I do find it unlikely that Qualcomm will ever change the meaning of this
>pattern now that they've started using it.  That would not make any
>sense. If they need to create a new vendor specific function type, then
>they can just use one of the "free" protocol numbers (and also subclass
>if they run out of protocol numbers).
>
>But it's your call.  If you want to play it safe and keep the VID+PID
>matching, then I'm fine with that too.
>
>
>Bjørn

Then please help me apply it directly. There is no more commit 
request for MV32 serials if they don't update base line.

Thanks.

Re: [PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[Bjørn Mork]

Slark Xiao <slark_xiao@163.com> writes:

> There are 2 models for MV32 serials. MV32-W-A is designed
> based on Qualcomm SDX62 chip, and MV32-W-B is designed based
> on Qualcomm SDX65 chip. So we use 2 different PID to separate it.
>
> Test evidence as below:
> T:  Bus=03 Lev=01 Prnt=01 Port=02 Cnt=03 Dev#=  3 Spd=480 MxCh= 0
> D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=1e2d ProdID=00f3 Rev=05.04
> S:  Manufacturer=Cinterion
> S:  Product=Cinterion PID 0x00F3 USB Mobile Broadband
> S:  SerialNumber=d7b4be8d
> C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
> I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
> I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
>
> T:  Bus=03 Lev=01 Prnt=01 Port=02 Cnt=03 Dev#= 10 Spd=480 MxCh= 0
> D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=1e2d ProdID=00f4 Rev=05.04
> S:  Manufacturer=Cinterion
> S:  Product=Cinterion PID 0x00F4 USB Mobile Broadband
> S:  SerialNumber=d095087d
> C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
> I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
> I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
>
> Signed-off-by: Slark Xiao <slark_xiao@163.com>

Thanks for answering all my questions so fast. This patch looks very
good to me

Acked-by: Bjørn Mork <bjorn@mork.no>

Re: [PATCH] net: usb: qmi_wwan: Add support for Cinterion MV32

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 10 Aug 2022 09:45:21 +0800 you wrote:
> There are 2 models for MV32 serials. MV32-W-A is designed
> based on Qualcomm SDX62 chip, and MV32-W-B is designed based
> on Qualcomm SDX65 chip. So we use 2 different PID to separate it.
> 
> Test evidence as below:
> T:  Bus=03 Lev=01 Prnt=01 Port=02 Cnt=03 Dev#=  3 Spd=480 MxCh= 0
> D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=1e2d ProdID=00f3 Rev=05.04
> S:  Manufacturer=Cinterion
> S:  Product=Cinterion PID 0x00F3 USB Mobile Broadband
> S:  SerialNumber=d7b4be8d
> C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
> I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
> I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
> 
> [...]

Here is the summary with links:
  - net: usb: qmi_wwan: Add support for Cinterion MV32
    https://git.kernel.org/netdev/net/c/ae7107baa5bd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html