| Message ID | 1678127568-10609-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 |
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
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 + * 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. + * + * 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; + + 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);
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 | 40 ++++++++++++++++++++++++++++++++++++++++ include/linux/usb/composite.h | 6 ++++++ include/linux/usb/gadget.h | 1 + 3 files changed, 47 insertions(+)