[PATCH v3 0/4] Add Aspeed AST2700 uhci support

[PATCH v3 0/4] Add Aspeed AST2700 uhci support

[Ryan Chen]

This patch series adds support for the UHCI controller found on the
Aspeed AST2700 SoC.

Compared to earlier SoCs (AST2400/2500/2600), AST2700 UHCI:
 - requires a reset line to be deasserted before use
 - supports 64-bit DMA addressing

This series updates the bindings and platform driver accordingly.

v3:
- uhci-platform.c
 - add reset_control_assert in uhci_hcd_platform_remove.

v2:
- usb-uhci.yaml
 - add required resets for aspeed,ast2700-uhci
- uhci-platform.c
 - change the err_clk before err_reset.

Ryan Chen (4):
  dt-bindings: usb: uhci: Add reset property
  usb: uhci: Add reset control support
  dt-bindings: usb: uhci: Add Aspeed AST2700 compatible
  usb: uhci: Add Aspeed AST2700 support

 .../devicetree/bindings/usb/usb-uhci.yaml     | 13 ++++++++
 drivers/usb/host/uhci-hcd.h                   |  1 +
 drivers/usb/host/uhci-platform.c              | 33 +++++++++++++++----
 3 files changed, 41 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH v3 1/4] dt-bindings: usb: uhci: Add reset property

[Ryan Chen]

The UHCI controller on Aspeed SoCs (including AST2700) requires
its reset line to be deasserted before the controller can be used.
Add an optional "resets" property to the UHCI device tree bindings
to describe the phandle to the reset controller.

This property is optional for platforms which do not require
explicit reset handling.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/usb/usb-uhci.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/usb-uhci.yaml b/Documentation/devicetree/bindings/usb/usb-uhci.yaml
index d8336f72dc1f..b1f2b9bd7921 100644
--- a/Documentation/devicetree/bindings/usb/usb-uhci.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-uhci.yaml
@@ -28,6 +28,9 @@ properties:
   interrupts:
     maxItems: 1
 
+  resets:
+    maxItems: 1
+
   '#ports':
     $ref: /schemas/types.yaml#/definitions/uint32
 
-- 
2.34.1

[PATCH v3 2/4] usb: uhci: Add reset control support

[Ryan Chen]

Some SoCs, such as the Aspeed AST2700, require the UHCI controller
to be taken out of reset before it can operate. Add optional reset
control support to the UHCI platform driver.

The driver now acquires an optional reset line from device tree,
deasserts it during probe, and asserts it again in the error path
and shutdown.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 drivers/usb/host/uhci-hcd.h      |  1 +
 drivers/usb/host/uhci-platform.c | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
index 13ee2a6144b2..4326d1f3ca76 100644
--- a/drivers/usb/host/uhci-hcd.h
+++ b/drivers/usb/host/uhci-hcd.h
@@ -445,6 +445,7 @@ struct uhci_hcd {
 	short load[MAX_PHASE];			/* Periodic allocations */
 
 	struct clk *clk;			/* (optional) clock source */
+	struct reset_control *rsts;		/* (optional) clock reset */
 
 	/* Reset host controller */
 	void	(*reset_hc) (struct uhci_hcd *uhci);
diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
index 62318291f566..f255358d6242 100644
--- a/drivers/usb/host/uhci-platform.c
+++ b/drivers/usb/host/uhci-platform.c
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
+#include <linux/reset.h>
 
 static int uhci_platform_init(struct usb_hcd *hcd)
 {
@@ -132,17 +133,29 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
 		goto err_rmr;
 	}
 
+	uhci->rsts = devm_reset_control_array_get_optional_shared(&pdev->dev);
+	if (IS_ERR(uhci->rsts)) {
+		ret = PTR_ERR(uhci->rsts);
+		goto err_clk;
+	}
+	ret = reset_control_deassert(uhci->rsts);
+	if (ret)
+		goto err_clk;
+
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0)
-		goto err_clk;
+		goto err_reset;
 
 	ret = usb_add_hcd(hcd, ret, IRQF_SHARED);
 	if (ret)
-		goto err_clk;
+		goto err_reset;
 
 	device_wakeup_enable(hcd->self.controller);
 	return 0;
 
+err_reset:
+	if (!IS_ERR_OR_NULL(uhci->rsts))
+		reset_control_assert(uhci->rsts);
 err_clk:
 	clk_disable_unprepare(uhci->clk);
 err_rmr:
@@ -156,6 +169,7 @@ static void uhci_hcd_platform_remove(struct platform_device *pdev)
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct uhci_hcd *uhci = hcd_to_uhci(hcd);
 
