Re: [PATCH v12 7/8] usb: Adding SuperSpeed support to dummy_hcd

[Felipe Balbi]

Hi,

On Wed, May 25, 2011 at 01:03:00PM +0300, Tanya Brokhman wrote:
> > > No, I'm afraid we're talking about different things here.
> > > As far as dummy_hcd is concerned:
> > > Dummy_hcd has a new module parameter that should be set to true if
> > the
> > > developer wishes the gadget driver to be connected to a SS root hub.
> > > Since at the moment most of the gadget drivers woun't work at SS
> > > connection (lack of SS descriptors) the default of this parameter is
> > > true and thus the connected gadget driver will be enumerated over a
> > HS
> > > root hub. Users don't have to remember t set anything in order to be
> > > able to work with dummy_hcd and the existing gadget drivers just as
> > they did up until now.
> > >
> > > We did add a new feature flag (CONFIG_USB_GADGET_SUPERSPEED) that is
> > > set during compilation. If that flag is defined, then (and only
> > then!)
> > > will the
> > > driver->speed be set to SS by composite_bind. Please keep in mind the
> > 
> > and that's the only thing I'm asking you to remove :-)
> 
> You want to remove the following change from in composite.c:
> 
> @@ -1386,6 +1604,9 @@ int usb_composite_probe(struct usb_composite_driver
> *driver,
>  		driver->iProduct = driver->name;
>  	composite_driver.function =  (char *) driver->name;
>  	composite_driver.driver.name = driver->name;
> +#ifdef CONFIG_USB_GADGET_SUPERSPEED
> +	composite_driver.speed = USB_SPEED_SUPER; #endif /* 
> +CONFIG_USB_GADGET_SUPERSPEED */
>  	composite = driver;
>  	composite_gadget_bind = bind;
> 
> Right? I'm sorry, but I really don't understand why... :( Removing it is the
> same as not defining CONFIG_USB_GADGET_SUPERSPEED.

we still want to build a SuperSpeed dummy_hcd but have HS gadget drivers
:-)

> > we should be to compile dummy_hcd in SuperSpeed and still have high-
> > speed gadget drivers :-)
> 
> But this is the case right now. Or do you mean that you want to load (not
> compile) the dummy_hcd with is_super_speed=1 and still be able to enumerate
> HS gadgets?

that too.

> > > following:
> > > 1. if CONFIG_USB_GADGET_SUPERSPEED=true, existing gadget drivers are
> > > still functional with dummy_hcd since as I already mentioned, they
> > > will be enumerated through HS root hub and thus the gadget.speed will
> > be set to HS.
> > > This is true for all gadget drivers, including the once that don't
> > > define SS descriptors.
> > 
> > only due the module parameter, right ?
> 
> Due to the module parameter default value being FALSE, yes.

Ok, so let's take your approach but change the speed in the gadget
driver structure, put your ifdef somewhere like here:

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 1ba4bef..d02d6e8 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1224,7 +1224,11 @@ composite_resume(struct usb_gadget *gadget)
 /*-------------------------------------------------------------------------*/
 
 static struct usb_gadget_driver composite_driver = {
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
+       .speed          = USB_SPEE_SUPER,
+#else
        .speed          = USB_SPEED_HIGH,
+#endif
 
        .unbind         = composite_unbind,
 

-- 
balbi

Attachment: signature.asc
Description: Digital signature