[PATCH v1 0/2] USB: xhci: add support for PWRON polarity invert (TI TUSB73x0)

[PATCH v1 0/2] USB: xhci: add support for PWRON polarity invert (TI TUSB73x0)

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

The TUSB73x0 is a USB 3.0 xHCI Host Controller Hub using a PCIe x1 Gen2
interface. The TUSB7320 supports up to two downstream ports, and the 
TUSB7340 supports up to four. It supports to configure the polarity of
the PWRONx# signals, which are used to control other peripherals.
In some systems, the default polarity needs to be inverted, which is
supported by the given hardware through the software configuration. This
patch series introduces TUSB73x0 PCIe device tree bindings and modifies the 
USB XHCI PCI driver to handle PWRONx# polarity via a device tree property.

TUSB73x0 datasheet: https://www.ti.com/lit/ds/symlink/tusb7320.pdf

Parth Pancholi (2):
  dt-bindings: usb: add TUSB73x0 PCIe
  USB: xhci: add support for PWRON polarity invert

 .../bindings/usb/ti,tusb73x0-pci.yaml         | 60 +++++++++++++++++++
 drivers/usb/host/xhci-pci.c                   |  3 +
 2 files changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml

-- 
2.39.5

[PATCH v1 1/2] dt-bindings: usb: add TUSB73x0 PCIe

[Francesco Dolcini]

From: Parth Pancholi <parth.pancholi@toradex.com>

Add device tree bindings for TI's TUSB73x0 PCIe-to-USB 3.0 xHCI
host controller. The controller supports software configuration
through PCIe registers, such as controlling the PWRONx polarity
via the USB control register (E0h).

Similar generic PCIe-based bindings can be found as qcom,ath11k-pci.yaml
as an example.

Datasheet: https://www.ti.com/lit/ds/symlink/tusb7320.pdf
Signed-off-by: Parth Pancholi <parth.pancholi@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 .../bindings/usb/ti,tusb73x0-pci.yaml         | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml

diff --git a/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml b/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
new file mode 100644
index 000000000000..bcb619b08ad3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/ti,tusb73x0-pci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TUSB73x0 USB 3.0 xHCI Host Controller (PCIe)
+
+maintainers:
+  - Francesco Dolcini <francesco.dolcini@toradex.com>
+
+description:
+  TUSB73x0 USB 3.0 xHCI Host Controller via PCIe x1 Gen2 interface.
+  The TUSB7320 supports up to two downstream ports, the TUSB7340 supports up
+  to four downstream ports.
+
+properties:
+  compatible:
+    const: pci104C,8241
+
+  reg:
+    maxItems: 1
+
+  ti,tusb7320-pwron-polarity-invert:
+    type: boolean
+    description:
+      Configure the polarity of the PWRONx# signals. When this is false, the PWRONx#
+      pins are active low and their internal pull-down resistors are enabled.
+      When this is true, the PWRONx# pins are active high and their internal pull-down
+      resistors are disabled.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    pcie {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        pcie@0 {
+            device_type = "pci";
+            reg = <0x0 0x0 0x0 0x0 0x0>;
+            bus-range = <0x01 0xff>;
+
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+
+            usb@0 {
+                  compatible = "pci104C,8241";
+                  reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+                  ti,tusb7320-pwron-polarity-invert;
+            };
+        };
+    };
-- 
2.39.5

Re: [PATCH v1 1/2] dt-bindings: usb: add TUSB73x0 PCIe

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3193 bytes --]

On Fri, Oct 04, 2024 at 02:45:20PM +0200, Francesco Dolcini wrote:
> From: Parth Pancholi <parth.pancholi@toradex.com>
> 
> Add device tree bindings for TI's TUSB73x0 PCIe-to-USB 3.0 xHCI
> host controller. The controller supports software configuration
> through PCIe registers, such as controlling the PWRONx polarity
> via the USB control register (E0h).
> 
> Similar generic PCIe-based bindings can be found as qcom,ath11k-pci.yaml
> as an example.
> 
> Datasheet: https://www.ti.com/lit/ds/symlink/tusb7320.pdf
> Signed-off-by: Parth Pancholi <parth.pancholi@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>  .../bindings/usb/ti,tusb73x0-pci.yaml         | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml b/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> new file mode 100644
> index 000000000000..bcb619b08ad3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/ti,tusb73x0-pci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TUSB73x0 USB 3.0 xHCI Host Controller (PCIe)
> +
> +maintainers:
> +  - Francesco Dolcini <francesco.dolcini@toradex.com>
> +
> +description:
> +  TUSB73x0 USB 3.0 xHCI Host Controller via PCIe x1 Gen2 interface.
> +  The TUSB7320 supports up to two downstream ports, the TUSB7340 supports up
> +  to four downstream ports.
> +
> +properties:
> +  compatible:
> +    const: pci104C,8241
> +
> +  reg:
> +    maxItems: 1
> +
> +  ti,tusb7320-pwron-polarity-invert:

To me, "polarity-invert" makes less sense than calling this "active-high"
making the property a flag. active-low would then be the case where the
property is not provided. Given you don't make the property required,
what you've got here is effectively a flag anyway.

> +    type: boolean
> +    description:
> +      Configure the polarity of the PWRONx# signals. When this is false, the PWRONx#
> +      pins are active low and their internal pull-down resistors are enabled.
> +      When this is true, the PWRONx# pins are active high and their internal pull-down
> +      resistors are disabled.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pcie {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        pcie@0 {
> +            device_type = "pci";
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +            bus-range = <0x01 0xff>;
> +
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            ranges;
> +
> +            usb@0 {
> +                  compatible = "pci104C,8241";
> +                  reg = <0x10000 0x0 0x0 0x0 0x0>;
> +
> +                  ti,tusb7320-pwron-polarity-invert;
> +            };
> +        };
> +    };
> -- 
> 2.39.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/2] dt-bindings: usb: add TUSB73x0 PCIe

