| Message ID | 1678127568-10609-1-git-send-email-quic_eserrao@quicinc.com |
|---|---|
| Headers | show |
| Series | Add function suspend/resume and remote wakeup support | expand |
Hi Elson, On Mon, Mar 06, 2023, Elson Roy Serrao wrote: > A function which is in function suspend state has to send a > function wake notification to the host in case it needs to This should be rephrased. The device doesn't send wakeup device notification while in suspend. > exit from this state and resume data transfer. Add support to > handle such requests by exposing a new gadget op. Please provide more info in the commit message describing the change. You did more than just adding a new gadget op here. > > Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com> > --- > drivers/usb/gadget/composite.c | 40 ++++++++++++++++++++++++++++++++++++++++ > include/linux/usb/composite.h | 6 ++++++ > include/linux/usb/gadget.h | 1 + > 3 files changed, 47 insertions(+) > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > index 7512854..7ae78c05 100644 > --- a/drivers/usb/gadget/composite.c > +++ b/drivers/usb/gadget/composite.c > @@ -492,6 +492,46 @@ int usb_interface_id(struct usb_configuration *config, > } > EXPORT_SYMBOL_GPL(usb_interface_id); > > +/** > + * usb_func_wakeup - sends function wake notification to the host. > + * @func: function that sends the remote wakeup notification. > + * > + * Applicable only to enhanced super speed devices when usb functions Enhanced SuperSpeed devices can operate in usb2 speeds. This should be rephased to refer to the operating speed instead. > + * are put in function suspend state and armed for function remote wakeup. > + * On completion, function wake notification is sent. If the device is in > + * low power state it tries to bring the device to active state before > + * sending the wake notification. Since it is a synchronous call, caller > + * must take care of not calling it in interrupt context. For non enhanced > + * super speed devices operating in lower speeds returns negative errno. Same here. Enhanced SuperSpeed devices can operate in lower speeds. It's a good idea that you discarded creating usb_gadget_func_wakeup(). In usb_gadget_wakeup(), we place the burden of checking whether the host enabled remote wakeup in the UDC driver. For usb_func_wakeup(), you place the burden of checking whether the host enabled function remote wakeup in the composite layer. If we have usb_gadget_func_wakeup(), then you may need to clarify how to handle that. > + * > + * Returns zero on success, else negative errno. > + */ > +int usb_func_wakeup(struct usb_function *func) > +{ > + struct usb_gadget *gadget = func->config->cdev->gadget; > + int id; > + > + if (!func->func_wakeup_armed) { > + ERROR(func->config->cdev, "not armed for func remote wakeup\n"); > + return -EINVAL; > + } > + > + if (!gadget->ops->func_wakeup) > + return -EOPNOTSUPP; Maybe this check should go first. > + > + for (id = 0; id < MAX_CONFIG_INTERFACES; id++) > + if (func->config->interface[id] == func) > + break; > + > + if (id == MAX_CONFIG_INTERFACES) { > + ERROR(func->config->cdev, "Invalid function\n"); > + return -EINVAL; > + } > + > + return gadget->ops->func_wakeup(gadget, id); > +} > +EXPORT_SYMBOL_GPL(usb_func_wakeup); > + > static u8 encode_bMaxPower(enum usb_device_speed speed, > struct usb_configuration *c) > { > diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h > index d949e91..a2448e9 100644 > --- a/include/linux/usb/composite.h > +++ b/include/linux/usb/composite.h > @@ -150,6 +150,9 @@ struct usb_os_desc_table { > * GetStatus() request when the recipient is Interface. > * @func_suspend: callback to be called when > * SetFeature(FUNCTION_SUSPEND) is reseived > + * @func_suspended: Indicates whether the function is in function suspend state. > + * @func_wakeup_armed: Indicates whether the function is armed by the host for > + * wakeup signaling. > * > * A single USB function uses one or more interfaces, and should in most > * cases support operation at both full and high speeds. Each function is > @@ -220,6 +223,8 @@ struct usb_function { > int (*get_status)(struct usb_function *); > int (*func_suspend)(struct usb_function *, > u8 suspend_opt); > + bool func_suspended; > + bool func_wakeup_armed; > /* private: */ > /* internals */ > struct list_head list; > @@ -241,6 +246,7 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g, struct usb_function *f, > > int config_ep_by_speed(struct usb_gadget *g, struct usb_function *f, > struct usb_ep *_ep); > +int usb_func_wakeup(struct usb_function *func); > > #define MAX_CONFIG_INTERFACES 16 /* arbitrary; max 255 */ > > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index 1d79612..75bda078 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -310,6 +310,7 @@ struct usb_udc; > struct usb_gadget_ops { > int (*get_frame)(struct usb_gadget *); > int (*wakeup)(struct usb_gadget *); > + int (*func_wakeup)(struct usb_gadget *gadget, int intf_id); > int (*set_remote_wakeup)(struct usb_gadget *, int set); > int (*set_selfpowered) (struct usb_gadget *, int is_selfpowered); > int (*vbus_session) (struct usb_gadget *, int is_active); > -- > 2.7.4 > Thanks, Thinh
On Mon, Mar 06, 2023, Elson Roy Serrao wrote: > An usb device can initate a remote wakeup and bring the link out of > suspend as dictated by the DEVICE_REMOTE_WAKEUP feature selector. > Add support to handle this packet and set the remote wakeup capability. > > Some hosts may take longer time to initiate the resume signaling after > device triggers a remote wakeup. So add async support to the wakeup API > by enabling link status change events. > > Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com> > --- > drivers/usb/dwc3/core.h | 2 ++ > drivers/usb/dwc3/ep0.c | 4 +++ > drivers/usb/dwc3/gadget.c | 76 +++++++++++++++++++++++++++++++++++++++++++---- > 3 files changed, 76 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 582ebd9..cc78236 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1110,6 +1110,7 @@ struct dwc3_scratchpad_array { > * 3 - Reserved > * @dis_metastability_quirk: set to disable metastability quirk. > * @dis_split_quirk: set to disable split boundary. > + * @wakeup_configured: set if the device is configured for remote wakeup. > * @imod_interval: set the interrupt moderation interval in 250ns > * increments or 0 to disable. > * @max_cfg_eps: current max number of IN eps used across all USB configs. > @@ -1327,6 +1328,7 @@ struct dwc3 { > > unsigned dis_split_quirk:1; > unsigned async_callbacks:1; > + unsigned wakeup_configured:1; > > u16 imod_interval; > > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c > index 61de693..f90fa55 100644 > --- a/drivers/usb/dwc3/ep0.c > +++ b/drivers/usb/dwc3/ep0.c > @@ -356,6 +356,9 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc, > usb_status |= 1 << USB_DEV_STAT_U1_ENABLED; > if (reg & DWC3_DCTL_INITU2ENA) > usb_status |= 1 << USB_DEV_STAT_U2_ENABLED; > + } else { > + usb_status |= dwc->gadget->wakeup_armed << > + USB_DEVICE_REMOTE_WAKEUP; > } > > break; > @@ -476,6 +479,7 @@ static int dwc3_ep0_handle_device(struct dwc3 *dwc, > > switch (wValue) { > case USB_DEVICE_REMOTE_WAKEUP: > + dwc->gadget->wakeup_armed = set; Check wakeup_configured here to decide whether it can be set. Thanks, Thinh