Hi, On Sun, Jul 03, 2011 at 05:29:32PM +0300, Amit Blay wrote: > 1. Add missing definitions in ch9.h for requests tarageted to an
typo... s/tarageted/targeted > Interface: GET_STATUS & SET_FEATURE(FUNC_SUSPEND). if it's targeted to an interface, why don't you just let the gadget driver handle it ? composite.c tracks all functions already and we should just call function->setup() to let the correct function handle this. > 2. Add func_wakeup api to usb_gadget_ops. > Super-Speed device must provide interface number to the UDC when > it triggers remote wakeup (function wake). > The func_wakeup api is used instead of the wakeup api, when the > gadget is connected in Super-Speed. The wakeup api is left as is, > and it is used when the gadget is connected in High-Speed. Therefore, > no change is required in non Super-Speed UDCs. first of all, this needs to be splitted. You shouldn't do more than one thing in a patch ;-) > Signed-off-by: Amit Blay <[email protected]> > > --- > drivers/usb/gadget/udc-core.c | 2 +- > drivers/usb/gadget/zero.c | 6 +++++- > include/linux/usb/ch9.h | 8 ++++++++ > include/linux/usb/gadget.h | 35 +++++++++++++++++++++++++++-------- > 4 files changed, 41 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c > index 05ba472..beb7e89 100644 > --- a/drivers/usb/gadget/udc-core.c > +++ b/drivers/usb/gadget/udc-core.c > @@ -347,7 +347,7 @@ static ssize_t usb_udc_srp_store(struct device *dev, > struct usb_udc *udc = dev_get_drvdata(dev); > > if (sysfs_streq(buf, "1")) > - usb_gadget_wakeup(udc->gadget); > + usb_gadget_wakeup(udc->gadget, 0); > > return n; > } > diff --git a/drivers/usb/gadget/zero.c b/drivers/usb/gadget/zero.c > index 00e2fd2..a5d13c6 100644 > --- a/drivers/usb/gadget/zero.c > +++ b/drivers/usb/gadget/zero.c > @@ -239,7 +239,11 @@ static void zero_autoresume(unsigned long _c) > * because of some direct user request. > */ > if (g->speed != USB_SPEED_UNKNOWN) { > - int status = usb_gadget_wakeup(g); > + /* > + * The single interface of the current configuration > + * triggers the wakeup. > + */ > + int status = usb_gadget_wakeup(g, 1); no no, I think this should be handled by the function itself (f_*.c). > INFO(cdev, "%s --> %d\n", __func__, status); > } > } > diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h > index 0fd3fbd..404c10e 100644 > --- a/include/linux/usb/ch9.h > +++ b/include/linux/usb/ch9.h > @@ -142,7 +142,13 @@ > #define USB_DEVICE_LTM_ENABLE 50 /* dev may send LTM */ > #define USB_INTRF_FUNC_SUSPEND 0 /* function suspend */ > > +/* > + * Function Suspend Options as defined by USB 3.0 > + * See USB 3.0 spec Table 9-7 > + */ > #define USB_INTR_FUNC_SUSPEND_OPT_MASK 0xFF00 > +#define USB_INTR_FUNC_SUSPEND_SUSP 1 /* function suspend option */ > +#define USB_INTR_FUNC_SUSPEND_RWAKE_EN 2 /* function wake enabled > option */ > > #define USB_ENDPOINT_HALT 0 /* IN/OUT will STALL */ > > @@ -150,6 +156,8 @@ > #define USB_DEV_STAT_U1_ENABLED 2 /* transition into U1 > state */ > #define USB_DEV_STAT_U2_ENABLED 3 /* transition into U2 > state */ > #define USB_DEV_STAT_LTM_ENABLED 4 /* Latency tolerance messages */ > +#define USB_INTR_STAT_RWAKE_CAP 0 /* Function wake > capable */ > +#define USB_INTR_STAT_RWAKE_EN 1 /* Function > wake enabled */ > > /** > * struct usb_ctrlrequest - SETUP data for a USB device control request > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index 087f4b9..3ebbc11 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -458,7 +458,11 @@ struct usb_gadget_ops { > int (*pullup) (struct usb_gadget *, int is_on); > int (*ioctl)(struct usb_gadget *, > unsigned code, unsigned long param); > + > + /* USB 3.0 additions */ this comment is not part of this patch ;-) (nitpicking, I know) > void (*get_config_params)(struct usb_dcd_config_params *); > + int (*func_wakeup)(struct usb_gadget *, int interface_id); > + > int (*udc_start)(struct usb_gadget *, > struct usb_gadget_driver *); > int (*udc_stop)(struct usb_gadget *, > @@ -607,21 +611,36 @@ static inline int usb_gadget_frame_number(struct > usb_gadget *gadget) > /** > * usb_gadget_wakeup - tries to wake up the host connected to this gadget > * @gadget: controller used to wake up the host > + * @interface_id: id of the first interface of the function that > + * triggered the wake up > * > * Returns zero on success, else negative error code if the hardware > * doesn't support such attempts, or its support has not been enabled > * by the usb host. Drivers must return device descriptors that report > * their ability to support this, or hosts won't enable it. > + * For Super-Speed devices only, the gadget should provide the > + * id of the first interface that triggered the wake up > + * (function wake). For non Super-Speed devices, the value of > + * this parameter doesn't matter. > * > - * This may also try to use SRP to wake the host and start enumeration, > - * even if OTG isn't otherwise in use. OTG devices may also start > - * remote wakeup even when hosts don't explicitly enable it. > + * This may also try to use SRP to wake the host and start > + * enumeration, even if OTG isn't otherwise in use. OTG devices > + * may also start remote wakeup even when hosts don't explicitly > + * enable it. > */ > -static inline int usb_gadget_wakeup(struct usb_gadget *gadget) > +static inline int usb_gadget_wakeup(struct usb_gadget *gadget, int > interface_id) > { > - if (!gadget->ops->wakeup) > - return -EOPNOTSUPP; > - return gadget->ops->wakeup(gadget); > + if (gadget->speed == USB_SPEED_SUPER) { > + if (!gadget->ops->func_wakeup) > + return -EOPNOTSUPP; > + > + return gadget->ops->func_wakeup(gadget, interface_id); I really don't like this... You're just abusing this interface. Either add a separate one (which I still don't think it's the right approach) or just let the gadget driver handle it, meaning that composite.c would call into f_*.c and the drivers which don't use composite.c will handle it their own way. -- balbi
signature.asc
Description: Digital signature
