Re: [PATCH 2/3] usb: dwc2: drd: fix dwc2_drd_role_sw_set when clock could be disabled

[Minas Harutyunyan]

On 10/5/2021 1:53 PM, Amelie Delaunay wrote:
> In case of USB_DR_MODE_PERIPHERAL, the OTG clock is disabled at the end of
> the probe (it is not the case if USB_DR_MODE_HOST or USB_DR_MODE_OTG).
> The clock is then enabled on udc_start.
> If dwc2_drd_role_sw_set is called before udc_start (it is the case if the
> usb cable is plugged at boot), GOTGCTL and GUSBCFG registers cannot be
> read/written, so session cannot be overridden.
> To avoid this case, check the ll_hw_enabled value and enable the clock if
> it is available, and disable it after the override.
> 
> Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support")
> Signed-off-by: Amelie Delaunay <amelie.delaunay@xxxxxxxxxxx>

Acked-by: Minas Harutyunyan <Minas.Harutyunyan@xxxxxxxxxxxx>

> ---
>   drivers/usb/dwc2/drd.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> index 80eae88d76dd..99672360f34b 100644
> --- a/drivers/usb/dwc2/drd.c
> +++ b/drivers/usb/dwc2/drd.c
> @@ -7,6 +7,7 @@
>    * Author(s): Amelie Delaunay <amelie.delaunay@xxxxxx>
>    */
>   
> +#include <linux/clk.h>
>   #include <linux/iopoll.h>
>   #include <linux/platform_device.h>
>   #include <linux/usb/role.h>
> @@ -86,6 +87,20 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
>   	}
>   #endif
>   
> +	/*
> +	 * In case of USB_DR_MODE_PERIPHERAL, clock is disabled at the end of
> +	 * the probe and enabled on udc_start.
> +	 * If role-switch set is called before the udc_start, we need to enable
> +	 * the clock to read/write GOTGCTL and GUSBCFG registers to override
> +	 * mode and sessions. It is the case if cable is plugged at boot.
> +	 */
> +	if (!hsotg->ll_hw_enabled && hsotg->clk) {
> +		int ret = clk_prepare_enable(hsotg->clk);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
>   	spin_lock_irqsave(&hsotg->lock, flags);
>   
>   	if (role == USB_ROLE_HOST) {
> @@ -110,6 +125,9 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
>   		/* This will raise a Connector ID Status Change Interrupt */
>   		dwc2_force_mode(hsotg, role == USB_ROLE_HOST);
>   
> +	if (!hsotg->ll_hw_enabled && hsotg->clk)
> +		clk_disable_unprepare(hsotg->clk);
> +
>   	dev_dbg(hsotg->dev, "%s-session valid\n",
>   		role == USB_ROLE_NONE ? "No" :
>   		role == USB_ROLE_HOST ? "A" : "B");
>