[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230215011913.hoidj25fjssrwnj6@synopsys.com>
Date: Wed, 15 Feb 2023 01:19:19 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Elson Roy Serrao <quic_eserrao@...cinc.com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
"balbi@...nel.org" <balbi@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"quic_wcheng@...cinc.com" <quic_wcheng@...cinc.com>,
"quic_jackp@...cinc.com" <quic_jackp@...cinc.com>
Subject: Re: [PATCH v4 4/5] usb: dwc3: Add function suspend and function
wakeup support
On Mon, Feb 13, 2023, Elson Roy Serrao wrote:
> USB host sends function suspend and function resume notifications to
> the interface through SET_FEATURE/CLEAR_FEATURE setup packets.
> Add support to handle these packets by delegating the requests to
> composite layer. Also add support to handle function wake notification
> requests to exit from function suspend state.
>
> Signed-off-by: Elson Roy Serrao <quic_eserrao@...cinc.com>
> ---
> drivers/usb/dwc3/core.h | 3 +++
> drivers/usb/dwc3/debug.h | 2 ++
> drivers/usb/dwc3/ep0.c | 12 ++++--------
> drivers/usb/dwc3/gadget.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index cc78236..1565b21 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -526,6 +526,7 @@
> #define DWC3_DGCMD_SET_ENDPOINT_NRDY 0x0c
> #define DWC3_DGCMD_SET_ENDPOINT_PRIME 0x0d
> #define DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK 0x10
> +#define DWC3_DGCMD_DEV_NOTIFICATION 0x07
>
> #define DWC3_DGCMD_STATUS(n) (((n) >> 12) & 0x0F)
> #define DWC3_DGCMD_CMDACT BIT(10)
> @@ -538,6 +539,8 @@
> #define DWC3_DGCMDPAR_TX_FIFO BIT(5)
> #define DWC3_DGCMDPAR_LOOPBACK_DIS (0 << 0)
> #define DWC3_DGCMDPAR_LOOPBACK_ENA BIT(0)
> +#define DWC3_DGCMDPAR_DN_FUNC_WAKE BIT(0)
> +#define DWC3_DGCMDPAR_INTF_SEL(n) ((n) << 4)
>
> /* Device Endpoint Command Register */
> #define DWC3_DEPCMD_PARAM_SHIFT 16
> diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
> index 8bb2c9e..09d7038 100644
> --- a/drivers/usb/dwc3/debug.h
> +++ b/drivers/usb/dwc3/debug.h
> @@ -72,6 +72,8 @@ dwc3_gadget_generic_cmd_string(u8 cmd)
> return "Set Endpoint Prime";
> case DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK:
> return "Run SoC Bus Loopback Test";
> + case DWC3_DGCMD_DEV_NOTIFICATION:
> + return "Device Notification";
> default:
> return "UNKNOWN";
> }
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index f90fa55..1e7cc15 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -30,6 +30,8 @@
> static void __dwc3_ep0_do_control_status(struct dwc3 *dwc, struct dwc3_ep *dep);
> static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
> struct dwc3_ep *dep, struct dwc3_request *req);
> +static int dwc3_ep0_delegate_req(struct dwc3 *dwc,
> + struct usb_ctrlrequest *ctrl);
>
> static void dwc3_ep0_prepare_one_trb(struct dwc3_ep *dep,
> dma_addr_t buf_dma, u32 len, u32 type, bool chain)
> @@ -368,7 +370,7 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc,
> * Function Remote Wake Capable D0
> * Function Remote Wakeup D1
> */
> - break;
> + return dwc3_ep0_delegate_req(dwc, ctrl);
>
> case USB_RECIP_ENDPOINT:
> dep = dwc3_wIndex_to_dep(dwc, ctrl->wIndex);
> @@ -514,13 +516,7 @@ static int dwc3_ep0_handle_intf(struct dwc3 *dwc,
>
> switch (wValue) {
> case USB_INTRF_FUNC_SUSPEND:
> - /*
> - * REVISIT: Ideally we would enable some low power mode here,
> - * however it's unclear what we should be doing here.
> - *
> - * For now, we're not doing anything, just making sure we return
> - * 0 so USB Command Verifier tests pass without any errors.
> - */
> + ret = dwc3_ep0_delegate_req(dwc, ctrl);
> break;
> default:
> ret = -EINVAL;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index b7fef5d..9905875 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2391,6 +2391,47 @@ static int dwc3_gadget_wakeup(struct usb_gadget *g)
> return ret;
> }
>
> +static void dwc3_resume_gadget(struct dwc3 *dwc);
> +
> +static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id)
> +{
> + struct dwc3 *dwc = gadget_to_dwc(g);
> + unsigned long flags;
> + int ret;
> + int link_state;
> +
> + if (!dwc->wakeup_configured) {
> + dev_err(dwc->dev, "remote wakeup not configured\n");
> + return -EINVAL;
> + }
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> + /*
> + * If the link is in low power, first bring the link to U0
> + * before triggering function remote wakeup.
This should be reword as below:
If the link is in U3, signal for remote wakeup and wait for link to
transition to U0 before sending device notification.
After the above change, you can add my ack:
Acked-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Thanks,
Thinh
> + */
> + link_state = dwc3_gadget_get_link_state(dwc);
> + if (link_state == DWC3_LINK_STATE_U3) {
> + ret = __dwc3_gadget_wakeup(dwc, false);
> + if (ret) {
> + spin_unlock_irqrestore(&dwc->lock, flags);
> + return -EINVAL;
> + }
> + dwc3_resume_gadget(dwc);
> + dwc->link_state = DWC3_LINK_STATE_U0;
> + }
> +
> + ret = dwc3_send_gadget_generic_command(dwc, DWC3_DGCMD_DEV_NOTIFICATION,
> + DWC3_DGCMDPAR_DN_FUNC_WAKE |
> + DWC3_DGCMDPAR_INTF_SEL(intf_id));
> + if (ret)
> + dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret);
> +
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +
> + return ret;
> +}
> +
> static int dwc3_gadget_set_remote_wakeup(struct usb_gadget *g, int set)
> {
> struct dwc3 *dwc = gadget_to_dwc(g);
> @@ -3028,6 +3069,7 @@ static void dwc3_gadget_async_callbacks(struct usb_gadget *g, bool enable)
> static const struct usb_gadget_ops dwc3_gadget_ops = {
> .get_frame = dwc3_gadget_get_frame,
> .wakeup = dwc3_gadget_wakeup,
> + .func_wakeup = dwc3_gadget_func_wakeup,
> .set_remote_wakeup = dwc3_gadget_set_remote_wakeup,
> .set_selfpowered = dwc3_gadget_set_selfpowered,
> .pullup = dwc3_gadget_pullup,
> --
> 2.7.4
>
Powered by blists - more mailing lists