Hi Alan,
On Fri, May 23, 2014 at 01:54:01AM +0800, Alan Stern wrote:
> On Thu, 22 May 2014, Pratyush Anand wrote:
>
> > OTG3 and EH Compliance Plan 1.0 talks about Super Speed OTG Verification
> > system (SS-OVS) which consists of an excersizer and analyzer.
> >
> > USB Compliance Suite from Lecroy or Ellisys can act as such SS-OVS for
> > Link Layer Validation (LVS).
> >
> > Some modifications are needed for an embedded Linux USB host to pass all
> > these tests. Most of these tests require just Link to be in U0. They do
> > not work with default Linux USB stack since, default stack does port
> > reset and then starts sending setup packet, which is not expected by
> > Link Layer Validation (LVS) device of Lecroy Compliance Suit. Then,
> > There are many Link Layer Tests which need host to generate specific
> > traffic.
> >
> > This patch supports specific traffic generation cases. As of now all the
> > host Lecroy Link Layer-USBIF tests (except TD7.26) passes
> > with this patch for single run using Lecroy USB Compliance Suite
> > Version 1.98 Build 239 and Lecroy USB Protocol Analyzer version 4.80
> > Build 1603. Therefore patch seems to be a good candidate for inclusion.
> > Further modification can be done on top of it.
> >
> > lvstest driver will not bind to any device by default. It can bind
> > manually to a super speed USB host controller root hub. Therefore, regular
> > hub driver must be unbound before this driver is bound. For example, if
> > 2-0:1.0 is the xhci root hub, then execute following to unbind hub driver.
> >
> > echo 2-0:1.0 > /sys/bus/usb/drivers/hub/unbind
> >
> > Then to bind lvs driver write Linux Foundation's vendor ID and SS root
> > hub's device ID into new_id file.
> >
> > echo "1D6B 3" > /sys/bus/usb/drivers/lvs/new_id
> >
> > Now connect LVS device with root hub port.
>
> You forgot to include an example here showing how to do this:
>
> echo 2-0\:1.0 > /sys/bus/usb/drivers/lvs/bind
>
We do not need to write into bind file. Writing a valid VID and PID
of SS root hub into new_id file does the binding also.
> > Test case specific traffic can be generated as follows whenever needed:
> >
> > 1. To issue "Get Device descriptor" command for TD.7.06:
> > echo 1 > /sys/bus/usb/devices/2-0\:1.0/get_dev_desc
> >
> > 2. To set U1 timeout to 127 for TD.7.18
> > echo 127 > /sys/bus/usb/devices/2-0\:1.0/u1_timeout
> >
> > 3. To set U2 timeout to 0 for TD.7.18
> > echo 0 > /sys/bus/usb/devices/2-0\:1.0/u2_timeout
> >
> > 4. To issue "Hot Reset" for TD.7.29
> > echo 1 > /sys/bus/usb/devices/2-0\:1.0/hot_reset
> >
> > 5. To issue "U3 Entry" for TD.7.35
> > echo 1 > /sys/bus/usb/devices/2-0\:1.0/u3_entry
> >
> > 6. To issue "U3 Exit" for TD.7.36
> > echo 1 > /sys/bus/usb/devices/2-0\:1.0/u3_exit
>
> None of those example need to contain the "1". For instance, the last
> one should be written:
>
> echo > /sys/bus/usb/devices/2-0\:1.0/u3_exit
Yes, any write will do except for ux_timeout.
>
> > diff --git a/Documentation/ABI/testing/sysfs-bus-usb-lvstest b/Documentation/ABI/testing/sysfs-bus-usb-lvstest
> > new file mode 100644
> > index 0000000..f47e8a3
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-usb-lvstest
> > @@ -0,0 +1,47 @@
> > +Link Layer Validation Device is a standard device for testing of Super
> > +Speed Link Layer tests. These nodes are available in sysfs only when lvs
> > +driver is bound with root hub device.
> > +
> > +What: /sys/bus/usb/devices/.../get_dev_desc
> > +Date: March 2014
> > +Contact: Pratyush Anand <pratyush.anand@xxxxxx>
> > +Description:
> > + Write to this node to issue "Get Device Descriptor"
> > + for Link Layer Validation device. It is needed for TD.7.06.
> > +
> > +What: /sys/bus/usb/devices/.../u1_timeout
> > +Date: March 2014
> > +Contact: Pratyush Anand <pratyush.anand@xxxxxx>
> > +Description:
> > + Set "U1 timeout" for the downstream port where Link Layer
> > + Validation device is connected. It is needed for TD.7.18,
> > + TD.7.19, TD.7.20 and TD.7.21.
>
> This doesn't say that the value must be between 0 and 127.
OK, that I will specify.
>
> > +
> > +What: /sys/bus/usb/devices/.../u2_timeout
> > +Date: March 2014
> > +Contact: Pratyush Anand <pratyush.anand@xxxxxx>
> > +Description:
> > + Set "U2 timeout" for the downstream port where Link Layer
> > + Validation device is connected. It is needed for TD.7.18,
> > + TD.7.19, TD.7.20 and TD.7.21.
>
> Same here.
OK.
>
> > +static void lvs_rh_work(struct work_struct *work)
> > +{
> > + struct lvs_rh *lvs = container_of(work, struct lvs_rh, rh_work);
> > + struct usb_interface *intf = lvs->intf;
> > + struct usb_device *hdev = interface_to_usbdev(intf);
> > + struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
> > + struct usb_hub_descriptor *descriptor = &lvs->descriptor;
> > + struct usb_port_status port_status;
> > + int i, ret = 0;
> > +
> > + /* Examine each root port */
> > + for (i = 1; i <= descriptor->bNbrPorts; i++) {
> > + ret = usb_control_msg(hdev, usb_rcvctrlpipe(hdev, 0),
> > + USB_REQ_GET_STATUS, USB_DIR_IN | USB_RT_PORT, 0, i,
> > + &port_status, sizeof(port_status), 1000);
>
> You must not use I/O buffers allocated on the stack. The buffer must
> be allocated by kmalloc or something equivalent.
OK.
>
> > + if (ret < 4)
> > + continue;
> > + /* handle only connection change notification */
> > + if (!(port_status.wPortChange & USB_PORT_STAT_C_CONNECTION))
> > + continue;
> > + usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
> > + USB_REQ_CLEAR_FEATURE, USB_RT_PORT,
> > + USB_PORT_FEAT_C_CONNECTION, i, NULL, 0, 1000);
> > +
> > + if (port_status.wPortStatus & USB_PORT_STAT_CONNECTION) {
> > + lvs->present = true;
> > + lvs->portnum = i;
> > + if (hcd->phy)
> > + usb_phy_notify_connect(hcd->phy,
> > + USB_SPEED_SUPER);
> > + } else {
> > + lvs->present = false;
> > + if (hcd->phy)
> > + usb_phy_notify_disconnect(hcd->phy,
> > + USB_SPEED_SUPER);
> > + }
> > + break;
> > + }
> > +
> > + ret = usb_submit_urb(lvs->urb, GFP_ATOMIC);
>
> This doesn't need to be GFP_ATOMIC; you can use GFP_KERNEL.
OK.
>
> > +static int lvs_rh_probe(struct usb_interface *intf,
> > + const struct usb_device_id *id)
> > +{
>
> ...
>
> > + ret = usb_submit_urb(lvs->urb, GFP_NOIO);
>
> And this doesn't need to be GFP_NOIO.
OK.
Regards
Pratyush
>
> Alan Stern
--
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