+	reset_control_assert(uhci->rsts);
 	clk_disable_unprepare(uhci->clk);
 	usb_remove_hcd(hcd);
 	usb_put_hcd(hcd);
-- 
2.34.1

Re: [PATCH v3 2/4] usb: uhci: Add reset control support

[Alan Stern]

On Fri, Sep 19, 2025 at 10:57:10AM +0800, Ryan Chen wrote:
> Some SoCs, such as the Aspeed AST2700, require the UHCI controller
> to be taken out of reset before it can operate. Add optional reset
> control support to the UHCI platform driver.
> 
> The driver now acquires an optional reset line from device tree,
> deasserts it during probe, and asserts it again in the error path
> and shutdown.
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
>  drivers/usb/host/uhci-hcd.h      |  1 +
>  drivers/usb/host/uhci-platform.c | 18 ++++++++++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
> index 13ee2a6144b2..4326d1f3ca76 100644
> --- a/drivers/usb/host/uhci-hcd.h
> +++ b/drivers/usb/host/uhci-hcd.h
> @@ -445,6 +445,7 @@ struct uhci_hcd {
>  	short load[MAX_PHASE];			/* Periodic allocations */
>  
>  	struct clk *clk;			/* (optional) clock source */
> +	struct reset_control *rsts;		/* (optional) clock reset */
>  
>  	/* Reset host controller */
>  	void	(*reset_hc) (struct uhci_hcd *uhci);
> diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
> index 62318291f566..f255358d6242 100644
> --- a/drivers/usb/host/uhci-platform.c
> +++ b/drivers/usb/host/uhci-platform.c
> @@ -11,6 +11,7 @@
>  #include <linux/of.h>
>  #include <linux/device.h>
>  #include <linux/platform_device.h>
> +#include <linux/reset.h>
>  
>  static int uhci_platform_init(struct usb_hcd *hcd)
>  {
> @@ -132,17 +133,29 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
>  		goto err_rmr;
>  	}
>  
> +	uhci->rsts = devm_reset_control_array_get_optional_shared(&pdev->dev);
> +	if (IS_ERR(uhci->rsts)) {
> +		ret = PTR_ERR(uhci->rsts);
> +		goto err_clk;
> +	}
> +	ret = reset_control_deassert(uhci->rsts);

Does this work right if uhci->rsts is NULL?

> +	if (ret)
> +		goto err_clk;
> +
>  	ret = platform_get_irq(pdev, 0);
>  	if (ret < 0)
> -		goto err_clk;
> +		goto err_reset;
>  
>  	ret = usb_add_hcd(hcd, ret, IRQF_SHARED);
>  	if (ret)
> -		goto err_clk;
> +		goto err_reset;
>  
>  	device_wakeup_enable(hcd->self.controller);
>  	return 0;
>  
> +err_reset:
> +	if (!IS_ERR_OR_NULL(uhci->rsts))
> +		reset_control_assert(uhci->rsts);

How could this code ever execute if uhci->rsts is an ERR_PTR?

Also, why does this code test for NULL...

>  err_clk:
>  	clk_disable_unprepare(uhci->clk);
>  err_rmr:
> @@ -156,6 +169,7 @@ static void uhci_hcd_platform_remove(struct platform_device *pdev)
>  	struct usb_hcd *hcd = platform_get_drvdata(pdev);
>  	struct uhci_hcd *uhci = hcd_to_uhci(hcd);
>  
> +	reset_control_assert(uhci->rsts);

when this code doesn't?

Alan Stern

>  	clk_disable_unprepare(uhci->clk);
>  	usb_remove_hcd(hcd);
>  	usb_put_hcd(hcd);
> -- 
> 2.34.1
> 

RE: [PATCH v3 2/4] usb: uhci: Add reset control support

[Ryan Chen]

> Subject: Re: [PATCH v3 2/4] usb: uhci: Add reset control support
> 
> On Fri, Sep 19, 2025 at 10:57:10AM +0800, Ryan Chen wrote:
> > Some SoCs, such as the Aspeed AST2700, require the UHCI controller to
> > be taken out of reset before it can operate. Add optional reset
> > control support to the UHCI platform driver.
> >
> > The driver now acquires an optional reset line from device tree,
> > deasserts it during probe, and asserts it again in the error path and
> > shutdown.
> >
> > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > ---
> >  drivers/usb/host/uhci-hcd.h      |  1 +
> >  drivers/usb/host/uhci-platform.c | 18 ++++++++++++++++--
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
> > index 13ee2a6144b2..4326d1f3ca76 100644
> > --- a/drivers/usb/host/uhci-hcd.h
> > +++ b/drivers/usb/host/uhci-hcd.h
> > @@ -445,6 +445,7 @@ struct uhci_hcd {
> >  	short load[MAX_PHASE];			/* Periodic allocations */
> >
> >  	struct clk *clk;			/* (optional) clock source */
> > +	struct reset_control *rsts;		/* (optional) clock reset */
> >
> >  	/* Reset host controller */
> >  	void	(*reset_hc) (struct uhci_hcd *uhci);
> > diff --git a/drivers/usb/host/uhci-platform.c
> > b/drivers/usb/host/uhci-platform.c
> > index 62318291f566..f255358d6242 100644
> > --- a/drivers/usb/host/uhci-platform.c
> > +++ b/drivers/usb/host/uhci-platform.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/of.h>
> >  #include <linux/device.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/reset.h>
> >
> >  static int uhci_platform_init(struct usb_hcd *hcd)  { @@ -132,17
> > +133,29 @@ static int uhci_hcd_platform_probe(struct platform_device
> *pdev)
> >  		goto err_rmr;
> >  	}
> >
> > +	uhci->rsts = devm_reset_control_array_get_optional_shared(&pdev->dev);
> > +	if (IS_ERR(uhci->rsts)) {
> > +		ret = PTR_ERR(uhci->rsts);
> > +		goto err_clk;
> > +	}
> > +	ret = reset_control_deassert(uhci->rsts);
> 
> Does this work right if uhci->rsts is NULL?
> 
> > +	if (ret)
> > +		goto err_clk;
> > +
> >  	ret = platform_get_irq(pdev, 0);
> >  	if (ret < 0)
> > -		goto err_clk;
> > +		goto err_reset;
> >
> >  	ret = usb_add_hcd(hcd, ret, IRQF_SHARED);
> >  	if (ret)
> > -		goto err_clk;
> > +		goto err_reset;
> >
> >  	device_wakeup_enable(hcd->self.controller);
> >  	return 0;
> >
> > +err_reset:
> > +	if (!IS_ERR_OR_NULL(uhci->rsts))
> > +		reset_control_assert(uhci->rsts);
> 
> How could this code ever execute if uhci->rsts is an ERR_PTR?
> 
> Also, why does this code test for NULL...

Will add with following.
	if (uhci->rsts) {
		ret = reset_control_deassert(uhci->rsts);
		if (ret)
			goto err_clk;
	}

I will test it with non-resets platform.
> 
> >  err_clk:
> >  	clk_disable_unprepare(uhci->clk);
> >  err_rmr:
> > @@ -156,6 +169,7 @@ static void uhci_hcd_platform_remove(struct
> platform_device *pdev)
> >  	struct usb_hcd *hcd = platform_get_drvdata(pdev);
> >  	struct uhci_hcd *uhci = hcd_to_uhci(hcd);
> >
> > +	reset_control_assert(uhci->rsts);
> 
> when this code doesn't?

Both reset_control_assert() and reset_control_deassert() already handle 
NULL and ERR_PTR safely in the reset framework.
https://github.com/torvalds/linux/blob/master/drivers/reset/core.c#L468-L472

> 
> Alan Stern
> 
> >  	clk_disable_unprepare(uhci->clk);
> >  	usb_remove_hcd(hcd);
> >  	usb_put_hcd(hcd);
> > --
> > 2.34.1
> >

RE: [PATCH v3 2/4] usb: uhci: Add reset control support

[Ryan Chen]

> Subject: RE: [PATCH v3 2/4] usb: uhci: Add reset control support
> 
> > Subject: Re: [PATCH v3 2/4] usb: uhci: Add reset control support
> >
> > On Fri, Sep 19, 2025 at 10:57:10AM +0800, Ryan Chen wrote:
> > > Some SoCs, such as the Aspeed AST2700, require the UHCI controller
> > > to be taken out of reset before it can operate. Add optional reset
> > > control support to the UHCI platform driver.
> > >
> > > The driver now acquires an optional reset line from device tree,
> > > deasserts it during probe, and asserts it again in the error path
> > > and shutdown.
> > >
> > > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > > ---
> > >  drivers/usb/host/uhci-hcd.h      |  1 +
> > >  drivers/usb/host/uhci-platform.c | 18 ++++++++++++++++--
> > >  2 files changed, 17 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/host/uhci-hcd.h
> > > b/drivers/usb/host/uhci-hcd.h index 13ee2a6144b2..4326d1f3ca76
> > > 100644
> > > --- a/drivers/usb/host/uhci-hcd.h
> > > +++ b/drivers/usb/host/uhci-hcd.h
> > > @@ -445,6 +445,7 @@ struct uhci_hcd {
> > >  	short load[MAX_PHASE];			/* Periodic allocations */
> > >
> > >  	struct clk *clk;			/* (optional) clock source */
> > > +	struct reset_control *rsts;		/* (optional) clock reset */
> > >
> > >  	/* Reset host controller */
> > >  	void	(*reset_hc) (struct uhci_hcd *uhci);
> > > diff --git a/drivers/usb/host/uhci-platform.c
> > > b/drivers/usb/host/uhci-platform.c
> > > index 62318291f566..f255358d6242 100644
> > > --- a/drivers/usb/host/uhci-platform.c
> > > +++ b/drivers/usb/host/uhci-platform.c
> > > @@ -11,6 +11,7 @@
> > >  #include <linux/of.h>
> > >  #include <linux/device.h>
> > >  #include <linux/platform_device.h>
> > > +#include <linux/reset.h>
> > >
> > >  static int uhci_platform_init(struct usb_hcd *hcd)  { @@ -132,17
> > > +133,29 @@ static int uhci_hcd_platform_probe(struct platform_device
> > *pdev)
> > >  		goto err_rmr;
> > >  	}
> > >
> > > +	uhci->rsts =
> devm_reset_control_array_get_optional_shared(&pdev->dev);
> > > +	if (IS_ERR(uhci->rsts)) {
> > > +		ret = PTR_ERR(uhci->rsts);
> > > +		goto err_clk;
> > > +	}
> > > +	ret = reset_control_deassert(uhci->rsts);
> >
> > Does this work right if uhci->rsts is NULL?

Update the information that I test with ast2600 (which is no rsts)
When uhci->rsts is null, reset_control_deassert(uhci->rsts) will return 0.
So It still work. I think this is support both rsts and non-rsts platform.

https://github.com/torvalds/linux/blob/master/drivers/reset/core.c#L468-L472


> >
> > > +	if (ret)
> > > +		goto err_clk;
> > > +
> > >  	ret = platform_get_irq(pdev, 0);
> > >  	if (ret < 0)
> > > -		goto err_clk;
> > > +		goto err_reset;
> > >
> > >  	ret = usb_add_hcd(hcd, ret, IRQF_SHARED);
> > >  	if (ret)
> > > -		goto err_clk;
> > > +		goto err_reset;
> > >
> > >  	device_wakeup_enable(hcd->self.controller);
> > >  	return 0;
> > >
> > > +err_reset:
> > > +	if (!IS_ERR_OR_NULL(uhci->rsts))
> > > +		reset_control_assert(uhci->rsts);
> >
> > How could this code ever execute if uhci->rsts is an ERR_PTR?
> >
> > Also, why does this code test for NULL...
> 
> Will add with following.
> 	if (uhci->rsts) {
> 		ret = reset_control_deassert(uhci->rsts);
> 		if (ret)
> 			goto err_clk;
> 	}
> 
> I will test it with non-resets platform.

No need to check if (uhci->rsts) with test with non-rsts platform.

> >
> > >  err_clk:
> > >  	clk_disable_unprepare(uhci->clk);
> > >  err_rmr:
> > > @@ -156,6 +169,7 @@ static void uhci_hcd_platform_remove(struct
> > platform_device *pdev)
> > >  	struct usb_hcd *hcd = platform_get_drvdata(pdev);
> > >  	struct uhci_hcd *uhci = hcd_to_uhci(hcd);
> > >
> > > +	reset_control_assert(uhci->rsts);
> >
> > when this code doesn't?
> 
> Both reset_control_assert() and reset_control_deassert() already handle NULL
> and ERR_PTR safely in the reset framework.
> https://github.com/torvalds/linux/blob/master/drivers/reset/core.c#L468-L472
> 
> >
> > Alan Stern
> >
> > >  	clk_disable_unprepare(uhci->clk);
> > >  	usb_remove_hcd(hcd);
> > >  	usb_put_hcd(hcd);
> > > --
> > > 2.34.1
> > >

[PATCH v3 3/4] dt-bindings: usb: uhci: Add Aspeed AST2700 compatible

[Ryan Chen]

Add the compatible string for Aspeed AST2700 SoC.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/usb/usb-uhci.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/usb-uhci.yaml b/Documentation/devicetree/bindings/usb/usb-uhci.yaml
index b1f2b9bd7921..7b774d0b2742 100644
--- a/Documentation/devicetree/bindings/usb/usb-uhci.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-uhci.yaml
@@ -20,6 +20,7 @@ properties:
               - aspeed,ast2400-uhci
               - aspeed,ast2500-uhci
               - aspeed,ast2600-uhci
+              - aspeed,ast2700-uhci
           - const: generic-uhci
 
   reg:
@@ -53,6 +54,15 @@ allOf:
       required:
         - clocks
 
+ - if:
+      properties:
+        compatible:
+          contains:
+            const: aspeed,ast2700-uhci
+    then:
+      required:
+        - resets
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1

Re: [PATCH v3 3/4] dt-bindings: usb: uhci: Add Aspeed AST2700 compatible

[Rob Herring (Arm)]

On Fri, 19 Sep 2025 10:57:11 +0800, Ryan Chen wrote:
> Add the compatible string for Aspeed AST2700 SoC.
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  Documentation/devicetree/bindings/usb/usb-uhci.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/usb/usb-uhci.yaml:57:2: [error] syntax error: expected <block end>, but found '<block sequence start>' (syntax)
./Documentation/devicetree/bindings/usb/usb-uhci.yaml:58:7: [warning] wrong indentation: expected 5 but found 6 (indentation)
./Documentation/devicetree/bindings/usb/usb-uhci.yaml:62:5: [warning] wrong indentation: expected 3 but found 4 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-uhci.yaml: ignoring, error parsing file
./Documentation/devicetree/bindings/usb/usb-uhci.yaml:57:2: expected <block end>, but found '<block sequence start>'
make[2]: *** Deleting file 'Documentation/devicetree/bindings/usb/usb-uhci.example.dts'
Documentation/devicetree/bindings/usb/usb-uhci.yaml:57:2: expected <block end>, but found '<block sequence start>'
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/usb/usb-uhci.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1525: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250919025712.719246-4-ryan_chen@aspeedtech.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

[PATCH v3 4/4] usb: uhci: Add Aspeed AST2700 support

[Ryan Chen]

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@aspeedtech.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);
 	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

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

[Alan Stern]

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@aspeedtech.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
> 

RE: [PATCH v3 4/4] usb: uhci: Add Aspeed AST2700 support

[Ryan Chen]

> 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@aspeedtech.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?


I'll update the code to fall back to a 32-bit DMA mask in that case
dma_mask_ptr = of_device_get_match_data(&pdev->dev);
if (!dma_mask_ptr)
    dma_mask_ptr = &dma_mask_32;

ret = dma_coerce_mask_and_coherent(&pdev->dev, *dma_mask_ptr);


> 
> 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?

I've added .data = &dma_mask_32 for the existing generic-uhci 
and platform-uhci entries so they also work correctly with this logic.

 +	{ .compatible = "generic-uhci", .data = &dma_mask_32},
 +	{ .compatible = "platform-uhci", .data = &dma_mask_32},
 +	{ .compatible = "aspeed,ast2700-uhci", .data = &dma_mask_64},

> 
> 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
> >

RE: [PATCH v3 4/4] usb: uhci: Add Aspeed AST2700 support

[Ryan Chen]

> Subject: RE: [PATCH v3 4/4] usb: uhci: Add Aspeed AST2700 support
> 
> > 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@aspeedtech.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?
> 
> 
> I'll update the code to fall back to a 32-bit DMA mask in that case
> dma_mask_ptr = of_device_get_match_data(&pdev->dev);
> if (!dma_mask_ptr)
>     dma_mask_ptr = &dma_mask_32;
> 
> ret = dma_coerce_mask_and_coherent(&pdev->dev, *dma_mask_ptr);
> 
> 
> >
> > 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?
> 
> I've added .data = &dma_mask_32 for the existing generic-uhci and
> platform-uhci entries so they also work correctly with this logic.
> 
>  +	{ .compatible = "generic-uhci", .data = &dma_mask_32},
>  +	{ .compatible = "platform-uhci", .data = &dma_mask_32},
>  +	{ .compatible = "aspeed,ast2700-uhci", .data = &dma_mask_64},
> 
Sorry, I think I mis-understood your request.
I will update use 32-bit as default.
	dma_mask_ptr = (u64 *)of_device_get_match_data(&pdev->dev);
	if (!dma_mask_ptr)
	    dma_mask_ptr = &dma_mask_32; --> default

	ret = dma_coerce_mask_and_coherent(&pdev->dev, *dma_mask_ptr);
	if (ret)
		return ret;

And only add for aspeed,ast2700-uhci dma_mask_64 in platform_uhci_ids

Thanks a lot.
Ryan

> >
> > 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
> > >