[PATCH] usb: dwc3: core: leave default DMA if the controller does not support 64-bit DMA

[PATCH] usb: dwc3: core: leave default DMA if the controller does not support 64-bit DMA

[William Wu]

On some DWC3 controllers (e.g. Rockchip SoCs), the DWC3 core
doesn't support 64-bit DMA address width. In this case, this
driver should use the default 32-bit mask. Otherwise, the DWC3
controller will break if it runs on above 4GB physical memory
environment.

This patch reads the DWC_USB3_AWIDTH bits of GHWPARAMS0 which
used for the DMA address width, and only configure 64-bit DMA
mask if the DWC_USB3_AWIDTH is 64.

Fixes: 45d39448b4d0 ("usb: dwc3: support 64 bit DMA in platform driver")
Signed-off-by: William Wu <william.wu@rock-chips.com>
---
 drivers/usb/dwc3/core.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c5c238ab3083..2fcbd05b2af1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1751,12 +1751,6 @@ static int dwc3_probe(struct platform_device *pdev)
 
 	dwc3_get_properties(dwc);
 
-	if (!dwc->sysdev_is_parent) {
-		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
-		if (ret)
-			return ret;
-	}
-
 	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
 	if (IS_ERR(dwc->reset))
 		return PTR_ERR(dwc->reset);
@@ -1823,6 +1817,13 @@ static int dwc3_probe(struct platform_device *pdev)
 	dwc3_cache_hwparams(dwc);
 	device_init_wakeup(&pdev->dev, of_property_read_bool(dev->of_node, "wakeup-source"));
 
+	if (!dwc->sysdev_is_parent &&
+	    DWC3_GHWPARAMS0_AWIDTH(dwc->hwparams.hwparams0) == 64) {
+		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
+		if (ret)
+			goto disable_clks;
+	}
+
 	spin_lock_init(&dwc->lock);
 	mutex_init(&dwc->mutex);
 
-- 
2.34.1

Re: [PATCH] usb: dwc3: core: leave default DMA if the controller does not support 64-bit DMA

[Sven Peter]

On Thu, Sep 1, 2022, at 10:34, William Wu wrote:
> On some DWC3 controllers (e.g. Rockchip SoCs), the DWC3 core
> doesn't support 64-bit DMA address width. In this case, this
> driver should use the default 32-bit mask. Otherwise, the DWC3
> controller will break if it runs on above 4GB physical memory
> environment.
>
> This patch reads the DWC_USB3_AWIDTH bits of GHWPARAMS0 which
> used for the DMA address width, and only configure 64-bit DMA
> mask if the DWC_USB3_AWIDTH is 64.
>
> Fixes: 45d39448b4d0 ("usb: dwc3: support 64 bit DMA in platform driver")
> Signed-off-by: William Wu <william.wu@rock-chips.com>

Reviewed-by: Sven Peter <sven@svenpeter.dev>

> ---
>  drivers/usb/dwc3/core.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c5c238ab3083..2fcbd05b2af1 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1751,12 +1751,6 @@ static int dwc3_probe(struct platform_device *pdev)
> 
>  	dwc3_get_properties(dwc);
> 
> -	if (!dwc->sysdev_is_parent) {
> -		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
> -		if (ret)
> -			return ret;
> -	}
> -
>  	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
>  	if (IS_ERR(dwc->reset))
>  		return PTR_ERR(dwc->reset);
> @@ -1823,6 +1817,13 @@ static int dwc3_probe(struct platform_device 
> *pdev)
>  	dwc3_cache_hwparams(dwc);
>  	device_init_wakeup(&pdev->dev, of_property_read_bool(dev->of_node, 
> "wakeup-source"));
> 
> +	if (!dwc->sysdev_is_parent &&
> +	    DWC3_GHWPARAMS0_AWIDTH(dwc->hwparams.hwparams0) == 64) {
> +		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
> +		if (ret)
> +			goto disable_clks;
> +	}

I guess you could also create the mask from DWC3_GHWPARAMS0_AWIDTH(dwc->hwparams.hwparams0)
directly instead of hardcoding it to 64bit here. Probably doesn't matter though unless
there are some weird systems where dwc3 can only do 48bit DMA but there's actually memory
above that.



Sven

Re: [PATCH] usb: dwc3: core: leave default DMA if the controller does not support 64-bit DMA

[wuliangfeng]

On 2022/9/2 上午1:38, Sven Peter wrote:
> 
> 
> On Thu, Sep 1, 2022, at 10:34, William Wu wrote:
>> On some DWC3 controllers (e.g. Rockchip SoCs), the DWC3 core
>> doesn't support 64-bit DMA address width. In this case, this
>> driver should use the default 32-bit mask. Otherwise, the DWC3
>> controller will break if it runs on above 4GB physical memory
>> environment.
>>
>> This patch reads the DWC_USB3_AWIDTH bits of GHWPARAMS0 which
>> used for the DMA address width, and only configure 64-bit DMA
>> mask if the DWC_USB3_AWIDTH is 64.
>>
>> Fixes: 45d39448b4d0 ("usb: dwc3: support 64 bit DMA in platform driver")
>> Signed-off-by: William Wu <william.wu@rock-chips.com>
> 
> Reviewed-by: Sven Peter <sven@svenpeter.dev>
> 
>> ---
>>   drivers/usb/dwc3/core.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index c5c238ab3083..2fcbd05b2af1 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1751,12 +1751,6 @@ static int dwc3_probe(struct platform_device *pdev)
>>
>>   	dwc3_get_properties(dwc);
>>
>> -	if (!dwc->sysdev_is_parent) {
>> -		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
>> -		if (ret)
>> -			return ret;
>> -	}
>> -
>>   	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
>>   	if (IS_ERR(dwc->reset))
>>   		return PTR_ERR(dwc->reset);
>> @@ -1823,6 +1817,13 @@ static int dwc3_probe(struct platform_device
>> *pdev)
>>   	dwc3_cache_hwparams(dwc);
>>   	device_init_wakeup(&pdev->dev, of_property_read_bool(dev->of_node,
>> "wakeup-source"));
>>
>> +	if (!dwc->sysdev_is_parent &&
>> +	    DWC3_GHWPARAMS0_AWIDTH(dwc->hwparams.hwparams0) == 64) {
>> +		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
>> +		if (ret)
>> +			goto disable_clks;
>> +	}
> 
> I guess you could also create the mask from DWC3_GHWPARAMS0_AWIDTH(dwc->hwparams.hwparams0)
> directly instead of hardcoding it to 64bit here. Probably doesn't matter though unless
> there are some weird systems where dwc3 can only do 48bit DMA but there's actually memory
> above that.
> 
> 
Refer to the DWC3 databook "Table 4-1 Basic Config Parameters", it 
describe that "If your system address bus width for example is 40bits, 
select 64 and tie the address bits[63:40] of the DWC3_usb3 to 0", so it 
seems that "DWC3_GHWPARAMS0_AWIDTH(dwc->hwparams.hwparams0) == 64" can 
cover 40-bit DMA or 48-bit DMA.