Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2725 bytes --]

On Mon, Aug 12, 2013 at 02:52:33PM -0400, Alan Stern wrote:
> On Mon, 12 Aug 2013, Felipe Balbi wrote:
> 
> > > > maybe a single callback for supporting 'testmodes' ? which receives the
> > > > test mode as argument ?
> > > 
> > > I don't have a clear picture of how you would apply such an approach to 
> > > this case.  There would have to be a way to tell the HCD to insert a
> > > 15-second delay between the Setup and Data stages of a particular
> > > control URB.  How would you do that?  Whatever method you choose,
> > 
> > each test is started after enumerating a known Vid/Pid pair. Based on
> > that, you *know* which test to run.
> 
> That's not what I meant.  Yes, the test-device driver knows what test
> it wants to run, based on the VID/PID.  I was asking how it would
> communicate this knowledge to the HCD.
> 
> For example, it doesn't make sense to have a callback that means 
> "Insert a 15-second delay into the next URB that I submit", because the 
> HCD doesn't know where URBs come from.

static int ehci_test_mode(struct usb_hcd *hcd, unsigned int test)
{
	struct ehci_hcd *ehci = to_ehci(hcd);

	....

	switch (test) {
	case USB_TEST_SINGLE_STEP_GET_DESC:
		start_test();
		wait_15_seconds();
		finish_test();
		break;
	...

	default:
		return -ENOTSUP;
	}

	return ret;
}

...

static struct hc_driver ehci_hcd_driver = {
	....

	.test_mode	= ehci_test_mode,

	...
};

> > > What other test modes would you want to support?
> > 
> > anything that USB[23]0CV supports today. There are even link layer tests
> > for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one
> > of a large(-ish) test suite which needs to be supported.
> 
> That's what I'm trying to find out.  What are the special features that 
> we would need to implement in order to support the entire test suite?

I haven't looked at USB??CV spec for a while, maybe Jack knows better ?

> > > Is it worth adding this support to the standard host controller
> > > drivers, or should there be a special version (a Kconfig option like
> > > CONFIG_RCU_TORTURE_TEST) to enable it?  Putting a lot of testing code
> > > in distribution kernels where it will never be used seems like a big
> > > waste.
> > 
> > right, I think it should be hidden by Kconfig, not arguing against that.
> 
> Therefore we both agree the $SUBJECT patch should not be accepted in
> its current form.  At the very least, the new code in ehci-hcd should
> be conditional on a CONFIG_USB_HCD_TEST_MODE option.  In addition, we
> may want some of the work (though at this point I'm not still clear on
> exactly what parts) to be moved into usbcore.

right

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]