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

[Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>]

On 12/23/2023 4:01 AM, Thinh Nguyen wrote:
> On Tue, Dec 19, 2023, Krishna Kurapati wrote:
>> Currently for dwc3_usb31 controller, if maximum_speed is limited to
>> 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.
>> 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@quicinc.com>
>> ---
>> Link to v2: https://urldefense.com/v3/__https://lore.kernel.org/all/20230514145118.20973-1-quic_kriskura@quicinc.com/__;!!A4F2R9G_pg!d8OsArNeKYrQqDG89M-Lh-1_c7zIMZp0x1CX6X_m_D2maHEA3QM3qiQV-g4mpb12ZOPT6F8D-ESuCwHxM26peHqe5jtIkQ$
>> Link to v1: https://urldefense.com/v3/__https://lore.kernel.org/all/20230512170107.18821-1-quic_kriskura@quicinc.com/__;!!A4F2R9G_pg!d8OsArNeKYrQqDG89M-Lh-1_c7zIMZp0x1CX6X_m_D2maHEA3QM3qiQV-g4mpb12ZOPT6F8D-ESuCwHxM26peHoSWlLDaw$
>>
>> Discussion regarding the same at:
>> https://urldefense.com/v3/__https://lore.kernel.org/all/e465c69c-3a9d-cbdb-d44e-96b99cfa1a92@quicinc.com/__;!!A4F2R9G_pg!d8OsArNeKYrQqDG89M-Lh-1_c7zIMZp0x1CX6X_m_D2maHEA3QM3qiQV-g4mpb12ZOPT6F8D-ESuCwHxM26peHqdyS2ZMQ$
>> ---
>>   drivers/usb/dwc3/core.c | 12 ++++++++++++
>>   drivers/usb/dwc3/core.h |  5 +++++
>>   2 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index b101dbf8c5dc..056ba95d9295 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1367,6 +1367,18 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>   
>>   	dwc3_config_threshold(dwc);
>>   
>> +	/*
>> +	 * Modify this for all supported Super Speed ports when
>> +	 * multiport support is added.
>> +	 */
>> +	if (hw_mode != DWC3_GHWPARAMS0_MODE_GADGET &&
>> +	    (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);
>> +	}
>> +
>>   	return 0;
>>   
>>   err_power_off_phy:
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index efe6caf4d0e8..e120611a5174 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -172,6 +172,8 @@
>>   #define DWC3_OEVTEN		0xcc0C
>>   #define DWC3_OSTS		0xcc10
>>   
>> +#define DWC3_LLUCTL		0xd024
>> +
>>   /* Bit fields */
>>   
>>   /* Global SoC Bus Configuration INCRx Register 0 */
>> @@ -657,6 +659,9 @@
>>   #define DWC3_OSTS_VBUSVLD		BIT(1)
>>   #define DWC3_OSTS_CONIDSTS		BIT(0)
>>   
>> +/* Force Gen1 speed on Gen2 link */
>> +#define DWC3_LLUCTL_FORCE_GEN1		BIT(10)
>> +
>>   /* Structures */
>>   
>>   struct dwc3_trb;
>> -- 
>> 2.42.0
>>
> 
> I believe you can also loop through and apply to all 16 offsets of
> register LLUCTL without affecting the controller. Regardless whether you
> will apply that for v2 or keep this version:
> 
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> 

Thanks for the review Thinh.

Multiport isn't merged yet. It will be rebased after interrupt cleanup 
and flattening device tree v2 of Bjorn. Once it is merged, I will modify 
this for logic multiple ports.

Regards,
Krishna,