Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i51094778:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-841-g7899e99a45-fm-20220811.002-g7899e99a
Mime-Version: 1.0
Message-Id: <8fc4eb4b-25c4-4d4c-8203-1e5c314436b3@www.fastmail.com>
In-Reply-To: <20220901083446.3799754-1-william.wu@rock-chips.com>
References: <20220901083446.3799754-1-william.wu@rock-chips.com>
Date:   Thu, 01 Sep 2022 19:38:53 +0200
From:   "Sven Peter" <sven@svenpeter.dev>
To:     "William Wu" <william.wu@rock-chips.com>,
        "Felipe Balbi" <balbi@kernel.org>,
        "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc:     linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        "Fabio Aiuto" <fabioaiuto83@gmail.com>, frank.wang@rock-chips.com,
        jianwei.zheng@rock-chips.com
Subject: Re: [PATCH] usb: dwc3: core: leave default DMA if the controller does not
 support 64-bit DMA
Content-Type: text/plain
Precedence: bulk


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