Re: [PATCH 2/5] usb: gadget: Add function wakeup support

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On 8/2/2022, Elson Roy Serrao wrote:
> An interface which is in function suspend state has to send a function
> wakeup notification to the host in case it needs to initate any data
> transfer. One notable difference between this and the existing remote
> wakeup mechanism is that this can be called per-interface, and a UDC
> would need to know the particular interface number to convey in its
> Device Notification transaction packet.  Hence, we need to introduce
> a new callback in the gadget_ops structure that UDC device drivers
> can implement.  Similarly add a convenience function in the composite
> driver which function drivers can call. Add support to handle such
> requests in the composite layer and invoke the gadget op.

Sending the function wake notification should be done in the controller 
driver. The controller driver knows when is the proper link state 
(U0/ON) the device is in and would notify the host then.

What we need to add in the usb_gadget is whether the device is remote 
wakeup capable. Something like a flag usb_gadget->rw_capable.

We would also need some functions like usb_gadget_enable_remote_wakeup() 
and usb_gadget_disable_remote_wakeup() for the gadget driver to notify 
the controller driver when it checks against USB_CONFIG_ATT_WAKEUP in 
the bmAttributes configuration.

BR,
Thinh

> Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
> ---
>   drivers/usb/gadget/composite.c | 32 ++++++++++++++++++++++++++++++++
>   drivers/usb/gadget/udc/core.c  |  9 +++++++++
>   include/linux/usb/composite.h  |  1 +
>   include/linux/usb/gadget.h     |  2 ++
>   4 files changed, 44 insertions(+)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 403563c..6bdce23 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -490,6 +490,38 @@ 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;
> +	unsigned long flags;
> +
> +	if (!func || !func->config || !func->config->cdev ||
> +	    !func->config->cdev->gadget)
> +		return -EINVAL;
> +
> +	DBG(func->config->cdev, "%s function wakeup\n", func->name);
> +
> +	spin_lock_irqsave(&func->config->cdev->lock, flags);
> +
> +	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 id:%d\n", id);
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id);
> +
> +err:
> +	spin_unlock_irqrestore(&func->config->cdev->lock, flags);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(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 7886497..fe5c504 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -816,6 +816,15 @@ int usb_gadget_activate(struct usb_gadget *gadget)
>   }
>   EXPORT_SYMBOL_GPL(usb_gadget_activate);
>   
> +int usb_gadget_func_wakeup(struct usb_gadget *gadget, int interface_id)
> +{
> +	if (gadget->speed < USB_SPEED_SUPER || !gadget->ops->func_wakeup)
> +		return -EOPNOTSUPP;
> +
> +	return gadget->ops->func_wakeup(gadget, interface_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 9d27622..31b35d7 100644
> --- a/include/linux/usb/composite.h
> +++ b/include/linux/usb/composite.h
> @@ -254,6 +254,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 3ad58b7..76f9de4 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -311,6 +311,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 interface_id);
>   	int	(*set_selfpowered) (struct usb_gadget *, int is_selfpowered);
>   	int	(*vbus_session) (struct usb_gadget *, int is_active);
>   	int	(*vbus_draw) (struct usb_gadget *, unsigned mA);
> @@ -612,6 +613,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 interface_id);
>   #else
>   static inline int usb_gadget_frame_number(struct usb_gadget *gadget)
>   { return 0; }