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

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi,

Am Samstag, 21. Oktober 2023, 00:43:39 CEST schrieb Thinh Nguyen:
> 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.

dwc->role_switch_default_mode can only be none if the DT is misconfigured, 
e.g. role-switch-default-mode = "";

Calls to usb_role_switch_set_role() from usb role switch drivers will not 
affect dwc->role_switch_default_mode.
I'm wondering if checking for a misconfigured DT is sensible. But this will be 
detected by 'make dtbs_check'.

Best regards,
Alexander

> 
> 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


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/