linux-kernel - Re: [PATCH v3 4/4] usb: uhci: Add Aspeed AST2700 support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9bb18e89-13bc-469b-8c05-61f8d55a558e@rowland.harvard.edu>
Date: Fri, 19 Sep 2025 11:08:43 -0400
From: Alan Stern <stern@...land.harvard.edu>
To: Ryan Chen <ryan_chen@...eedtech.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Philipp Zabel <p.zabel@...gutronix.de>, linux-usb@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/4] usb: uhci: Add Aspeed AST2700 support

On Fri, Sep 19, 2025 at 10:57:12AM +0800, Ryan Chen wrote:
> Unlike earlier Aspeed SoCs (AST2400/2500/2600) which are limited to
> 32-bit DMA addressing, the UHCI controller in AST2700 supports 64-bit
> DMA. Update the platform UHCI driver to select the appropriate DMA
> mask based on the device tree compatible string.
> 
> Signed-off-by: Ryan Chen <ryan_chen@...eedtech.com>
> ---
>  drivers/usb/host/uhci-platform.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
> index f255358d6242..5b4be9a5764a 100644
> --- a/drivers/usb/host/uhci-platform.c
> +++ b/drivers/usb/host/uhci-platform.c
> @@ -71,6 +71,7 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
>  	struct usb_hcd *hcd;
>  	struct uhci_hcd	*uhci;
>  	struct resource *res;
> +	u64 *dma_mask_ptr;
>  	int ret;
>  
>  	if (usb_disabled())
> @@ -81,7 +82,8 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
>  	 * Since shared usb code relies on it, set it here for now.
>  	 * Once we have dma capability bindings this can go away.
>  	 */
> -	ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	dma_mask_ptr = (u64 *)of_device_get_match_data(&pdev->dev);
> +	ret = dma_coerce_mask_and_coherent(&pdev->dev, *dma_mask_ptr);

What will happen here if of_device_get_match_data() returns 0 or an 
error?  Shouldn't you test for that and then use dma_mask_32 as the 
default mask?

And if you do this then do you need to add the .data fields to the 
existing entries in the platform_uhci_ids table below?

Alan Stern

>  	if (ret)
>  		return ret;
>  
> @@ -114,7 +116,8 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
>  		}
>  		if (of_device_is_compatible(np, "aspeed,ast2400-uhci") ||
>  		    of_device_is_compatible(np, "aspeed,ast2500-uhci") ||
> -		    of_device_is_compatible(np, "aspeed,ast2600-uhci")) {
> +		    of_device_is_compatible(np, "aspeed,ast2600-uhci") ||
> +		    of_device_is_compatible(np, "aspeed,ast2700-uhci")) {
>  			uhci->is_aspeed = 1;
>  			dev_info(&pdev->dev,
>  				 "Enabled Aspeed implementation workarounds\n");
> @@ -189,9 +192,13 @@ static void uhci_hcd_platform_shutdown(struct platform_device *op)
>  	uhci_hc_died(hcd_to_uhci(hcd));
>  }
>  
> +static const u64 dma_mask_32 =	DMA_BIT_MASK(32);
> +static const u64 dma_mask_64 =	DMA_BIT_MASK(64);
> +
>  static const struct of_device_id platform_uhci_ids[] = {
> -	{ .compatible = "generic-uhci", },
> -	{ .compatible = "platform-uhci", },
> +	{ .compatible = "generic-uhci", .data = &dma_mask_32},
> +	{ .compatible = "platform-uhci", .data = &dma_mask_32},
> +	{ .compatible = "aspeed,ast2700-uhci", .data = &dma_mask_64},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, platform_uhci_ids);
> -- 
> 2.34.1
>