Re: [RFC] usb: dwc3: core: set force_gen1 bit in USB31 devices if max speed is SS

[Thinh Nguyen]

On Fri, May 12, 2023, Krishna Kurapati wrote:
> Currently for dwc3_usb31 devices, if maximum_speed is limited to

We usually call the controller dwc_usb3, dwc_usb31, or dwc_usb32.

> super-speed in DT, then device mode is limited to SS, but host mode
> still works in SSP.
> 
> The documentation for max-speed property is as follows:
> 
> "Tells USB controllers we want to work up to a certain speed.
> Incase  this isn't passed via DT, USB controllers should default to
> their maximum HW capability."
> 
> It doesn't specify that the property is only for device mode.

Since this isn't really a fix, can we rephrase the lines below

> Fix this by forcing controller supported max speed to Gen1 by
> setting LLUCTL.Force_Gen1 bit if controller is DWC3_USB31 and
> max speed is mentioned as SS in DT.

As follow:
There are cases where we need to limit the host's maximum speed to
SuperSpeed only. Use this property for host mode to contrain host's
speed to SuperSpeed.


> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@xxxxxxxxxxx>
> ---
> Discussion regarding the same at:
> https://urldefense.com/v3/__https://lore.kernel.org/all/e465c69c-3a9d-cbdb-d44e-96b99cfa1a92@xxxxxxxxxxx/__;!!A4F2R9G_pg!YiQpjZIJAw-yu6gEwbKqb5nusjnKQ9dQJrulx39lQP-7JMhcNA2xd8uLJoZ_HE8SuG4Rm2uvhJTSdQ2k0fJVAxU2RWYHHg$ 
> 
>  drivers/usb/dwc3/core.c | 13 +++++++++++++
>  drivers/usb/dwc3/core.h |  4 ++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 0beaab932e7d..989dc76ecbca 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -116,6 +116,18 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
>  	dwc->current_dr_role = mode;
>  }
>  
> +static void dwc3_configure_host_speed(struct dwc3 *dwc)
> +{
> +	u32 reg;
> +
> +	if (DWC3_IP_IS(DWC31) &&
> +	   (dwc->maximum_speed == USB_SPEED_SUPER)) {
> +		reg = dwc3_readl(dwc->regs, DWC3_LLUCTL);
> +		reg |= DWC3_LLUCTL_FORCE_GEN1;
> +		dwc3_writel(dwc->regs, DWC3_LLUCTL, reg);
> +	}
> +}
> +
>  static void __dwc3_set_mode(struct work_struct *work)
>  {
>  	struct dwc3 *dwc = work_to_dwc(work);
> @@ -194,6 +206,7 @@ static void __dwc3_set_mode(struct work_struct *work)
>  
>  	switch (desired_dr_role) {
>  	case DWC3_GCTL_PRTCAP_HOST:
> +		dwc3_configure_host_speed(dwc);

The LLUCTL doesn't change until there's a Vcc reset. Let's just
initialize it once during dwc3_core_init() if the GHWPARAM indicates the
controller is DRD or host only.

>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index d56457c02996..29b780a58dc6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -121,6 +121,10 @@
>  #define DWC3_GPRTBIMAP_FS0	0xc188
>  #define DWC3_GPRTBIMAP_FS1	0xc18c
>  #define DWC3_GUCTL2		0xc19c
> +#define DWC3_LLUCTL		0xd024

Please place the register according to its offset order.

> +
> +/* Force Gen1 speed on Gen2 link */
> +#define DWC3_LLUCTL_FORCE_GEN1	BIT(10)
>  
>  #define DWC3_VER_NUMBER		0xc1a0
>  #define DWC3_VER_TYPE		0xc1a4
> -- 
> 2.40.0
> 

Thanks,
Thinh