On Fri, Apr 08, 2016 at 08:27:16AM +0000, Schemmel Hans-Christoph wrote: > > -----Original Message----- > > From: Johan Hovold [mailto:jhovold@xxxxxxxxx] On Behalf Of Johan Hovold > > Sent: Donnerstag, 7. April 2016 11:51 > > To: Schemmel Hans-Christoph > > Cc: johan@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH] option.c: Support for Gemalto's Cinterion PH8 and AHxx products added > > > > > +/* Gemalto's Cinterion products (formerly Siemens)*/ > > > > You forgot a space after the closing parentheses. Just leave the > > comment unchanged. > > > > > +#define CINTERION_PRODUCT_PH8_2RMNET 0x0082 > > > +#define CINTERION_PRODUCT_PH8_AUDIO 0x0083 > > > +#define CINTERION_PRODUCT_AHXX_2RMNET 0x0084 > > > +#define CINTERION_PRODUCT_AHXX_AUDIO 0x0085 > > > > Could you provide the usb-devices output for these devices? > > > > Thanks, > > Johan > > > > > - .driver_info = (kernel_ulong_t)&net_intf4_blacklist }, > > > - { USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_AHXX, 0xff) }, > > > + .driver_info = (kernel_ulong_t)&cinterion_rmnet1_blacklist }, > > > + { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_AHXX), > > > + .driver_info = (kernel_ulong_t)&cinterion_rmnet1_blacklist }, > > > > Why are you modifying existing entries here? > > Hello Johan, > > regarding your comments: > > > > +/* Gemalto's Cinterion products (formerly Siemens)*/ > > > > You forgot a space after the closing parentheses. Just leave the > > comment unchanged. > > The Cinterion Company is now part of Gemalto and I just would like to > reflect this with an updated comment. I didn't notice that you added the company name as well, that's fine. Just add the missing space. > > Could you provide the usb-devices output for these devices? > Please note that the new PH8 can be configured to enumerate the > network interfaces as CDC-ECM (using cdc_ether) or vendor specific > (using QMI_WWAN). Thanks for the detailed info. This confirmed my suspicion that we could do things in a simpler way. Details below. > The output is the following: > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > CINTERION_PRODUCT_PH8_2RMNET 0x0082 > => CDC-ECM enumeration > T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#= 15 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=0082 Rev=00.00 > S: Manufacturer=Cinterion > S: Product=PH8 > C: #Ifs= 8 Cfg#= 1 Atr=e0 MxPwr=10mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether > I: If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether > I: If#= 6 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether > I: If#= 7 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > CINTERION_PRODUCT_PH8_2RMNET 0x0082 > => Vendor enumeration > T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#= 22 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=0082 Rev=00.00 > S: Manufacturer=Cinterion > S: Product=PH8 > C: #Ifs= 6 Cfg#= 1 Atr=e0 MxPwr=10mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > I: If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ You should use USB_DEVICE_INTERFACE_CLASS and only blacklist interfaces 4 and 5 (instead of explicitly blacklisting interfaces 4 through 7). > CINTERION_PRODUCT_PH8_AUDIO 0x0083 > => CDC-ECM enumeration > T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#= 16 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=0083 Rev=00.00 > S: Manufacturer=Cinterion > S: Product=PH8 > C: #Ifs= 9 Cfg#= 1 Atr=e0 MxPwr=10mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether > I: If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether > I: If#= 6 Alt= 0 #EPs= 1 Cls=01(audio) Sub=01 Prot=00 Driver=snd-usb-audio > I: If#= 7 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio > I: If#= 8 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > CINTERION_PRODUCT_PH8_AUDIO 0x0083 > => Vendor enumeration > T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#= 23 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=0083 Rev=00.00 > S: Manufacturer=Cinterion > S: Product=PH8 > C: #Ifs= 8 Cfg#= 1 Atr=e0 MxPwr=10mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > I: If#= 5 Alt= 0 #EPs= 1 Cls=01(audio) Sub=01 Prot=00 Driver=snd-usb-audio > I: If#= 6 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio > I: If#= 7 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Similarly here you should also use USB_DEVICE_INTERFACE_CLASS and then you only need to blacklist interface 4 using the generic net_intf4_blacklist struct. > CINTERION_PRODUCT_AHXX_2RMNET 0x0084 > T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#= 18 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=0084 Rev=00.00 > S: Manufacturer=Cinterion > S: Product=AHx > C: #Ifs= 8 Cfg#= 1 Atr=e0 MxPwr=10mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether > I: If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether > I: If#= 6 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether > I: If#= 7 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Here you don't need to blacklist anything if you use USB_DEVICE_INTERFACE_CLASS. > CINTERION_PRODUCT_AHXX_AUDIO 0x0085 > T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#= 19 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=0085 Rev=00.00 > S: Manufacturer=Cinterion > S: Product=AHx > C: #Ifs= 9 Cfg#= 1 Atr=e0 MxPwr=10mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether > I: If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether > I: If#= 6 Alt= 0 #EPs= 1 Cls=01(audio) Sub=01 Prot=00 Driver=snd-usb-audio > I: If#= 7 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio > I: If#= 8 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio Ditto. > > Why are you modifying existing entries here? > > The latest version of PH8 > CINTERION_PRODUCT_PH8 0x0053 > can also enumerate the network interfaces as CDC-ECM or Vendor > specific (QMI_WWAN). With the CDC-ECM enumeration the interfaces 4 + > 5 must be blacklisted now. > > The usb-devices output: > > CINTERION_PRODUCT_PH8 0x0053 > => CDC-ECM enumeration > T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#= 26 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=0053 Rev=00.00 > S: Manufacturer=Cinterion > S: Product=PH8 > C: #Ifs= 6 Cfg#= 1 Atr=e0 MxPwr=10mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether > I: If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > CINTERION_PRODUCT_PH8 0x0053 > => Vendor enumeration > T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=01 Dev#= 24 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=0053 Rev=00.00 > S: Manufacturer=Cinterion > S: Product=PH8 > C: #Ifs= 5 Cfg#= 1 Atr=e0 MxPwr=10mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan And here you really only need to blacklist interface 4. > The entry for CINTERION_PRODUCT_AHXX was just modified to have a more > uniform format. Could you provide the usb-devices output for this one as well? As AHXX was already using USB_DEVICE_INTERFACE_CLASS I suspect you should not modify this one at all. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html