| Message ID | 1676586588-25989-4-git-send-email-quic_eserrao@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add function suspend/resume and remote wakeup support | expand |
On Thu, Feb 16, 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 > exit from this state and resume data transfer. Add support to > handle such requests by exposing a new gadget op. > > Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com> > --- > drivers/usb/gadget/composite.c | 24 ++++++++++++++++++++++++ > drivers/usb/gadget/udc/core.c | 21 +++++++++++++++++++++ > include/linux/usb/composite.h | 6 ++++++ > include/linux/usb/gadget.h | 4 ++++ > 4 files changed, 55 insertions(+) > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > index a37a8f4..f649f997 100644 > --- a/drivers/usb/gadget/composite.c > +++ b/drivers/usb/gadget/composite.c > @@ -492,6 +492,30 @@ int usb_interface_id(struct usb_configuration *config, > } > EXPORT_SYMBOL_GPL(usb_interface_id); > > +int usb_func_wakeup(struct usb_function *func) > +{ > + int ret, id; > + > + if (!func->func_wakeup_armed) { > + ERROR(func->config->cdev, "not armed for func remote wakeup\n"); > + return -EINVAL; > + } > + > + 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; > + } > + > + ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(usb_func_wakeup); > + > static u8 encode_bMaxPower(enum usb_device_speed speed, > struct usb_configuration *c) > { > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index 3dcbba7..59e7a7e 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -846,6 +846,27 @@ int usb_gadget_activate(struct usb_gadget *gadget) > } > EXPORT_SYMBOL_GPL(usb_gadget_activate); > > +/** > + * usb_gadget_func_wakeup - sends function wake notification to the host. > + * @gadget: controller used to wake up the host > + * @interface_id: interface on which function wake notification is sent. Device notification is only applicable for eSS devices. What will happen if the device is operating in lower speed and the driver calls this function? Thanks, Thinh > + * > + * 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. > + * > + * Returns zero on success, else negative errno. > + */ > +int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id) > +{ > + if (!gadget->ops->func_wakeup) > + return -EOPNOTSUPP; > + > + return gadget->ops->func_wakeup(gadget, intf_id); > +} > +EXPORT_SYMBOL_GPL(usb_gadget_func_wakeup);
On 2/16/2023 3:58 PM, Thinh Nguyen wrote: > On Thu, Feb 16, 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 >> exit from this state and resume data transfer. Add support to >> handle such requests by exposing a new gadget op. >> >> Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com> >> --- >> drivers/usb/gadget/composite.c | 24 ++++++++++++++++++++++++ >> drivers/usb/gadget/udc/core.c | 21 +++++++++++++++++++++ >> include/linux/usb/composite.h | 6 ++++++ >> include/linux/usb/gadget.h | 4 ++++ >> 4 files changed, 55 insertions(+) >> >> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c >> index a37a8f4..f649f997 100644 >> --- a/drivers/usb/gadget/composite.c >> +++ b/drivers/usb/gadget/composite.c >> @@ -492,6 +492,30 @@ int usb_interface_id(struct usb_configuration *config, >> } >> EXPORT_SYMBOL_GPL(usb_interface_id); >> >> +int usb_func_wakeup(struct usb_function *func) >> +{ >> + int ret, id; >> + >> + if (!func->func_wakeup_armed) { >> + ERROR(func->config->cdev, "not armed for func remote wakeup\n"); >> + return -EINVAL; >> + } >> + >> + 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; >> + } >> + >> + ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id); >> + >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(usb_func_wakeup); >> + >> static u8 encode_bMaxPower(enum usb_device_speed speed, >> struct usb_configuration *c) >> { >> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c >> index 3dcbba7..59e7a7e 100644 >> --- a/drivers/usb/gadget/udc/core.c >> +++ b/drivers/usb/gadget/udc/core.c >> @@ -846,6 +846,27 @@ int usb_gadget_activate(struct usb_gadget *gadget) >> } >> EXPORT_SYMBOL_GPL(usb_gadget_activate); >> >> +/** >> + * usb_gadget_func_wakeup - sends function wake notification to the host. >> + * @gadget: controller used to wake up the host >> + * @interface_id: interface on which function wake notification is sent. > > Device notification is only applicable for eSS devices. What will happen > if the device is operating in lower speed and the driver calls this > function? > > Thanks, > Thinh > Since the non-eSS devices dont support function suspend, the function suspend feature selector is not sent by the host and the function is not armed for sending function remote wakeup. So the usb_func_wakeup() API that is called from the function drivers fails the attempt int usb_func_wakeup(struct usb_function *func) { int ret, id; if (!func->func_wakeup_armed) { ERROR(func->config->cdev, "not armed for func remote wakeup\n"); return -EINVAL; } Let me know if its better to add an explicit speed check as well here. Thanks Elson >> + * >> + * 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. >> + * >> + * Returns zero on success, else negative errno. >> + */ >> +int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id) >> +{ >> + if (!gadget->ops->func_wakeup) >> + return -EOPNOTSUPP; >> + >> + return gadget->ops->func_wakeup(gadget, intf_id); >> +} >> +EXPORT_SYMBOL_GPL(usb_gadget_func_wakeup);
On Thu, Feb 16, 2023, Elson Serrao wrote: > > > On 2/16/2023 3:58 PM, Thinh Nguyen wrote: > > On Thu, Feb 16, 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 > > > exit from this state and resume data transfer. Add support to > > > handle such requests by exposing a new gadget op. > > > > > > Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com> > > > --- > > > drivers/usb/gadget/composite.c | 24 ++++++++++++++++++++++++ > > > drivers/usb/gadget/udc/core.c | 21 +++++++++++++++++++++ > > > include/linux/usb/composite.h | 6 ++++++ > > > include/linux/usb/gadget.h | 4 ++++ > > > 4 files changed, 55 insertions(+) > > > > > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > > > index a37a8f4..f649f997 100644 > > > --- a/drivers/usb/gadget/composite.c > > > +++ b/drivers/usb/gadget/composite.c > > > @@ -492,6 +492,30 @@ int usb_interface_id(struct usb_configuration *config, > > > } > > > EXPORT_SYMBOL_GPL(usb_interface_id); > > > +int usb_func_wakeup(struct usb_function *func) > > > +{ > > > + int ret, id; > > > + > > > + if (!func->func_wakeup_armed) { > > > + ERROR(func->config->cdev, "not armed for func remote wakeup\n"); > > > + return -EINVAL; > > > + } > > > + > > > + 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; > > > + } > > > + > > > + ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id); > > > + > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(usb_func_wakeup); > > > + > > > static u8 encode_bMaxPower(enum usb_device_speed speed, > > > struct usb_configuration *c) > > > { > > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > > > index 3dcbba7..59e7a7e 100644 > > > --- a/drivers/usb/gadget/udc/core.c > > > +++ b/drivers/usb/gadget/udc/core.c > > > @@ -846,6 +846,27 @@ int usb_gadget_activate(struct usb_gadget *gadget) > > > } > > > EXPORT_SYMBOL_GPL(usb_gadget_activate); > > > +/** > > > + * usb_gadget_func_wakeup - sends function wake notification to the host. > > > + * @gadget: controller used to wake up the host > > > + * @interface_id: interface on which function wake notification is sent. > > > > Device notification is only applicable for eSS devices. What will happen > > if the device is operating in lower speed and the driver calls this > > function? > > > > Thanks, > > Thinh > > > > Since the non-eSS devices dont support function suspend, the function > suspend feature selector is not sent by the host and the function is not > armed for sending function remote wakeup. So the usb_func_wakeup() API that > is called from the function drivers fails the attempt We may be able to say that for usb_func_wakeup() and document more on the func_wakeup_armed flag. However, the driver can still call usb_gadget_func_wakeup() directly right? Can you document the expected behavior of usb_gadget_func_wakeup(). Would it fall back to behave like usb_gadget_wakeup()? Or would it become no-op? Whichever you choose, please document it. Thanks, Thinh > > int usb_func_wakeup(struct usb_function *func) > { > int ret, id; > > if (!func->func_wakeup_armed) { > ERROR(func->config->cdev, "not armed for func remote wakeup\n"); > return -EINVAL; > } > > Let me know if its better to add an explicit speed check as well here. > > Thanks > Elson >
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index a37a8f4..f649f997 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -492,6 +492,30 @@ int usb_interface_id(struct usb_configuration *config, } EXPORT_SYMBOL_GPL(usb_interface_id); +int usb_func_wakeup(struct usb_function *func) +{ + int ret, id; + + if (!func->func_wakeup_armed) { + ERROR(func->config->cdev, "not armed for func remote wakeup\n"); + return -EINVAL; + } + + 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; + } + + ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id); + + return ret; +} +EXPORT_SYMBOL_GPL(usb_func_wakeup); + static u8 encode_bMaxPower(enum usb_device_speed speed, struct usb_configuration *c) { diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 3dcbba7..59e7a7e 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -846,6 +846,27 @@ int usb_gadget_activate(struct usb_gadget *gadget) } EXPORT_SYMBOL_GPL(usb_gadget_activate); +/** + * usb_gadget_func_wakeup - sends function wake notification to the host. + * @gadget: controller used to wake up the host + * @interface_id: interface on which function wake notification is sent. + * + * 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. + * + * Returns zero on success, else negative errno. + */ +int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id) +{ + if (!gadget->ops->func_wakeup) + return -EOPNOTSUPP; + + return gadget->ops->func_wakeup(gadget, intf_id); +} +EXPORT_SYMBOL_GPL(usb_gadget_func_wakeup); + /* ------------------------------------------------------------------------- */ #ifdef CONFIG_HAS_DMA 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..2a512c5 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); @@ -617,6 +618,7 @@ int usb_gadget_disconnect(struct usb_gadget *gadget); int usb_gadget_deactivate(struct usb_gadget *gadget); int usb_gadget_activate(struct usb_gadget *gadget); int usb_gadget_check_config(struct usb_gadget *gadget); +int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id); #else static inline int usb_gadget_frame_number(struct usb_gadget *gadget) { return 0; } @@ -644,6 +646,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget) { return 0; } static inline int usb_gadget_check_config(struct usb_gadget *gadget) { return 0; } +static inline int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id) +{ return 0; } #endif /* CONFIG_USB_GADGET */ /*-------------------------------------------------------------------------*/
A function which is in function suspend state has to send a function wake notification to the host in case it needs to exit from this state and resume data transfer. Add support to handle such requests by exposing a new gadget op. Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com> --- drivers/usb/gadget/composite.c | 24 ++++++++++++++++++++++++ drivers/usb/gadget/udc/core.c | 21 +++++++++++++++++++++ include/linux/usb/composite.h | 6 ++++++ include/linux/usb/gadget.h | 4 ++++ 4 files changed, 55 insertions(+)