[Francesco Dolcini]

Hello Conor,

On Fri, Oct 04, 2024 at 04:23:18PM +0100, Conor Dooley wrote:
> On Fri, Oct 04, 2024 at 02:45:20PM +0200, Francesco Dolcini wrote:
> > From: Parth Pancholi <parth.pancholi@toradex.com>
> > 
> > Add device tree bindings for TI's TUSB73x0 PCIe-to-USB 3.0 xHCI
> > host controller. The controller supports software configuration
> > through PCIe registers, such as controlling the PWRONx polarity
> > via the USB control register (E0h).
> > 
> > Similar generic PCIe-based bindings can be found as qcom,ath11k-pci.yaml
> > as an example.
> > 
> > Datasheet: https://www.ti.com/lit/ds/symlink/tusb7320.pdf
> > Signed-off-by: Parth Pancholi <parth.pancholi@toradex.com>
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> >  .../bindings/usb/ti,tusb73x0-pci.yaml         | 60 +++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml b/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> > new file mode 100644
> > index 000000000000..bcb619b08ad3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/ti,tusb73x0-pci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TUSB73x0 USB 3.0 xHCI Host Controller (PCIe)
> > +
> > +maintainers:
> > +  - Francesco Dolcini <francesco.dolcini@toradex.com>
> > +
> > +description:
> > +  TUSB73x0 USB 3.0 xHCI Host Controller via PCIe x1 Gen2 interface.
> > +  The TUSB7320 supports up to two downstream ports, the TUSB7340 supports up
> > +  to four downstream ports.
> > +
> > +properties:
> > +  compatible:
> > +    const: pci104C,8241
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  ti,tusb7320-pwron-polarity-invert:
> 
> To me, "polarity-invert" makes less sense than calling this "active-high"
> making the property a flag. active-low would then be the case where the
> property is not provided. Given you don't make the property required,
> what you've got here is effectively a flag anyway.

We had the same doubt when deciding which property name to propose, looking
at the existing bindings it seemed that "polarity-invert" was more common.

FTR the datasheet explicetly name the signals with a # suffix (PWRON1#,
PWRON2#, ...), they are defined as active-low by default.

With that said, if we prefer to have `ti,tusb7320-pwron-active-high`, I am 100%
good with it.

Francesco

Re: [PATCH v1 1/2] dt-bindings: usb: add TUSB73x0 PCIe

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3288 bytes --]

On Fri, Oct 04, 2024 at 05:31:04PM +0200, Francesco Dolcini wrote:
> Hello Conor,
> 
> On Fri, Oct 04, 2024 at 04:23:18PM +0100, Conor Dooley wrote:
> > On Fri, Oct 04, 2024 at 02:45:20PM +0200, Francesco Dolcini wrote:
> > > From: Parth Pancholi <parth.pancholi@toradex.com>
> > > 
> > > Add device tree bindings for TI's TUSB73x0 PCIe-to-USB 3.0 xHCI
> > > host controller. The controller supports software configuration
> > > through PCIe registers, such as controlling the PWRONx polarity
> > > via the USB control register (E0h).
> > > 
> > > Similar generic PCIe-based bindings can be found as qcom,ath11k-pci.yaml
> > > as an example.
> > > 
> > > Datasheet: https://www.ti.com/lit/ds/symlink/tusb7320.pdf
> > > Signed-off-by: Parth Pancholi <parth.pancholi@toradex.com>
> > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > ---
> > >  .../bindings/usb/ti,tusb73x0-pci.yaml         | 60 +++++++++++++++++++
> > >  1 file changed, 60 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml b/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> > > new file mode 100644
> > > index 000000000000..bcb619b08ad3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/ti,tusb73x0-pci.yaml
> > > @@ -0,0 +1,60 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/usb/ti,tusb73x0-pci.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: TUSB73x0 USB 3.0 xHCI Host Controller (PCIe)
> > > +
> > > +maintainers:
> > > +  - Francesco Dolcini <francesco.dolcini@toradex.com>
> > > +
> > > +description:
> > > +  TUSB73x0 USB 3.0 xHCI Host Controller via PCIe x1 Gen2 interface.
> > > +  The TUSB7320 supports up to two downstream ports, the TUSB7340 supports up
> > > +  to four downstream ports.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: pci104C,8241
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  ti,tusb7320-pwron-polarity-invert:
> > 
> > To me, "polarity-invert" makes less sense than calling this "active-high"
> > making the property a flag. active-low would then be the case where the
> > property is not provided. Given you don't make the property required,
> > what you've got here is effectively a flag anyway.
> 
> We had the same doubt when deciding which property name to propose, looking
> at the existing bindings it seemed that "polarity-invert" was more common.
> 
> FTR the datasheet explicetly name the signals with a # suffix (PWRON1#,
> PWRON2#, ...), they are defined as active-low by default.
> 
> With that said, if we prefer to have `ti,tusb7320-pwron-active-high`, I am 100%
> good with it.

I think "active-high" is more explicit about what it does, but I'm not
too bothered about it. Given it isn't a required property and the
absence of the property means active-low (for backwards compatibility
and alignment with the default hardware behaviour) the property is a
flag, not a boolean, so it does need a type change at the very least.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]