| Message ID | 20190923105102.37413-1-ran.wang_1@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: hub add filter for device with specific VID&PID | expand |
On Mon, Sep 23, 2019 at 06:51:02PM +0800, Ran Wang wrote: > USB 2.0 Embedded Host PET Automated Test (CH6) 6.7.23 A-UUT "Unsupported > Device" Message require to stop enumerating device with VID=0x1a0a PID=0x0201 > and pop message to declare this device is not supported. Why is this a requirement? And why those specific vid/pid values? What do they refer to? > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > --- > drivers/usb/core/hub.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index bbcfa63..3cda0da 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -4982,6 +4982,18 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, > if (status < 0) > goto loop; > > + /* USB 2.0 Embedded Host PET Automated Test (CH6) > + * 6.7.23 A-UUT "Unsupported Device" Message > + * require to filter out below device when enumeration > + */ Nit, can you align your comment lines, to match the other multi-line comments in this file? Otherwise it starts to look bad over time. > + if ((udev->descriptor.idVendor == 0x1a0a) > + && (udev->descriptor.idProduct == 0x0201)) { Are you sure you don't have to convert this value into cpu endian before checking it? thanks, greg k-h
On Mon, 23 Sep 2019, Ran Wang wrote: > USB 2.0 Embedded Host PET Automated Test (CH6) 6.7.23 A-UUT "Unsupported > Device" Message require to stop enumerating device with VID=0x1a0a PID=0x0201 > and pop message to declare this device is not supported. > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > --- > drivers/usb/core/hub.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index bbcfa63..3cda0da 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -4982,6 +4982,18 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, > if (status < 0) > goto loop; > > + /* USB 2.0 Embedded Host PET Automated Test (CH6) > + * 6.7.23 A-UUT "Unsupported Device" Message > + * require to filter out below device when enumeration > + */ > + if ((udev->descriptor.idVendor == 0x1a0a) > + && (udev->descriptor.idProduct == 0x0201)) { > + dev_err(&udev->dev, "This device is not supported: idVendor=0x%x idProduct=0x%x\n", > + udev->descriptor.idVendor, > + udev->descriptor.idProduct); There's no need to write out the Vendor and Product IDs. They already appear in the "New device" message. > + goto done; > + } > + > if (udev->quirks & USB_QUIRK_DELAY_INIT) > msleep(2000); Shouldn't this be implemented as a device quirk? Alan Stern
Hi Greg, On Monday, September 23, 2019 19:07, Greg Kroah-Hartman wrote: > > On Mon, Sep 23, 2019 at 06:51:02PM +0800, Ran Wang wrote: > > USB 2.0 Embedded Host PET Automated Test (CH6) 6.7.23 A-UUT > > "Unsupported Device" Message require to stop enumerating device with > > VID=0x1a0a PID=0x0201 and pop message to declare this device is not > supported. > > Why is this a requirement? This comes from <USB On-The-Go and Embedded Host Automated Compliance Plan for the On-The-Go& Embedded Host Supplement Revision2.0> Below is related description I quote from it: 6.7.23 A-UUT "Unsupported Device" Message Purpose: This test verifies that an A-UUT produces a device non-supported error message when a device it doesn't recognize, and does not support HNP, connects to it. Applies to: All Targeted Hosts Description: Get VBUS turned on, and connect to the A-UUT. Get enumerated and respond as an unknown device not supporting HNP. Check that a suitable error message is generated. Pass Criteria: Message "Unsupported Device"or similar is displayed on UUT 6.7.23.1 Test Procedure 1. Start with cable still attached, PET applying 10μF capacitance and 10kΩ pull-down resistance between VBUS and ground, data lines not pulled up. 2. Get VBUS turned on, using the method described in Section6.7.1. 3. Wait for almost TB_SVLD_BCON max (1s - 0.1s = 0.9s) from VBUS reaching VOTG_SESS_VLD max. 4. Connect PET using D+ pull-up. 5. Allow A-UUT to enumerate PET, responding with a VID / PID combination not on the TPL of the UUT and also with the OTG descriptor stating that it does not support HNP. 6. Start 30s timer when Device Descriptor is read. 7. Display Message "Click OK if 'Unsupported Device' indication displayed on UUT". 8. If operator clicks OK before 30s timer expires, then UUT passes test. 9. If 30selapses first, then UUT fails test. 10. PET disconnects by removing any termination on the data lines, but leaves a capacitance of 10μF and a pull-down resistance of 10kΩ connected across VBUS. 11. Wait 2s to allow disconnection to be detected. End of Test. > And why those specific vid/pid values? What do they refer to? For step 5, we got the VID / PID number from USB IF certified lab(Allion.inc at Taiwang). Looks like this is a reserved ID pair and will not be allocated to any vendor for their products. So it's hence used for this case test (like saying: you should be able to pop a not-support message for this reserved VID&PID). > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > --- > > drivers/usb/core/hub.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index > > bbcfa63..3cda0da 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -4982,6 +4982,18 @@ static void hub_port_connect(struct usb_hub *hub, > int port1, u16 portstatus, > > if (status < 0) > > goto loop; > > > > + /* USB 2.0 Embedded Host PET Automated Test (CH6) > > + * 6.7.23 A-UUT "Unsupported Device" Message > > + * require to filter out below device when enumeration > > + */ > > Nit, can you align your comment lines, to match the other multi-line comments > in this file? Otherwise it starts to look bad over time. Yes, will update. > > > > + if ((udev->descriptor.idVendor == 0x1a0a) > > + && (udev->descriptor.idProduct == 0x0201)) { > > Are you sure you don't have to convert this value into cpu endian before > checking it? Thanks for pointing out, how about this: if ((le16_to_cpu(udev->descriptor.idVendor) == 0x1a0a) && (le16_to_cpu(udev->descriptor.idProduct) == 0x0201)) { Regards, Ran
Hi Alan, On Monday, September 23, 2019 23:01, Alan Stern wrote: > > On Mon, 23 Sep 2019, Ran Wang wrote: > > > USB 2.0 Embedded Host PET Automated Test (CH6) 6.7.23 A-UUT > > "Unsupported Device" Message require to stop enumerating device with > > VID=0x1a0a PID=0x0201 and pop message to declare this device is not > supported. > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > --- > > drivers/usb/core/hub.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index > > bbcfa63..3cda0da 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -4982,6 +4982,18 @@ static void hub_port_connect(struct usb_hub *hub, > int port1, u16 portstatus, > > if (status < 0) > > goto loop; > > > > + /* USB 2.0 Embedded Host PET Automated Test (CH6) > > + * 6.7.23 A-UUT "Unsupported Device" Message > > + * require to filter out below device when enumeration > > + */ > > + if ((udev->descriptor.idVendor == 0x1a0a) > > + && (udev->descriptor.idProduct == 0x0201)) { > > + dev_err(&udev->dev, "This device is not supported: > idVendor=0x%x idProduct=0x%x\n", > > + udev->descriptor.idVendor, > > + udev->descriptor.idProduct); > > There's no need to write out the Vendor and Product IDs. They already appear > in the "New device" message. OK > > + goto done; > > + } > > + > > if (udev->quirks & USB_QUIRK_DELAY_INIT) > > msleep(2000); > > Shouldn't this be implemented as a device quirk? Yes, I was also looking for the way in quirk but not quite sure currently. So we can stop initializing a device (with specific VID&PID) in quirk, right? Actually in drivers/usb/core/hub.c function usb_enumerate_device(), it will call is_targeted(udev) which has below implementation: 54 /* OTG PET device is always targeted (see OTG 2.0 ECN 6.4.2) */ 55 if ((le16_to_cpu(dev->descriptor.idVendor) == 0x1a0a && 56 le16_to_cpu(dev->descriptor.idProduct) == 0x0200)) 57 return 1; The ID is very close to what I need to response. So, do I need to add code here? Thanks. Ran
Hi Alan, On Saturday, September 28, 2019 01:08, Alan Stern wrote: > > On Tue, 24 Sep 2019, Ran Wang wrote: > > > Hi Greg, > > > > On Monday, September 23, 2019 19:07, Greg Kroah-Hartman wrote: > > > > > > On Mon, Sep 23, 2019 at 06:51:02PM +0800, Ran Wang wrote: > > > > USB 2.0 Embedded Host PET Automated Test (CH6) 6.7.23 A-UUT > > > > "Unsupported Device" Message require to stop enumerating device > > > > with VID=0x1a0a PID=0x0201 and pop message to declare this device > > > > is not > > > supported. > > > > > > Why is this a requirement? > > > > This comes from <USB On-The-Go and Embedded Host Automated Compliance > > Plan for the On-The-Go& Embedded Host Supplement Revision2.0> > > How much do we care about our support for USB OTG? Isn't it dying off? As I know, some vendors still want to pass this Embedded Host compliance test to make sure their product's quality. And above test case is included. > > Below is related description I quote from it: > > 6.7.23 A-UUT "Unsupported Device" Message > > Purpose: This test verifies that an A-UUT produces a device non-supported > error message > > when a device it doesn't recognize, and does not support HNP, connects > to it. > > Applies to: All Targeted Hosts > > Description: Get VBUS turned on, and connect to the A-UUT. Get enumerated > and respond > > as an unknown device not supporting HNP. Check that a suitable error > message is generated. > > Pass Criteria: Message "Unsupported Device"or similar is displayed on > > UUT > > > > 6.7.23.1 Test Procedure > > 1. Start with cable still attached, PET applying 10 F capacitance and 10k pull- > down > > resistance between VBUS and ground, data lines not pulled up. > > 2. Get VBUS turned on, using the method described in Section6.7.1. > > 3. Wait for almost TB_SVLD_BCON max (1s - 0.1s = 0.9s) from VBUS reaching > VOTG_SESS_VLD max. > > 4. Connect PET using D+ pull-up. > > 5. Allow A-UUT to enumerate PET, responding with a VID / PID combination > not on the TPL > > of the UUT and also with the OTG descriptor stating that it does not support > HNP. > > 6. Start 30s timer when Device Descriptor is read. > > 7. Display Message "Click OK if 'Unsupported Device' indication displayed on > UUT". > > 8. If operator clicks OK before 30s timer expires, then UUT passes test. > > 9. If 30selapses first, then UUT fails test. > > 10. PET disconnects by removing any termination on the data lines, but leaves > a capacitance of > > 10 F and a pull-down resistance of 10k connected across VBUS. > > 11. Wait 2s to allow disconnection to be detected. > > End of Test. > > In fact, the system should respond the same way to any unrecognized device > that doesn't support HNP, right? There's nothing special about these VID/PID > values. Yes, but I saw there are already some implementation in upstream kernel for this purpose, just PID different: drivers/usb/core/otg_whitelist.h function is_targeted() > > > And why those specific vid/pid values? What do they refer to? > > > > For step 5, we got the VID / PID number from USB IF certified > > lab(Allion.inc at Taiwang). Looks like this is a reserved ID pair and > > will not be allocated to any vendor for their products. So it's hence used for > this case test (like saying: you should be able to pop a not-support message for > this reserved VID&PID). > > Don't we do this already? Yes, but in function is_stargeted(), I found it's a little be different: Current upstream: VID = 0x1a0a, PID = 0x0200 Info from USB-IF certified lab: VID = 0x1a0a, PID = 0x0201 Regards, Ran > Alan Stern
Hi Alan, On Sunday, September 29, 2019 10:35, wrote: > > Hi Alan, > > On Saturday, September 28, 2019 01:08, Alan Stern wrote: <snip> > > > > In fact, the system should respond the same way to any unrecognized > > device that doesn't support HNP, right? There's nothing special about > > these VID/PID values. > > Yes, but I saw there are already some implementation in upstream kernel for this > purpose, just PID different: drivers/usb/core/otg_whitelist.h function > is_targeted() > > > > > And why those specific vid/pid values? What do they refer to? > > > > > > For step 5, we got the VID / PID number from USB IF certified > > > lab(Allion.inc at Taiwang). Looks like this is a reserved ID pair > > > and will not be allocated to any vendor for their products. So it's > > > hence used for > > this case test (like saying: you should be able to pop a not-support > > message for this reserved VID&PID). > > > > Don't we do this already? > > Yes, but in function is_stargeted(), I found it's a little be different: > Current upstream: VID = 0x1a0a, PID = 0x0200 > Info from USB-IF certified lab: VID = 0x1a0a, PID = 0x0201 > Sorry that I mis-understood the logic of is_stargeted() and it's caller. So the proper way to resolve my problem is: 1. Select CONFIG_USB_OTG in .config 2. Add property 'tpl-support' to device tree 3. Customize whitelist_table[] according to my Target-Peripheral-List requirement. Am I right? :) Regards, Ran
> > > > > > In fact, the system should respond the same way to any unrecognized > > > device that doesn't support HNP, right? There's nothing special about > > > these VID/PID values. > > > > Yes, but I saw there are already some implementation in upstream kernel for this > > purpose, just PID different: drivers/usb/core/otg_whitelist.h function > > is_targeted() > > > > > > > And why those specific vid/pid values? What do they refer to? > > > > > > > > For step 5, we got the VID / PID number from USB IF certified > > > > lab(Allion.inc at Taiwang). Looks like this is a reserved ID pair > > > > and will not be allocated to any vendor for their products. So it's > > > > hence used for > > > this case test (like saying: you should be able to pop a not-support > > > message for this reserved VID&PID). > > > > > > Don't we do this already? > > > > Yes, but in function is_stargeted(), I found it's a little be different: > > Current upstream: VID = 0x1a0a, PID = 0x0200 > > Info from USB-IF certified lab: VID = 0x1a0a, PID = 0x0201 > > > > Sorry that I mis-understood the logic of is_stargeted() and it's caller. > > So the proper way to resolve my problem is: > 1. Select CONFIG_USB_OTG in .config > 2. Add property 'tpl-support' to device tree > 3. Customize whitelist_table[] according to my Target-Peripheral-List > requirement. > Yan, this is the correct way, we (NXP i.MX Series) have followed this way for USB EH certification several years. Peter > Am I right? :) > > Regards, > Ran
On Sat, Sep 28, 2019 at 1:11 AM Alan Stern <stern@rowland.harvard.edu> wrote: > > On Tue, 24 Sep 2019, Ran Wang wrote: > > > Hi Greg, > > > > On Monday, September 23, 2019 19:07, Greg Kroah-Hartman wrote: > > > > > > On Mon, Sep 23, 2019 at 06:51:02PM +0800, Ran Wang wrote: > > > > USB 2.0 Embedded Host PET Automated Test (CH6) 6.7.23 A-UUT > > > > "Unsupported Device" Message require to stop enumerating device with > > > > VID=0x1a0a PID=0x0201 and pop message to declare this device is not > > > supported. > > > > > > Why is this a requirement? > > > > This comes from <USB On-The-Go and Embedded Host Automated Compliance Plan > > for the On-The-Go& Embedded Host Supplement Revision2.0> > > How much do we care about our support for USB OTG? Isn't it dying off? > Hi Alan, The OTG Compliance is really dead, but for non-PC host product, if we would like to pass USB certification, it needs to pass EH compliance test. Before going to Lab, the vendor will fill Compliance Checklist, it could declare HNP, SRP and ADP are not supported, then the PET test will bypass related test items. Peter > > Below is related description I quote from it: > > 6.7.23 A-UUT "Unsupported Device" Message > > Purpose: This test verifies that an A-UUT produces a device non-supported error message > > when a device it doesn't recognize, and does not support HNP, connects to it. > > Applies to: All Targeted Hosts > > Description: Get VBUS turned on, and connect to the A-UUT. Get enumerated and respond > > as an unknown device not supporting HNP. Check that a suitable error message is generated. > > Pass Criteria: Message "Unsupported Device"or similar is displayed on UUT > > > > 6.7.23.1 Test Procedure > > 1. Start with cable still attached, PET applying 10ìF capacitance and 10kÙ pull-down > > resistance between VBUS and ground, data lines not pulled up. > > 2. Get VBUS turned on, using the method described in Section6.7.1. > > 3. Wait for almost TB_SVLD_BCON max (1s - 0.1s = 0.9s) from VBUS reaching VOTG_SESS_VLD max. > > 4. Connect PET using D+ pull-up. > > 5. Allow A-UUT to enumerate PET, responding with a VID / PID combination not on the TPL > > of the UUT and also with the OTG descriptor stating that it does not support HNP. > > 6. Start 30s timer when Device Descriptor is read. > > 7. Display Message "Click OK if 'Unsupported Device' indication displayed on UUT". > > 8. If operator clicks OK before 30s timer expires, then UUT passes test. > > 9. If 30selapses first, then UUT fails test. > > 10. PET disconnects by removing any termination on the data lines, but leaves a capacitance of > > 10ìF and a pull-down resistance of 10kÙ connected across VBUS. > > 11. Wait 2s to allow disconnection to be detected. > > End of Test. > > In fact, the system should respond the same way to any unrecognized > device that doesn't support HNP, right? There's nothing special about > these VID/PID values. > > > > And why those specific vid/pid values? What do they refer to? > > > > For step 5, we got the VID / PID number from USB IF certified lab(Allion.inc at Taiwang). Looks like > > this is a reserved ID pair and will not be allocated to any vendor for their products. So it's hence used for this > > case test (like saying: you should be able to pop a not-support message for this reserved VID&PID). > > Don't we do this already? > > Alan Stern >
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index bbcfa63..3cda0da 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4982,6 +4982,18 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, if (status < 0) goto loop; + /* USB 2.0 Embedded Host PET Automated Test (CH6) + * 6.7.23 A-UUT "Unsupported Device" Message + * require to filter out below device when enumeration + */ + if ((udev->descriptor.idVendor == 0x1a0a) + && (udev->descriptor.idProduct == 0x0201)) { + dev_err(&udev->dev, "This device is not supported: idVendor=0x%x idProduct=0x%x\n", + udev->descriptor.idVendor, + udev->descriptor.idProduct); + goto done; + } + if (udev->quirks & USB_QUIRK_DELAY_INIT) msleep(2000);
USB 2.0 Embedded Host PET Automated Test (CH6) 6.7.23 A-UUT "Unsupported Device" Message require to stop enumerating device with VID=0x1a0a PID=0x0201 and pop message to declare this device is not supported. Signed-off-by: Ran Wang <ran.wang_1@nxp.com> --- drivers/usb/core/hub.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)