[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a2ea1fa0-645d-2a16-f396-797e5b96fd45@omp.ru>
Date: Tue, 3 Sep 2024 16:20:26 +0300
From: Sergey Shtylyov <s.shtylyov@....ru>
To: Tomas Marek <tomas.marek@...est.cz>, <hminas@...opsys.com>,
<gregkh@...uxfoundation.org>
CC: <Arthur.Petrosyan@...opsys.com>, <linux-usb@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <oleg.karfich@...o.com>
Subject: Re: [PATCH] usb: dwc2: drd: fix clock gating on USB role switch
On 9/3/24 12:41 PM, Tomas Marek wrote:
> The dwc2_handle_usb_suspend_intr() function disables gadget clocks in USB
> peripheral mode when no other power-down mode is available (introduced by
> commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.")).
> However, the dwc2_drd_role_sw_set() USB role update handler attempts to
> read DWC2 registers if the USB role has changed while the USB is in suspend
> mode (when the clocks are gated). This causes the system to hang.
>
> Release gadget clocks before handling the USB role update.
>
> Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.")
>
> Signed-off-by: Tomas Marek <tomas.marek@...est.cz>
> ---
> drivers/usb/dwc2/drd.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> index a8605b02115b..ccb33cd1f04b 100644
> --- a/drivers/usb/dwc2/drd.c
> +++ b/drivers/usb/dwc2/drd.c
> @@ -127,6 +127,18 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> role = USB_ROLE_DEVICE;
> }
>
> +#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \
> + IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
> + if (dwc2_is_device_mode(hsotg)) {
Why not avoid #ifdef by doing e.g.:
if ((IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \
IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE) &&
dwc2_is_device_mode(hsotg)) {
> + if (hsotg->lx_state == DWC2_L2) {
> + if (hsotg->params.power_down ==
> + DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended &&
> + !hsotg->params.no_clock_gating)
Well, these *if*s are mergeable too... :-)
> + dwc2_gadget_exit_clock_gating(hsotg, 0);
> + }
> + }
> +#endif
> +
[...]
MBR, Sergey
Powered by blists - more mailing lists