| Message ID | 20220810014521.9383-1-slark_xiao@163.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ae7107baa5bde3cea62778fd4515865fe50181f4 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: usb: qmi_wwan: Add support for Cinterion MV32 | expand |
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
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.
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.
"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
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.
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>
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!
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 */
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(+)