Re: [PATCH 1/1] usb: dwc3: let role switch handle initial state

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

Hi,

On Tue, Oct 17, 2023, Alexander Stein wrote:
> From: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> 
> When we have a role switch device attached, we should not configure our
> initial role. Leave this up to the role switch device, that should
> detect and signal the initial role.
> 
> This fixes situations where a Type-A plug is connected already when the
> driver is loaded but the default role is set to none or device. In this
> case only an disconnect / reconnect gets the correct mode.

If the default role is none, why isn't there a notification to update
the role on initialization from the connector? The current role should
not be none.

BR,
Thinh

> 
> Signed-off-by: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> I couldn't find any information whether USB role switch drivers are
> supposed to call usb_role_switch_set_role() during their probe.
> But this seems sensible, otherwise the actual/initial state is unknown.
> 
>  drivers/usb/dwc3/drd.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index 039bf241769af..1c2e504a5d8ba 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -496,15 +496,8 @@ static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch *sw)
>  static int dwc3_setup_role_switch(struct dwc3 *dwc)
>  {
>  	struct usb_role_switch_desc dwc3_role_switch = {NULL};
> -	u32 mode;
>  
>  	dwc->role_switch_default_mode = usb_get_role_switch_default_mode(dwc->dev);
> -	if (dwc->role_switch_default_mode == USB_DR_MODE_HOST) {
> -		mode = DWC3_GCTL_PRTCAP_HOST;
> -	} else {
> -		dwc->role_switch_default_mode = USB_DR_MODE_PERIPHERAL;
> -		mode = DWC3_GCTL_PRTCAP_DEVICE;
> -	}
>  
>  	dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
>  	dwc3_role_switch.set = dwc3_usb_role_switch_set;
> @@ -526,7 +519,11 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc)
>  		}
>  	}
>  
> -	dwc3_set_mode(dwc, mode);
> +	/*
> +	 * usb_role_switch should implement initial detection and call
> +	 * dwc3_usb_role_switch_set to get the state machine running
> +	 */
> +
>  	return 0;
>  }
>  #else
> -- 
> 2.34.1
>