* [PATCH 01/11] dt-bindings: usb: tegra-xudc: Add Tegra234 XUSB controller support
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-10-25 23:24 ` Rob Herring
2022-10-24 7:41 ` [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding Wayne Chang
` (9 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
Extend the Tegra XUSB controller device tree binding with Tegra234
support.
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
.../bindings/usb/nvidia,tegra-xudc.yaml | 24 ++++++++++++-------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
index fd6e7c81426e..517fb692f199 100644
--- a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
@@ -22,6 +22,7 @@ properties:
- nvidia,tegra210-xudc # For Tegra210
- nvidia,tegra186-xudc # For Tegra186
- nvidia,tegra194-xudc # For Tegra194
+ - nvidia,tegra234-xudc # For Tegra234
reg:
minItems: 2
@@ -90,21 +91,27 @@ properties:
phys:
minItems: 1
+ maxItems: 8
description:
Must contain an entry for each entry in phy-names.
See ../phy/phy-bindings.txt for details.
phy-names:
minItems: 1
+ maxItems: 8
items:
- - const: usb2-0
- - const: usb2-1
- - const: usb2-2
- - const: usb2-3
- - const: usb3-0
- - const: usb3-1
- - const: usb3-2
- - const: usb3-3
+ anyOf:
+ - const: usb2-0
+ - const: usb2-1
+ - const: usb2-2
+ - const: usb2-3
+ - const: usb3-0
+ - const: usb3-1
+ - const: usb3-2
+ - const: usb3-3
+
+ dma-coherent:
+ type: boolean
avddio-usb-supply:
description: PCIe/USB3 analog logic power supply. Must supply 1.05 V.
@@ -153,6 +160,7 @@ allOf:
enum:
- nvidia,tegra186-xudc
- nvidia,tegra194-xudc
+ - nvidia,tegra234-xudc
then:
properties:
reg:
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 01/11] dt-bindings: usb: tegra-xudc: Add Tegra234 XUSB controller support
2022-10-24 7:41 ` [PATCH 01/11] dt-bindings: usb: tegra-xudc: Add Tegra234 XUSB controller support Wayne Chang
@ 2022-10-25 23:24 ` Rob Herring
2022-11-03 10:36 ` Wayne Chang
0 siblings, 1 reply; 50+ messages in thread
From: Rob Herring @ 2022-10-25 23:24 UTC (permalink / raw)
To: Wayne Chang
Cc: gregkh, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On Mon, Oct 24, 2022 at 03:41:18PM +0800, Wayne Chang wrote:
> Extend the Tegra XUSB controller device tree binding with Tegra234
> support.
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> .../bindings/usb/nvidia,tegra-xudc.yaml | 24 ++++++++++++-------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
> index fd6e7c81426e..517fb692f199 100644
> --- a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
> @@ -22,6 +22,7 @@ properties:
> - nvidia,tegra210-xudc # For Tegra210
> - nvidia,tegra186-xudc # For Tegra186
> - nvidia,tegra194-xudc # For Tegra194
> + - nvidia,tegra234-xudc # For Tegra234
>
> reg:
> minItems: 2
> @@ -90,21 +91,27 @@ properties:
>
> phys:
> minItems: 1
> + maxItems: 8
> description:
> Must contain an entry for each entry in phy-names.
> See ../phy/phy-bindings.txt for details.
>
> phy-names:
> minItems: 1
> + maxItems: 8
> items:
> - - const: usb2-0
> - - const: usb2-1
> - - const: usb2-2
> - - const: usb2-3
> - - const: usb3-0
> - - const: usb3-1
> - - const: usb3-2
> - - const: usb3-3
> + anyOf:
> + - const: usb2-0
> + - const: usb2-1
> + - const: usb2-2
> + - const: usb2-3
> + - const: usb3-0
> + - const: usb3-1
> + - const: usb3-2
> + - const: usb3-3
items:
pattern: '^usb[23]-[0-3]$'
And an explanation why you need any random order. If it is just
different for Tegra234, then you need an if/then schema for this.
> +
> + dma-coherent:
> + type: boolean
>
> avddio-usb-supply:
> description: PCIe/USB3 analog logic power supply. Must supply 1.05 V.
> @@ -153,6 +160,7 @@ allOf:
> enum:
> - nvidia,tegra186-xudc
> - nvidia,tegra194-xudc
> + - nvidia,tegra234-xudc
> then:
> properties:
> reg:
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 01/11] dt-bindings: usb: tegra-xudc: Add Tegra234 XUSB controller support
2022-10-25 23:24 ` Rob Herring
@ 2022-11-03 10:36 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-11-03 10:36 UTC (permalink / raw)
To: Rob Herring
Cc: gregkh, krzysztof.kozlowski+dt, Thierry Reding, Jonathan Hunter,
thierry.reding, heikki.krogerus, Ajay Gupta, kishon, vkoul,
p.zabel, balbi, mathias.nyman, Jui Chang Kuo, linux-usb,
devicetree, linux-kernel, Sing-Han Chen, linux-i2c, linux-phy,
linux-tegra
On 10/26/22 07:24, Rob Herring wrote:
> External email: Use caution opening links or attachments
>
>
> On Mon, Oct 24, 2022 at 03:41:18PM +0800, Wayne Chang wrote:
>> Extend the Tegra XUSB controller device tree binding with Tegra234
>> support.
>>
>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>> ---
>> .../bindings/usb/nvidia,tegra-xudc.yaml | 24 ++++++++++++-------
>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
>> index fd6e7c81426e..517fb692f199 100644
>> --- a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
>> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.yaml
>> @@ -22,6 +22,7 @@ properties:
>> - nvidia,tegra210-xudc # For Tegra210
>> - nvidia,tegra186-xudc # For Tegra186
>> - nvidia,tegra194-xudc # For Tegra194
>> + - nvidia,tegra234-xudc # For Tegra234
>>
>> reg:
>> minItems: 2
>> @@ -90,21 +91,27 @@ properties:
>>
>> phys:
>> minItems: 1
>> + maxItems: 8
>> description:
>> Must contain an entry for each entry in phy-names.
>> See ../phy/phy-bindings.txt for details.
>>
>> phy-names:
>> minItems: 1
>> + maxItems: 8
>> items:
>> - - const: usb2-0
>> - - const: usb2-1
>> - - const: usb2-2
>> - - const: usb2-3
>> - - const: usb3-0
>> - - const: usb3-1
>> - - const: usb3-2
>> - - const: usb3-3
>> + anyOf:
>> + - const: usb2-0
>> + - const: usb2-1
>> + - const: usb2-2
>> + - const: usb2-3
>> + - const: usb3-0
>> + - const: usb3-1
>> + - const: usb3-2
>> + - const: usb3-3
>
> items:
> pattern: '^usb[23]-[0-3]$'
>
> And an explanation why you need any random order. If it is just
> different for Tegra234, then you need an if/then schema for this.
Thanks for the review.
We need to pick up one or more for the corresponding phys of the USB ports.
It should be a common settings among all the chips.
Adding anyOf here for the reason above and passing the dtb check.
Please let me know If I have any misunderstanding here.
thanks,
Wayne.
>
>> +
>> + dma-coherent:
>> + type: boolean
>>
>> avddio-usb-supply:
>> description: PCIe/USB3 analog logic power supply. Must supply 1.05 V.
>> @@ -153,6 +160,7 @@ allOf:
>> enum:
>> - nvidia,tegra186-xudc
>> - nvidia,tegra194-xudc
>> + - nvidia,tegra234-xudc
>> then:
>> properties:
>> reg:
>> --
>> 2.25.1
>>
>>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
2022-10-24 7:41 ` [PATCH 01/11] dt-bindings: usb: tegra-xudc: Add Tegra234 XUSB controller support Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-10-24 13:30 ` Rob Herring
2022-10-24 14:54 ` Rob Herring
2022-10-24 7:41 ` [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver Wayne Chang
` (8 subsequent siblings)
10 siblings, 2 replies; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
Add device-tree binding documentation for the XUSB host controller present
on Tegra194 and Tegra234 SoC. This controller supports the USB 3.1
specification.
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
.../bindings/usb/nvidia,tegra-xhci.yaml | 213 ++++++++++++++++++
1 file changed, 213 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml b/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
new file mode 100644
index 000000000000..d261a419a04f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
@@ -0,0 +1,213 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/usb/nvidia,tegra-xhci.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Device tree binding for NVIDIA Tegra XUSB host controller
+
+description:
+ The Tegra XHCI controller supports both USB 2.0 HighSpeed/FullSpeed and
+ USB 3.1 SuperSpeed protocols.
+
+maintainers:
+ - Wayne Chang <waynec@nvidia.com>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - nvidia,tegra194-xusb # For Tegra194
+ - nvidia,tegra234-xusb # For Tegra234
+
+ reg:
+ minItems: 2
+ items:
+ - description: XUSB host controller registers
+ - description: XUSB host PCI Config registers
+ - description: XUSB host bar2 registers
+
+ reg-names:
+ minItems: 2
+ items:
+ - const: hcd
+ - const: fpci
+ - const: bar2
+
+ interrupts:
+ items:
+ - description: Must contain the XUSB host interrupt.
+ - description: Must contain the XUSB mbox interrupt.
+
+ clocks:
+ items:
+ - description: Clock to enable core XUSB host clock.
+ - description: Clock to enable XUSB falcon clock.
+ - description: Clock to enable XUSB super speed clock.
+ - description: Clock to enable XUSB super speed dev clock.
+ - description: Clock to enable XUSB high speed dev clock.
+ - description: Clock to enable XUSB full speed dev clock.
+ - description: Clock to enable XUSB UTMI PLL clock.
+ - description: Clock to enable core XUSB dev clock.
+ - description: Clock to enable XUSB PLLE clock.
+
+ clock-names:
+ items:
+ - const: xusb_host
+ - const: xusb_falcon_src
+ - const: xusb_ss
+ - const: xusb_ss_src
+ - const: xusb_hs_src
+ - const: xusb_fs_src
+ - const: pll_u_480m
+ - const: clk_m
+ - const: pll_e
+
+ interconnects:
+ items:
+ - description: memory read client
+ - description: memory write client
+
+ interconnect-names:
+ items:
+ - const: dma-mem # read
+ - const: write
+
+ iommus:
+ maxItems: 1
+
+ power-domains:
+ items:
+ - description: XUSBC(host) power-domain
+ - description: XUSBA(superspeed) power-domain
+
+ power-domain-names:
+ items:
+ - const: xusb_host
+ - const: xusb_ss
+
+ nvidia,xusb-padctl:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ phandle to the XUSB pad controller that is used to configure the USB pads
+ used by the XUDC controller.
+
+ phys:
+ minItems: 1
+ maxItems: 8
+ description:
+ Must contain an entry for each entry in phy-names.
+ See ../phy/phy-bindings.txt for details.
+
+ phy-names:
+ minItems: 1
+ maxItems: 8
+ items:
+ anyOf:
+ - const: usb2-0
+ - const: usb2-1
+ - const: usb2-2
+ - const: usb2-3
+ - const: usb3-0
+ - const: usb3-1
+ - const: usb3-2
+ - const: usb3-3
+
+ dma-coherent:
+ type: boolean
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - clocks
+ - clock-names
+ - power-domains
+ - power-domain-names
+ - nvidia,xusb-padctl
+ - phys
+ - phy-names
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nvidia,tegra194-xusb
+ then:
+ properties:
+ reg:
+ minItems: 2
+ reg-names:
+ minItems: 2
+ clocks:
+ minItems: 9
+ clock-names:
+ minItems: 9
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nvidia,tegra234-xusb
+ then:
+ properties:
+ reg:
+ minItems: 3
+ reg-names:
+ minItems: 3
+ clocks:
+ minItems: 9
+ clock-names:
+ minItems: 9
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/tegra234-gpio.h>
+ #include <dt-bindings/clock/tegra234-clock.h>
+ #include <dt-bindings/memory/tegra234-mc.h>
+ #include <dt-bindings/power/tegra234-powergate.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ usb@3610000 {
+ compatible = "nvidia,tegra234-xusb";
+ reg = <0x03610000 0x40000>,
+ <0x03600000 0x10000>,
+ <0x03650000 0x10000>;
+ reg-names = "hcd", "fpci", "bar2";
+
+ interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
+
+ clocks = <&bpmp TEGRA234_CLK_XUSB_CORE_HOST>,
+ <&bpmp TEGRA234_CLK_XUSB_FALCON>,
+ <&bpmp TEGRA234_CLK_XUSB_CORE_SS>,
+ <&bpmp TEGRA234_CLK_XUSB_SS>,
+ <&bpmp TEGRA234_CLK_CLK_M>,
+ <&bpmp TEGRA234_CLK_XUSB_FS>,
+ <&bpmp TEGRA234_CLK_UTMIP_PLL>,
+ <&bpmp TEGRA234_CLK_CLK_M>,
+ <&bpmp TEGRA234_CLK_PLLE>;
+ clock-names = "xusb_host", "xusb_falcon_src",
+ "xusb_ss", "xusb_ss_src", "xusb_hs_src",
+ "xusb_fs_src", "pll_u_480m", "clk_m",
+ "pll_e";
+ interconnects = <&mc TEGRA234_MEMORY_CLIENT_XUSB_HOSTR &emc>,
+ <&mc TEGRA234_MEMORY_CLIENT_XUSB_HOSTW &emc>;
+ interconnect-names = "dma-mem", "write";
+ iommus = <&smmu_niso1 TEGRA234_SID_XUSB_HOST>;
+
+ power-domains = <&bpmp TEGRA234_POWER_DOMAIN_XUSBC>,
+ <&bpmp TEGRA234_POWER_DOMAIN_XUSBA>;
+ power-domain-names = "xusb_host", "xusb_ss";
+
+ nvidia,xusb-padctl = <&xusb_padctl>;
+
+ phys = <&pad_lanes_usb2_0>;
+ phy-names = "usb2-0";
+
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-24 7:41 ` [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding Wayne Chang
@ 2022-10-24 13:30 ` Rob Herring
2022-10-24 15:58 ` Jon Hunter
2022-10-24 14:54 ` Rob Herring
1 sibling, 1 reply; 50+ messages in thread
From: Rob Herring @ 2022-10-24 13:30 UTC (permalink / raw)
To: Wayne Chang
Cc: jonathanh, linux-phy, robh+dt, gregkh, kishon, vkoul,
linux-tegra, linux-kernel, singhanc, linux-i2c, mathias.nyman,
heikki.krogerus, linux-usb, thierry.reding, devicetree, treding,
krzysztof.kozlowski+dt, balbi, ajayg, jckuo, p.zabel
On Mon, 24 Oct 2022 15:41:19 +0800, Wayne Chang wrote:
> Add device-tree binding documentation for the XUSB host controller present
> on Tegra194 and Tegra234 SoC. This controller supports the USB 3.1
> specification.
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> .../bindings/usb/nvidia,tegra-xhci.yaml | 213 ++++++++++++++++++
> 1 file changed, 213 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.example.dts:36.27-28 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
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.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-24 13:30 ` Rob Herring
@ 2022-10-24 15:58 ` Jon Hunter
0 siblings, 0 replies; 50+ messages in thread
From: Jon Hunter @ 2022-10-24 15:58 UTC (permalink / raw)
To: Rob Herring, Wayne Chang
Cc: linux-phy, robh+dt, gregkh, kishon, vkoul, linux-tegra,
linux-kernel, singhanc, linux-i2c, mathias.nyman,
heikki.krogerus, linux-usb, thierry.reding, devicetree, treding,
krzysztof.kozlowski+dt, balbi, ajayg, jckuo, p.zabel
On 24/10/2022 14:30, Rob Herring wrote:
> On Mon, 24 Oct 2022 15:41:19 +0800, Wayne Chang wrote:
>> Add device-tree binding documentation for the XUSB host controller present
>> on Tegra194 and Tegra234 SoC. This controller supports the USB 3.1
>> specification.
>>
>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>> ---
>> .../bindings/usb/nvidia,tegra-xhci.yaml | 213 ++++++++++++++++++
>> 1 file changed, 213 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.example.dts:36.27-28 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
I believe that this is because another DT patch [0] is missing (see
patch 0/11). Thierry has just picked this up for -next and so hopefully
will resolve this.
Cheers
Jon
[0]
https://lore.kernel.org/all/20221003125141.123759-1-jonathanh@nvidia.com/
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-24 7:41 ` [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding Wayne Chang
2022-10-24 13:30 ` Rob Herring
@ 2022-10-24 14:54 ` Rob Herring
2022-10-25 8:02 ` Wayne Chang
1 sibling, 1 reply; 50+ messages in thread
From: Rob Herring @ 2022-10-24 14:54 UTC (permalink / raw)
To: Wayne Chang
Cc: gregkh, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On Mon, Oct 24, 2022 at 03:41:19PM +0800, Wayne Chang wrote:
> Add device-tree binding documentation for the XUSB host controller present
> on Tegra194 and Tegra234 SoC. This controller supports the USB 3.1
> specification.
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> .../bindings/usb/nvidia,tegra-xhci.yaml | 213 ++++++++++++++++++
> 1 file changed, 213 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
>
> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml b/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
> new file mode 100644
> index 000000000000..d261a419a04f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
> @@ -0,0 +1,213 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/usb/nvidia,tegra-xhci.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Device tree binding for NVIDIA Tegra XUSB host controller
Drop 'Device tree binding for '
> +
> +description:
> + The Tegra XHCI controller supports both USB 2.0 HighSpeed/FullSpeed and
> + USB 3.1 SuperSpeed protocols.
> +
> +maintainers:
> + - Wayne Chang <waynec@nvidia.com>
> +
Ref to usb-xhci.yaml? Or usb-hcd.yaml.
> +properties:
> + compatible:
> + items:
> + - enum:
> + - nvidia,tegra194-xusb # For Tegra194
> + - nvidia,tegra234-xusb # For Tegra234
The comment is kind of redundant.
> +
> + reg:
> + minItems: 2
> + items:
> + - description: XUSB host controller registers
> + - description: XUSB host PCI Config registers
> + - description: XUSB host bar2 registers
Drop 'XUSB host '
> +
> + reg-names:
> + minItems: 2
> + items:
> + - const: hcd
> + - const: fpci
> + - const: bar2
> +
> + interrupts:
> + items:
> + - description: Must contain the XUSB host interrupt.
> + - description: Must contain the XUSB mbox interrupt.
Drop 'Must contain the '
> +
> + clocks:
> + items:
> + - description: Clock to enable core XUSB host clock.
> + - description: Clock to enable XUSB falcon clock.
> + - description: Clock to enable XUSB super speed clock.
> + - description: Clock to enable XUSB super speed dev clock.
> + - description: Clock to enable XUSB high speed dev clock.
> + - description: Clock to enable XUSB full speed dev clock.
> + - description: Clock to enable XUSB UTMI PLL clock.
> + - description: Clock to enable core XUSB dev clock.
> + - description: Clock to enable XUSB PLLE clock.
Drop 'Clock to enable '
> +
> + clock-names:
> + items:
> + - const: xusb_host
> + - const: xusb_falcon_src
> + - const: xusb_ss
> + - const: xusb_ss_src
> + - const: xusb_hs_src
> + - const: xusb_fs_src
> + - const: pll_u_480m
> + - const: clk_m
> + - const: pll_e
> +
> + interconnects:
> + items:
> + - description: memory read client
> + - description: memory write client
> +
> + interconnect-names:
> + items:
> + - const: dma-mem # read
> + - const: write
> +
> + iommus:
> + maxItems: 1
> +
> + power-domains:
> + items:
> + - description: XUSBC(host) power-domain
> + - description: XUSBA(superspeed) power-domain
> +
> + power-domain-names:
> + items:
> + - const: xusb_host
> + - const: xusb_ss
Drop 'xusb_'.
> +
> + nvidia,xusb-padctl:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + phandle to the XUSB pad controller that is used to configure the USB pads
> + used by the XUDC controller.
> +
> + phys:
> + minItems: 1
> + maxItems: 8
> + description:
> + Must contain an entry for each entry in phy-names.
> + See ../phy/phy-bindings.txt for details.
Drop description.
> +
> + phy-names:
> + minItems: 1
> + maxItems: 8
> + items:
> + anyOf:
> + - const: usb2-0
> + - const: usb2-1
> + - const: usb2-2
> + - const: usb2-3
> + - const: usb3-0
> + - const: usb3-1
> + - const: usb3-2
> + - const: usb3-3
> +
> + dma-coherent:
> + type: boolean
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - clocks
> + - clock-names
> + - power-domains
> + - power-domain-names
> + - nvidia,xusb-padctl
> + - phys
> + - phy-names
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - nvidia,tegra194-xusb
> + then:
> + properties:
> + reg:
> + minItems: 2
> + reg-names:
> + minItems: 2
> + clocks:
> + minItems: 9
> + clock-names:
> + minItems: 9
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - nvidia,tegra234-xusb
> + then:
> + properties:
> + reg:
> + minItems: 3
> + reg-names:
> + minItems: 3
> + clocks:
> + minItems: 9
> + clock-names:
> + minItems: 9
Same number of items, why are clocks in the if/then?
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/tegra234-gpio.h>
> + #include <dt-bindings/clock/tegra234-clock.h>
> + #include <dt-bindings/memory/tegra234-mc.h>
> + #include <dt-bindings/power/tegra234-powergate.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + usb@3610000 {
> + compatible = "nvidia,tegra234-xusb";
> + reg = <0x03610000 0x40000>,
> + <0x03600000 0x10000>,
> + <0x03650000 0x10000>;
> + reg-names = "hcd", "fpci", "bar2";
> +
> + interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> +
> + clocks = <&bpmp TEGRA234_CLK_XUSB_CORE_HOST>,
> + <&bpmp TEGRA234_CLK_XUSB_FALCON>,
> + <&bpmp TEGRA234_CLK_XUSB_CORE_SS>,
> + <&bpmp TEGRA234_CLK_XUSB_SS>,
> + <&bpmp TEGRA234_CLK_CLK_M>,
> + <&bpmp TEGRA234_CLK_XUSB_FS>,
> + <&bpmp TEGRA234_CLK_UTMIP_PLL>,
> + <&bpmp TEGRA234_CLK_CLK_M>,
> + <&bpmp TEGRA234_CLK_PLLE>;
> + clock-names = "xusb_host", "xusb_falcon_src",
> + "xusb_ss", "xusb_ss_src", "xusb_hs_src",
> + "xusb_fs_src", "pll_u_480m", "clk_m",
> + "pll_e";
> + interconnects = <&mc TEGRA234_MEMORY_CLIENT_XUSB_HOSTR &emc>,
> + <&mc TEGRA234_MEMORY_CLIENT_XUSB_HOSTW &emc>;
> + interconnect-names = "dma-mem", "write";
> + iommus = <&smmu_niso1 TEGRA234_SID_XUSB_HOST>;
> +
> + power-domains = <&bpmp TEGRA234_POWER_DOMAIN_XUSBC>,
> + <&bpmp TEGRA234_POWER_DOMAIN_XUSBA>;
> + power-domain-names = "xusb_host", "xusb_ss";
> +
> + nvidia,xusb-padctl = <&xusb_padctl>;
> +
> + phys = <&pad_lanes_usb2_0>;
> + phy-names = "usb2-0";
> +
> + };
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-24 14:54 ` Rob Herring
@ 2022-10-25 8:02 ` Wayne Chang
2022-10-28 2:19 ` Krzysztof Kozlowski
0 siblings, 1 reply; 50+ messages in thread
From: Wayne Chang @ 2022-10-25 8:02 UTC (permalink / raw)
To: Rob Herring
Cc: gregkh, krzysztof.kozlowski+dt, Thierry Reding, Jonathan Hunter,
thierry.reding, heikki.krogerus, Ajay Gupta, kishon, vkoul,
p.zabel, balbi, mathias.nyman, Jui Chang Kuo, linux-usb,
devicetree, linux-kernel, Sing-Han Chen, linux-i2c, linux-phy,
linux-tegra
Thanks for the review.
All the changes will be updated in the next patchset except the
power-domain-name.
On 10/24/22 22:54, Rob Herring wrote:
> External email: Use caution opening links or attachments
>
>
>> +description:
>> + The Tegra XHCI controller supports both USB 2.0 HighSpeed/FullSpeed and
>> + USB 3.1 SuperSpeed protocols.
>> +
>> +maintainers:
>> + - Wayne Chang <waynec@nvidia.com>
>> +
>
> Ref to usb-xhci.yaml? Or usb-hcd.yaml.
>
It should be usb-xhci.yaml. thanks.
>> + power-domain-names:
>> + items:
>> + - const: xusb_host
>> + - const: xusb_ss
>
> Drop 'xusb_'.
The properties are constant and we use the name to get the power domain.
tegra->genpd_dev_host = dev_pm_domain_attach_by_name(dev, "xusb_host");
tegra->genpd_dev_ss = dev_pm_domain_attach_by_name(dev, "xusb_ss");
we might not be able to drop the xusb_
thanks,
Wayne.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-25 8:02 ` Wayne Chang
@ 2022-10-28 2:19 ` Krzysztof Kozlowski
2022-10-28 9:25 ` Jon Hunter
0 siblings, 1 reply; 50+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-28 2:19 UTC (permalink / raw)
To: Wayne Chang, Rob Herring
Cc: gregkh, krzysztof.kozlowski+dt, Thierry Reding, Jonathan Hunter,
thierry.reding, heikki.krogerus, Ajay Gupta, kishon, vkoul,
p.zabel, balbi, mathias.nyman, Jui Chang Kuo, linux-usb,
devicetree, linux-kernel, Sing-Han Chen, linux-i2c, linux-phy,
linux-tegra
On 25/10/2022 04:02, Wayne Chang wrote:
>
>>> + power-domain-names:
>>> + items:
>>> + - const: xusb_host
>>> + - const: xusb_ss
>>
>> Drop 'xusb_'.
>
> The properties are constant and we use the name to get the power domain.
>
> tegra->genpd_dev_host = dev_pm_domain_attach_by_name(dev, "xusb_host");
>
> tegra->genpd_dev_ss = dev_pm_domain_attach_by_name(dev, "xusb_ss");
>
> we might not be able to drop the xusb_
These are new bindings, so why do say they are "constant"? New bindings
means you did not use them. If you used them before bindings... what can
we say? Don't?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-28 2:19 ` Krzysztof Kozlowski
@ 2022-10-28 9:25 ` Jon Hunter
2022-10-28 11:07 ` Jon Hunter
0 siblings, 1 reply; 50+ messages in thread
From: Jon Hunter @ 2022-10-28 9:25 UTC (permalink / raw)
To: Krzysztof Kozlowski, Wayne Chang, Rob Herring
Cc: gregkh, krzysztof.kozlowski+dt, Thierry Reding, thierry.reding,
heikki.krogerus, Ajay Gupta, kishon, vkoul, p.zabel, balbi,
mathias.nyman, Jui Chang Kuo, linux-usb, devicetree,
linux-kernel, Sing-Han Chen, linux-i2c, linux-phy, linux-tegra
On 28/10/2022 03:19, Krzysztof Kozlowski wrote:
> On 25/10/2022 04:02, Wayne Chang wrote:
>>
>>>> + power-domain-names:
>>>> + items:
>>>> + - const: xusb_host
>>>> + - const: xusb_ss
>>>
>>> Drop 'xusb_'.
>>
>> The properties are constant and we use the name to get the power domain.
>>
>> tegra->genpd_dev_host = dev_pm_domain_attach_by_name(dev, "xusb_host");
>>
>> tegra->genpd_dev_ss = dev_pm_domain_attach_by_name(dev, "xusb_ss");
>>
>> we might not be able to drop the xusb_
>
> These are new bindings, so why do say they are "constant"? New bindings
> means you did not use them. If you used them before bindings... what can
> we say? Don't?
Not exactly. However, what we should do here is convert the legacy
binding doc [0] and replace with this one. But yes we are stuck with the
'xusb_host' naming.
Jon
[0] Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-28 9:25 ` Jon Hunter
@ 2022-10-28 11:07 ` Jon Hunter
2022-10-28 11:30 ` Thierry Reding
0 siblings, 1 reply; 50+ messages in thread
From: Jon Hunter @ 2022-10-28 11:07 UTC (permalink / raw)
To: Krzysztof Kozlowski, Wayne Chang, Rob Herring
Cc: gregkh, krzysztof.kozlowski+dt, Thierry Reding, thierry.reding,
heikki.krogerus, Ajay Gupta, kishon, vkoul, p.zabel, balbi,
mathias.nyman, Jui Chang Kuo, linux-usb, devicetree,
linux-kernel, Sing-Han Chen, linux-i2c, linux-phy, linux-tegra
On 28/10/2022 10:25, Jon Hunter wrote:
>
> On 28/10/2022 03:19, Krzysztof Kozlowski wrote:
>> On 25/10/2022 04:02, Wayne Chang wrote:
>>>
>>>>> + power-domain-names:
>>>>> + items:
>>>>> + - const: xusb_host
>>>>> + - const: xusb_ss
>>>>
>>>> Drop 'xusb_'.
>>>
>>> The properties are constant and we use the name to get the power domain.
>>>
>>> tegra->genpd_dev_host = dev_pm_domain_attach_by_name(dev,
>>> "xusb_host");
>>>
>>> tegra->genpd_dev_ss = dev_pm_domain_attach_by_name(dev, "xusb_ss");
>>>
>>> we might not be able to drop the xusb_
>>
>> These are new bindings, so why do say they are "constant"? New bindings
>> means you did not use them. If you used them before bindings... what can
>> we say? Don't?
>
> Not exactly. However, what we should do here is convert the legacy
> binding doc [0] and replace with this one. But yes we are stuck with the
> 'xusb_host' naming.
Thierry already has a patch to do this [0]. So we should fix that up and
included in this series.
Jon
[0]
https://lore.kernel.org/linux-tegra/20211209165339.614498-3-thierry.reding@gmail.com/
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-28 11:07 ` Jon Hunter
@ 2022-10-28 11:30 ` Thierry Reding
2022-11-03 10:24 ` Wayne Chang
0 siblings, 1 reply; 50+ messages in thread
From: Thierry Reding @ 2022-10-28 11:30 UTC (permalink / raw)
To: Jon Hunter
Cc: Krzysztof Kozlowski, Wayne Chang, Rob Herring, gregkh,
krzysztof.kozlowski+dt, Thierry Reding, heikki.krogerus,
Ajay Gupta, kishon, vkoul, p.zabel, balbi, mathias.nyman,
Jui Chang Kuo, linux-usb, devicetree, linux-kernel,
Sing-Han Chen, linux-i2c, linux-phy, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]
On Fri, Oct 28, 2022 at 12:07:38PM +0100, Jon Hunter wrote:
>
> On 28/10/2022 10:25, Jon Hunter wrote:
> >
> > On 28/10/2022 03:19, Krzysztof Kozlowski wrote:
> > > On 25/10/2022 04:02, Wayne Chang wrote:
> > > >
> > > > > > + power-domain-names:
> > > > > > + items:
> > > > > > + - const: xusb_host
> > > > > > + - const: xusb_ss
> > > > >
> > > > > Drop 'xusb_'.
> > > >
> > > > The properties are constant and we use the name to get the power domain.
> > > >
> > > > tegra->genpd_dev_host = dev_pm_domain_attach_by_name(dev,
> > > > "xusb_host");
> > > >
> > > > tegra->genpd_dev_ss = dev_pm_domain_attach_by_name(dev, "xusb_ss");
> > > >
> > > > we might not be able to drop the xusb_
> > >
> > > These are new bindings, so why do say they are "constant"? New bindings
> > > means you did not use them. If you used them before bindings... what can
> > > we say? Don't?
> >
> > Not exactly. However, what we should do here is convert the legacy
> > binding doc [0] and replace with this one. But yes we are stuck with the
> > 'xusb_host' naming.
>
>
> Thierry already has a patch to do this [0]. So we should fix that up and
> included in this series.
>
> Jon
>
> [0] https://lore.kernel.org/linux-tegra/20211209165339.614498-3-thierry.reding@gmail.com/
I have a v2 with at least some of the comments addressed. I'll go
through them again and send it out. If we can get that reviewed, it can
be included in this series and the Tegra234 addition be applied on top.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding
2022-10-28 11:30 ` Thierry Reding
@ 2022-11-03 10:24 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-11-03 10:24 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter
Cc: Krzysztof Kozlowski, Rob Herring, gregkh, krzysztof.kozlowski+dt,
Thierry Reding, heikki.krogerus, Ajay Gupta, kishon, vkoul,
p.zabel, balbi, mathias.nyman, Jui Chang Kuo, linux-usb,
devicetree, linux-kernel, Sing-Han Chen, linux-i2c, linux-phy,
linux-tegra
On 10/28/22 19:30, Thierry Reding wrote:
> On Fri, Oct 28, 2022 at 12:07:38PM +0100, Jon Hunter wrote:
>> On 28/10/2022 10:25, Jon Hunter wrote:
>>> On 28/10/2022 03:19, Krzysztof Kozlowski wrote:
>>>> On 25/10/2022 04:02, Wayne Chang wrote:
>>>>>>> + power-domain-names:
>>>>>>> + items:
>>>>>>> + - const: xusb_host
>>>>>>> + - const: xusb_ss
>>>>>> Drop 'xusb_'.
>>>>> The properties are constant and we use the name to get the power domain.
>>>>>
>>>>> tegra->genpd_dev_host = dev_pm_domain_attach_by_name(dev,
>>>>> "xusb_host");
>>>>>
>>>>> tegra->genpd_dev_ss = dev_pm_domain_attach_by_name(dev, "xusb_ss");
>>>>>
>>>>> we might not be able to drop the xusb_
>>>> These are new bindings, so why do say they are "constant"? New bindings
>>>> means you did not use them. If you used them before bindings... what can
>>>> we say? Don't?
>>> Not exactly. However, what we should do here is convert the legacy
>>> binding doc [0] and replace with this one. But yes we are stuck with the
>>> 'xusb_host' naming.
>>
>> Thierry already has a patch to do this [0]. So we should fix that up and
>> included in this series.
>>
>> Jon
>>
>> [0]https://lore.kernel.org/linux-tegra/20211209165339.614498-3-thierry.reding@gmail.com/
> I have a v2 with at least some of the comments addressed. I'll go
> through them again and send it out. If we can get that reviewed, it can
> be included in this series and the Tegra234 addition be applied on top.
Thanks for the help, Thierry. I'll update the binding after your change
got updated.
>
> Thierry
thanks,
Wayne.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
2022-10-24 7:41 ` [PATCH 01/11] dt-bindings: usb: tegra-xudc: Add Tegra234 XUSB controller support Wayne Chang
2022-10-24 7:41 ` [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host controller binding Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-10-26 1:07 ` Rob Herring
2022-10-26 7:13 ` Jon Hunter
2022-10-24 7:41 ` [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin Wayne Chang
` (7 subsequent siblings)
10 siblings, 2 replies; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
add device-tree binding documentation for Cypress cypd4226 type-C
controller's I2C interface. It is a standard i2c slave with GPIO
input as IRQ interface.
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
.../bindings/usb/cypress,cypd4226.yaml | 86 +++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
new file mode 100644
index 000000000000..5ac28ab4e7a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/cypress,cypd4226.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cypress cypd4226 UCSI I2C Type-C Controller
+
+maintainers:
+ - Wayne Chang <waynec@nvidia.com>
+
+description: |
+ The Cypress cypd4226 UCSI I2C type-C controller is a I2C interface type-C
+ controller.
+
+properties:
+ compatible:
+ const: cypress,cypd4226
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ reg:
+ const: 0x08
+
+ interrupts:
+ maxItems: 1
+
+ cypress,firmware-build:
+ enum:
+ - nv
+ - gn
+ description: |
+ the name of the CCGx firmware built for product series.
+ should be set one of following:
+ - "nv" for the RTX product series
+ - "gn" for the Jetson product series
+
+patternProperties:
+ '^connector@[0-9a-f]+$':
+ $ref: /schemas/connector/usb-connector.yaml#
+ properties:
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: true
+
+examples:
+ - |
+ #include <dt-bindings/gpio/tegra194-gpio.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #interrupt-cells = <2>;
+
+ ucsi_ccg: ucsi_ccg@8 {
+ compatible = "cypress,cypd4226";
+ interrupt-parent = <&gpio_aon>;
+ interrupts = <TEGRA194_AON_GPIO(BB, 2) IRQ_TYPE_LEVEL_LOW>;
+ reg = <0x08>;
+ cypress,firmware-build = "gn";
+ status = "okay";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ccg_typec_con0: connector@0 {
+ compatible = "usb-c-connector";
+ reg = <0>;
+ label = "USB-C";
+ data-role = "dual";
+ port {
+ ucsi_ccg_p0: endpoint {
+ remote-endpoint = <&usb_role_switch0>;
+ };
+ };
+ };
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver
2022-10-24 7:41 ` [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver Wayne Chang
@ 2022-10-26 1:07 ` Rob Herring
2022-10-26 7:13 ` Jon Hunter
1 sibling, 0 replies; 50+ messages in thread
From: Rob Herring @ 2022-10-26 1:07 UTC (permalink / raw)
To: Wayne Chang
Cc: gregkh, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On Mon, Oct 24, 2022 at 03:41:20PM +0800, Wayne Chang wrote:
> add device-tree binding documentation for Cypress cypd4226 type-C
> controller's I2C interface. It is a standard i2c slave with GPIO
> input as IRQ interface.
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> .../bindings/usb/cypress,cypd4226.yaml | 86 +++++++++++++++++++
> 1 file changed, 86 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
>
> diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> new file mode 100644
> index 000000000000..5ac28ab4e7a1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/cypress,cypd4226.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cypress cypd4226 UCSI I2C Type-C Controller
> +
> +maintainers:
> + - Wayne Chang <waynec@nvidia.com>
> +
> +description: |
Don't need '|'.
> + The Cypress cypd4226 UCSI I2C type-C controller is a I2C interface type-C
> + controller.
> +
> +properties:
> + compatible:
> + const: cypress,cypd4226
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + reg:
> + const: 0x08
> +
> + interrupts:
> + maxItems: 1
> +
> + cypress,firmware-build:
> + enum:
> + - nv
> + - gn
> + description: |
> + the name of the CCGx firmware built for product series.
> + should be set one of following:
> + - "nv" for the RTX product series
> + - "gn" for the Jetson product series
> +
> +patternProperties:
> + '^connector@[0-9a-f]+$':
Looks like the part only has 2 PD controllers, so 2 connectors only,
right?
> + $ref: /schemas/connector/usb-connector.yaml#
unevaluatedProperties: false
> + properties:
> + reg:
> + maxItems: 1
maximum: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +additionalProperties: true
false
true is only for incomplete, common schemas.
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/tegra194-gpio.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #interrupt-cells = <2>;
> +
> + ucsi_ccg: ucsi_ccg@8 {
> + compatible = "cypress,cypd4226";
> + interrupt-parent = <&gpio_aon>;
> + interrupts = <TEGRA194_AON_GPIO(BB, 2) IRQ_TYPE_LEVEL_LOW>;
> + reg = <0x08>;
> + cypress,firmware-build = "gn";
> + status = "okay";
Don't need status in examples.
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ccg_typec_con0: connector@0 {
> + compatible = "usb-c-connector";
> + reg = <0>;
> + label = "USB-C";
> + data-role = "dual";
> + port {
> + ucsi_ccg_p0: endpoint {
> + remote-endpoint = <&usb_role_switch0>;
> + };
> + };
> + };
> + };
> + };
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver
2022-10-24 7:41 ` [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver Wayne Chang
2022-10-26 1:07 ` Rob Herring
@ 2022-10-26 7:13 ` Jon Hunter
2022-10-28 12:31 ` Thierry Reding
1 sibling, 1 reply; 50+ messages in thread
From: Jon Hunter @ 2022-10-26 7:13 UTC (permalink / raw)
To: Wayne Chang, gregkh, robh+dt, krzysztof.kozlowski+dt, treding,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
On 24/10/2022 08:41, Wayne Chang wrote:
> add device-tree binding documentation for Cypress cypd4226 type-C
> controller's I2C interface. It is a standard i2c slave with GPIO
> input as IRQ interface.
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> .../bindings/usb/cypress,cypd4226.yaml | 86 +++++++++++++++++++
> 1 file changed, 86 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
>
> diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> new file mode 100644
> index 000000000000..5ac28ab4e7a1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/cypress,cypd4226.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cypress cypd4226 UCSI I2C Type-C Controller
> +
> +maintainers:
> + - Wayne Chang <waynec@nvidia.com>
> +
> +description: |
> + The Cypress cypd4226 UCSI I2C type-C controller is a I2C interface type-C
> + controller.
> +
> +properties:
> + compatible:
> + const: cypress,cypd4226
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + reg:
> + const: 0x08
> +
> + interrupts:
> + maxItems: 1
> +
> + cypress,firmware-build:
> + enum:
> + - nv
> + - gn
> + description: |
> + the name of the CCGx firmware built for product series.
> + should be set one of following:
> + - "nv" for the RTX product series
Please add 'NVIDIA' so that it is 'for the NVIDIA RTX product series'
> + - "gn" for the Jetson product series
Same here please add 'NVIDIA' so that it is 'for the NVIDIA Jetson
product series'.
Rob, any concerns about this property in general? Unfortunately, ACPI
choose a 16-bit type for this and used 'nv' for the RTX product. I don't
find 'gn' for Jetson very descriptive but we need a way to differentiate
from RTX.
This is needed in the Cypress CCGX driver for the following ...
https://lore.kernel.org/lkml/20220928150840.3804313-1-waynec@nvidia.com/
Ideally, this should have been included in this series but was sent
before. We can always re-work/update the above patch even though it has
been queued up now.
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver
2022-10-26 7:13 ` Jon Hunter
@ 2022-10-28 12:31 ` Thierry Reding
2022-10-28 12:42 ` Jon Hunter
0 siblings, 1 reply; 50+ messages in thread
From: Thierry Reding @ 2022-10-28 12:31 UTC (permalink / raw)
To: Jon Hunter, Rob Herring, Krzysztof Kozlowski
Cc: Wayne Chang, gregkh, treding, heikki.krogerus, ajayg, kishon,
vkoul, p.zabel, balbi, mathias.nyman, jckuo, linux-usb,
devicetree, linux-kernel, singhanc, linux-i2c, linux-phy,
linux-tegra
[-- Attachment #1: Type: text/plain, Size: 3898 bytes --]
On Wed, Oct 26, 2022 at 08:13:57AM +0100, Jon Hunter wrote:
>
> On 24/10/2022 08:41, Wayne Chang wrote:
> > add device-tree binding documentation for Cypress cypd4226 type-C
> > controller's I2C interface. It is a standard i2c slave with GPIO
> > input as IRQ interface.
> >
> > Signed-off-by: Wayne Chang <waynec@nvidia.com>
> > ---
> > .../bindings/usb/cypress,cypd4226.yaml | 86 +++++++++++++++++++
> > 1 file changed, 86 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> > new file mode 100644
> > index 000000000000..5ac28ab4e7a1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> > @@ -0,0 +1,86 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/cypress,cypd4226.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Cypress cypd4226 UCSI I2C Type-C Controller
> > +
> > +maintainers:
> > + - Wayne Chang <waynec@nvidia.com>
> > +
> > +description: |
> > + The Cypress cypd4226 UCSI I2C type-C controller is a I2C interface type-C
> > + controller.
> > +
> > +properties:
> > + compatible:
> > + const: cypress,cypd4226
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + reg:
> > + const: 0x08
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + cypress,firmware-build:
> > + enum:
> > + - nv
> > + - gn
> > + description: |
> > + the name of the CCGx firmware built for product series.
> > + should be set one of following:
> > + - "nv" for the RTX product series
>
> Please add 'NVIDIA' so that it is 'for the NVIDIA RTX product series'
>
> > + - "gn" for the Jetson product series
>
> Same here please add 'NVIDIA' so that it is 'for the NVIDIA Jetson product
> series'.
>
> Rob, any concerns about this property in general? Unfortunately, ACPI choose
> a 16-bit type for this and used 'nv' for the RTX product. I don't find 'gn'
> for Jetson very descriptive but we need a way to differentiate from RTX.
>
> This is needed in the Cypress CCGX driver for the following ...
>
> https://lore.kernel.org/lkml/20220928150840.3804313-1-waynec@nvidia.com/
>
> Ideally, this should have been included in this series but was sent before.
> We can always re-work/update the above patch even though it has been queued
> up now.
The driver seems to use this 16-bit value only to compare with a
corresponding field in the firmware headers. How exactly we obtain this
value is therefore not important. However, since this 16-bit value is
embedded in firmware images, we also cannot substitute them with
something more sensible.
However, I'm also a little confused as to the meaning of the property.
Looking at the driver, the fw_build field is used to check for
"supported vendors". "nv" and "gn" are clearly the same vendor (NVIDIA),
so that's at least not 100% accurate. The DT bindings say that this
denotes the product series, which seems to be more in line with how the
driver uses it.
The driver also uses it to implement changes in behavior across
different variants, which is something that we would typically describe
using compatible strings. So I wonder if we should, at least for device
tree, switch to using different compatible strings rather than this
separate matching mechanism. We could then associate a "product series"
with the compatible string rather than having this extra field.
There's also an argument to be made for keeping the interface the same
between ACPI and DT.
Rob, Krzysztof?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver
2022-10-28 12:31 ` Thierry Reding
@ 2022-10-28 12:42 ` Jon Hunter
2022-10-28 14:07 ` Thierry Reding
0 siblings, 1 reply; 50+ messages in thread
From: Jon Hunter @ 2022-10-28 12:42 UTC (permalink / raw)
To: Thierry Reding, Rob Herring, Krzysztof Kozlowski
Cc: Wayne Chang, gregkh, treding, heikki.krogerus, ajayg, kishon,
vkoul, p.zabel, balbi, mathias.nyman, jckuo, linux-usb,
devicetree, linux-kernel, singhanc, linux-i2c, linux-phy,
linux-tegra
On 28/10/2022 13:31, Thierry Reding wrote:
> On Wed, Oct 26, 2022 at 08:13:57AM +0100, Jon Hunter wrote:
>>
>> On 24/10/2022 08:41, Wayne Chang wrote:
>>> add device-tree binding documentation for Cypress cypd4226 type-C
>>> controller's I2C interface. It is a standard i2c slave with GPIO
>>> input as IRQ interface.
>>>
>>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>>> ---
>>> .../bindings/usb/cypress,cypd4226.yaml | 86 +++++++++++++++++++
>>> 1 file changed, 86 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
>>> new file mode 100644
>>> index 000000000000..5ac28ab4e7a1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
>>> @@ -0,0 +1,86 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/usb/cypress,cypd4226.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Cypress cypd4226 UCSI I2C Type-C Controller
>>> +
>>> +maintainers:
>>> + - Wayne Chang <waynec@nvidia.com>
>>> +
>>> +description: |
>>> + The Cypress cypd4226 UCSI I2C type-C controller is a I2C interface type-C
>>> + controller.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: cypress,cypd4226
>>> +
>>> + '#address-cells':
>>> + const: 1
>>> +
>>> + '#size-cells':
>>> + const: 0
>>> +
>>> + reg:
>>> + const: 0x08
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + cypress,firmware-build:
>>> + enum:
>>> + - nv
>>> + - gn
>>> + description: |
>>> + the name of the CCGx firmware built for product series.
>>> + should be set one of following:
>>> + - "nv" for the RTX product series
>>
>> Please add 'NVIDIA' so that it is 'for the NVIDIA RTX product series'
>>
>>> + - "gn" for the Jetson product series
>>
>> Same here please add 'NVIDIA' so that it is 'for the NVIDIA Jetson product
>> series'.
>>
>> Rob, any concerns about this property in general? Unfortunately, ACPI choose
>> a 16-bit type for this and used 'nv' for the RTX product. I don't find 'gn'
>> for Jetson very descriptive but we need a way to differentiate from RTX.
>>
>> This is needed in the Cypress CCGX driver for the following ...
>>
>> https://lore.kernel.org/lkml/20220928150840.3804313-1-waynec@nvidia.com/
>>
>> Ideally, this should have been included in this series but was sent before.
>> We can always re-work/update the above patch even though it has been queued
>> up now.
>
> The driver seems to use this 16-bit value only to compare with a
> corresponding field in the firmware headers. How exactly we obtain this
> value is therefore not important. However, since this 16-bit value is
> embedded in firmware images, we also cannot substitute them with
> something more sensible.
I am actually wondering if this is actually embedded in any images
because I see it populated by the i2c-nvidia-gpu.c driver [0]. So I am
wondering if we can use PROPERTY_ENTRY_STRING() for this driver instead
and have a more descriptive name such as 'nvidia,rtx'?
Jon
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-nvidia-gpu.c#n261
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver
2022-10-28 12:42 ` Jon Hunter
@ 2022-10-28 14:07 ` Thierry Reding
2022-11-03 10:47 ` Wayne Chang
0 siblings, 1 reply; 50+ messages in thread
From: Thierry Reding @ 2022-10-28 14:07 UTC (permalink / raw)
To: Jon Hunter
Cc: Rob Herring, Krzysztof Kozlowski, Wayne Chang, gregkh, treding,
heikki.krogerus, ajayg, kishon, vkoul, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 5498 bytes --]
On Fri, Oct 28, 2022 at 01:42:36PM +0100, Jon Hunter wrote:
>
> On 28/10/2022 13:31, Thierry Reding wrote:
> > On Wed, Oct 26, 2022 at 08:13:57AM +0100, Jon Hunter wrote:
> > >
> > > On 24/10/2022 08:41, Wayne Chang wrote:
> > > > add device-tree binding documentation for Cypress cypd4226 type-C
> > > > controller's I2C interface. It is a standard i2c slave with GPIO
> > > > input as IRQ interface.
> > > >
> > > > Signed-off-by: Wayne Chang <waynec@nvidia.com>
> > > > ---
> > > > .../bindings/usb/cypress,cypd4226.yaml | 86 +++++++++++++++++++
> > > > 1 file changed, 86 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> > > > new file mode 100644
> > > > index 000000000000..5ac28ab4e7a1
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
> > > > @@ -0,0 +1,86 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/usb/cypress,cypd4226.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Cypress cypd4226 UCSI I2C Type-C Controller
> > > > +
> > > > +maintainers:
> > > > + - Wayne Chang <waynec@nvidia.com>
> > > > +
> > > > +description: |
> > > > + The Cypress cypd4226 UCSI I2C type-C controller is a I2C interface type-C
> > > > + controller.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: cypress,cypd4226
> > > > +
> > > > + '#address-cells':
> > > > + const: 1
> > > > +
> > > > + '#size-cells':
> > > > + const: 0
> > > > +
> > > > + reg:
> > > > + const: 0x08
> > > > +
> > > > + interrupts:
> > > > + maxItems: 1
> > > > +
> > > > + cypress,firmware-build:
> > > > + enum:
> > > > + - nv
> > > > + - gn
> > > > + description: |
> > > > + the name of the CCGx firmware built for product series.
> > > > + should be set one of following:
> > > > + - "nv" for the RTX product series
> > >
> > > Please add 'NVIDIA' so that it is 'for the NVIDIA RTX product series'
> > >
> > > > + - "gn" for the Jetson product series
> > >
> > > Same here please add 'NVIDIA' so that it is 'for the NVIDIA Jetson product
> > > series'.
> > >
> > > Rob, any concerns about this property in general? Unfortunately, ACPI choose
> > > a 16-bit type for this and used 'nv' for the RTX product. I don't find 'gn'
> > > for Jetson very descriptive but we need a way to differentiate from RTX.
> > >
> > > This is needed in the Cypress CCGX driver for the following ...
> > >
> > > https://lore.kernel.org/lkml/20220928150840.3804313-1-waynec@nvidia.com/
> > >
> > > Ideally, this should have been included in this series but was sent before.
> > > We can always re-work/update the above patch even though it has been queued
> > > up now.
> >
> > The driver seems to use this 16-bit value only to compare with a
> > corresponding field in the firmware headers. How exactly we obtain this
> > value is therefore not important. However, since this 16-bit value is
> > embedded in firmware images, we also cannot substitute them with
> > something more sensible.
>
> I am actually wondering if this is actually embedded in any images because I
> see it populated by the i2c-nvidia-gpu.c driver [0]. So I am wondering if we
> can use PROPERTY_ENTRY_STRING() for this driver instead and have a more
> descriptive name such as 'nvidia,rtx'?
What I mean by "embedded in firmware images" is that the value read from
the property is compared to values read from a firmware blob (either one
read back from the chip or one loaded using request_firmware()). See for
example ccg_check_vendor_version() and ccg_check_fw_version().
So the way that this 16-bit number is used is to define what type of
vendor firmware we support. So this is also used to avoid trying to load
a Tegra firmware on a GPU and vice versa.
So yes, we could potentially still make the i2c-nvidia-gpu.c driver add
a "nvidia,rtx" string to make it more descriptive like DT, but then we'd
still need to somehow resolve that to the "nv" string for the assignment
to uc->fw_build.
Not sure about how that would impact the AMD bits. Another of those CCGX
UCSI devices is registered by the i2c-designware-pcidrv.c driver, but it
doesn't pass a software node. From what I can tell that simply means all
of those checks will work with fw_build == 0x00. Primarily I think that
will cause flashing of the firmware not to be supported.
So yeah, having that string be something else (i.e. more descriptive)
and then match on that instead would definitely work. After looking at
this some more, using existing driver-matching may not work after all
because while there's ACPI matching and with this series DT matching,
the various GPU I2C instantiations are purely done in software, so they
have neither and therefore would need a secondary lookup mechanism. We
may be stuck with that ccgx,firmware-build property, but as you said it
should be possible to at least sanitize it.
Thierry
>
> Jon
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-nvidia-gpu.c#n261
> --
> nvpublic
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver
2022-10-28 14:07 ` Thierry Reding
@ 2022-11-03 10:47 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-11-03 10:47 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter
Cc: Rob Herring, Krzysztof Kozlowski, gregkh, Thierry Reding,
heikki.krogerus, Ajay Gupta, kishon, vkoul, p.zabel, balbi,
mathias.nyman, Jui Chang Kuo, linux-usb, devicetree,
linux-kernel, Sing-Han Chen, linux-i2c, linux-phy, linux-tegra
On 10/28/22 22:07, Thierry Reding wrote:
> On Fri, Oct 28, 2022 at 01:42:36PM +0100, Jon Hunter wrote:
>> On 28/10/2022 13:31, Thierry Reding wrote:
>>> On Wed, Oct 26, 2022 at 08:13:57AM +0100, Jon Hunter wrote:
>>>> On 24/10/2022 08:41, Wayne Chang wrote:
>>>>> add device-tree binding documentation for Cypress cypd4226 type-C
>>>>> controller's I2C interface. It is a standard i2c slave with GPIO
>>>>> input as IRQ interface.
>>>>>
>>>>> Signed-off-by: Wayne Chang<waynec@nvidia.com>
>>>>> ---
>>>>> .../bindings/usb/cypress,cypd4226.yaml | 86 +++++++++++++++++++
>>>>> 1 file changed, 86 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..5ac28ab4e7a1
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml
>>>>> @@ -0,0 +1,86 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id:http://devicetree.org/schemas/usb/cypress,cypd4226.yaml#
>>>>> +$schema:http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Cypress cypd4226 UCSI I2C Type-C Controller
>>>>> +
>>>>> +maintainers:
>>>>> + - Wayne Chang<waynec@nvidia.com>
>>>>> +
>>>>> +description: |
>>>>> + The Cypress cypd4226 UCSI I2C type-C controller is a I2C interface type-C
>>>>> + controller.
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + const: cypress,cypd4226
>>>>> +
>>>>> + '#address-cells':
>>>>> + const: 1
>>>>> +
>>>>> + '#size-cells':
>>>>> + const: 0
>>>>> +
>>>>> + reg:
>>>>> + const: 0x08
>>>>> +
>>>>> + interrupts:
>>>>> + maxItems: 1
>>>>> +
>>>>> + cypress,firmware-build:
>>>>> + enum:
>>>>> + - nv
>>>>> + - gn
>>>>> + description: |
>>>>> + the name of the CCGx firmware built for product series.
>>>>> + should be set one of following:
>>>>> + - "nv" for the RTX product series
>>>> Please add 'NVIDIA' so that it is 'for the NVIDIA RTX product series'
>>>>
>>>>> + - "gn" for the Jetson product series
>>>> Same here please add 'NVIDIA' so that it is 'for the NVIDIA Jetson product
>>>> series'.
>>>>
>>>> Rob, any concerns about this property in general? Unfortunately, ACPI choose
>>>> a 16-bit type for this and used 'nv' for the RTX product. I don't find 'gn'
>>>> for Jetson very descriptive but we need a way to differentiate from RTX.
>>>>
>>>> This is needed in the Cypress CCGX driver for the following ...
>>>>
>>>> https://lore.kernel.org/lkml/20220928150840.3804313-1-waynec@nvidia.com/
>>>>
>>>> Ideally, this should have been included in this series but was sent before.
>>>> We can always re-work/update the above patch even though it has been queued
>>>> up now.
>>> The driver seems to use this 16-bit value only to compare with a
>>> corresponding field in the firmware headers. How exactly we obtain this
>>> value is therefore not important. However, since this 16-bit value is
>>> embedded in firmware images, we also cannot substitute them with
>>> something more sensible.
>> I am actually wondering if this is actually embedded in any images because I
>> see it populated by the i2c-nvidia-gpu.c driver [0]. So I am wondering if we
>> can use PROPERTY_ENTRY_STRING() for this driver instead and have a more
>> descriptive name such as 'nvidia,rtx'?
> What I mean by "embedded in firmware images" is that the value read from
> the property is compared to values read from a firmware blob (either one
> read back from the chip or one loaded using request_firmware()). See for
> example ccg_check_vendor_version() and ccg_check_fw_version().
>
> So the way that this 16-bit number is used is to define what type of
> vendor firmware we support. So this is also used to avoid trying to load
> a Tegra firmware on a GPU and vice versa.
>
> So yes, we could potentially still make the i2c-nvidia-gpu.c driver add
> a "nvidia,rtx" string to make it more descriptive like DT, but then we'd
> still need to somehow resolve that to the "nv" string for the assignment
> to uc->fw_build.
>
> Not sure about how that would impact the AMD bits. Another of those CCGX
> UCSI devices is registered by the i2c-designware-pcidrv.c driver, but it
> doesn't pass a software node. From what I can tell that simply means all
> of those checks will work with fw_build == 0x00. Primarily I think that
> will cause flashing of the firmware not to be supported.
>
> So yeah, having that string be something else (i.e. more descriptive)
> and then match on that instead would definitely work. After looking at
> this some more, using existing driver-matching may not work after all
> because while there's ACPI matching and with this series DT matching,
> the various GPU I2C instantiations are purely done in software, so they
> have neither and therefore would need a secondary lookup mechanism. We
> may be stuck with that ccgx,firmware-build property, but as you said it
> should be possible to at least sanitize it.
>
OK. Thanks for the review. I'll make the change to extend the property
into string in the next patch series.
thanks,
Wayne.
> Thierry
>
>> Jon
>>
>> [0]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-nvidia-gpu.c#n261
>> --
>> nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
` (2 preceding siblings ...)
2022-10-24 7:41 ` [PATCH 03/11] dt-bindings: usb: Add binding for Cypress cypd4226 I2C driver Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-10-28 2:23 ` Krzysztof Kozlowski
2022-10-24 7:41 ` [PATCH 05/11] usb: typec: ucsi_ccg: Add OF support Wayne Chang
` (6 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
This commit enables XUSB host, device, and pad controller on
Jetson AGX Orin.
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
.../boot/dts/nvidia/tegra234-p3701-0000.dtsi | 48 +++++
.../nvidia/tegra234-p3737-0000+p3701-0000.dts | 184 ++++++++++++++++++
arch/arm64/boot/dts/nvidia/tegra234.dtsi | 170 ++++++++++++++++
3 files changed, 402 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
index 9e4d72cfa69f..8acef87a5398 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
@@ -61,6 +61,29 @@ mmc@3460000 {
non-removable;
};
+ padctl@3520000 {
+ vclamp-usb-supply = <&vdd_ao_1v8>;
+ avdd-usb-supply = <&vdd_ao_3v3>;
+
+ ports {
+ usb2-0 {
+ vbus-supply = <&vdd_5v0_sys>;
+ };
+
+ usb2-1 {
+ vbus-supply = <&vdd_5v0_sys>;
+ };
+
+ usb2-2 {
+ vbus-supply = <&vdd_5v0_sys>;
+ };
+
+ usb2-3 {
+ vbus-supply = <&vdd_5v0_sys>;
+ };
+ };
+ };
+
rtc@c2a0000 {
status = "okay";
};
@@ -69,4 +92,29 @@ pmc@c360000 {
nvidia,invert-interrupt;
};
};
+
+ vdd_5v0_sys: regulator@0 {
+ compatible = "regulator-fixed";
+ regulator-name = "VIN_SYS_5V0";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ vdd_ao_1v8: regulator@1 {
+ compatible = "regulator-fixed";
+ regulator-name = "vdd-AO-1v8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ };
+
+ vdd_ao_3v3: regulator@2 {
+ compatible = "regulator-fixed";
+ regulator-name = "vdd-AO-3v3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
};
diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
index 57ab75328814..b4630280bb32 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
@@ -2011,6 +2011,190 @@ hda@3510000 {
nvidia,model = "NVIDIA Jetson AGX Orin HDA";
status = "okay";
};
+
+ padctl@3520000 {
+ status = "okay";
+
+ pads {
+ usb2 {
+ lanes {
+ usb2-0 {
+ status = "okay";
+ };
+
+ usb2-1 {
+ status = "okay";
+ };
+
+ usb2-2 {
+ status = "okay";
+ };
+
+ usb2-3 {
+ status = "okay";
+ };
+ };
+ };
+
+ usb3 {
+ lanes {
+ usb3-0 {
+ status = "okay";
+ };
+
+ usb3-1 {
+ status = "okay";
+ };
+
+ usb3-2 {
+ status = "okay";
+ };
+ };
+ };
+ };
+
+ ports {
+ usb2-0 {
+ mode = "otg";
+ usb-role-switch;
+ status = "okay";
+ port {
+ hs_typec_p1: endpoint {
+ remote-endpoint = <&hs_ucsi_ccg_p1>;
+ };
+ };
+ };
+
+ usb2-1 {
+ mode = "host";
+ status = "okay";
+ port {
+ hs_typec_p0: endpoint {
+ remote-endpoint = <&hs_ucsi_ccg_p0>;
+ };
+ };
+ };
+
+ usb2-2 {
+ mode = "host";
+ status = "okay";
+ };
+
+ usb2-3 {
+ mode = "host";
+ status = "okay";
+ };
+
+ usb3-0 {
+ nvidia,usb2-companion = <1>;
+ status = "okay";
+ port {
+ ss_typec_p0: endpoint {
+ remote-endpoint = <&ss_ucsi_ccg_p0>;
+ };
+ };
+ };
+
+ usb3-1 {
+ nvidia,usb2-companion = <0>;
+ status = "okay";
+ port {
+ ss_typec_p1: endpoint {
+ remote-endpoint = <&ss_ucsi_ccg_p1>;
+ };
+ };
+ };
+
+ usb3-2 {
+ nvidia,usb2-companion = <3>;
+ status = "okay";
+ };
+ };
+ };
+
+ usb@3550000 {
+ status = "okay";
+
+ phys = <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-0}>,
+ <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-1}>;
+ phy-names = "usb2-0", "usb3-1";
+ };
+
+ usb@3610000 {
+ status = "okay";
+
+ phys = <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-0}>,
+ <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-1}>,
+ <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-2}>,
+ <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-3}>,
+ <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-0}>,
+ <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-1}>,
+ <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-2}>;
+ phy-names = "usb2-0", "usb2-1", "usb2-2", "usb2-3",
+ "usb3-0", "usb3-1", "usb3-2";
+ };
+
+ i2c@c240000 {
+ status = "okay";
+ ucsi_ccg: ucsi_ccg@8 {
+ compatible = "cypress,cypd4226";
+ cypress,firmware-build = "gn";
+ interrupt-parent = <&gpio>;
+ interrupts = <TEGRA234_MAIN_GPIO(Y, 4) IRQ_TYPE_LEVEL_LOW>;
+ reg = <0x08>;
+ status = "okay";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ccg_typec_con0: connector@0 {
+ compatible = "usb-c-connector";
+ reg = <0>;
+ label = "USB-C";
+ data-role = "host";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ hs_ucsi_ccg_p0: endpoint {
+ remote-endpoint = <&hs_typec_p0>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ss_ucsi_ccg_p0: endpoint {
+ remote-endpoint = <&ss_typec_p0>;
+ };
+ };
+ };
+ ccg_typec_con1: connector@1 {
+ compatible = "usb-c-connector";
+ reg = <1>;
+ label = "USB-C";
+ data-role = "dual";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ hs_ucsi_ccg_p1: endpoint {
+ remote-endpoint = <&hs_typec_p1>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ss_ucsi_ccg_p1: endpoint {
+ remote-endpoint = <&ss_typec_p1>;
+ };
+ };
+ };
+ };
+ };
};
chosen {
diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 0170bfa8a467..27635d459e4c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -942,6 +942,174 @@ hda@3510000 {
status = "disabled";
};
+ xusb_padctl: padctl@3520000 {
+ compatible = "nvidia,tegra234-xusb-padctl";
+ reg = <0x03520000 0x20000>,
+ <0x03540000 0x10000>;
+ reg-names = "padctl", "ao";
+ interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
+
+ resets = <&bpmp TEGRA234_RESET_XUSB_PADCTL>;
+ reset-names = "padctl";
+
+ status = "disabled";
+
+ pads {
+ usb2 {
+ clocks = <&bpmp TEGRA234_CLK_USB2_TRK>;
+ clock-names = "trk";
+
+ lanes {
+ usb2-0 {
+ nvidia,function = "xusb";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
+ usb2-1 {
+ nvidia,function = "xusb";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
+ usb2-2 {
+ nvidia,function = "xusb";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
+ usb2-3 {
+ nvidia,function = "xusb";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+ };
+ };
+
+ usb3 {
+ lanes {
+ usb3-0 {
+ nvidia,function = "xusb";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
+ usb3-1 {
+ nvidia,function = "xusb";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
+ usb3-2 {
+ nvidia,function = "xusb";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
+ usb3-3 {
+ nvidia,function = "xusb";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+ };
+ };
+ };
+
+ ports {
+ usb2-0 {
+ status = "disabled";
+ };
+
+ usb2-1 {
+ status = "disabled";
+ };
+
+ usb2-2 {
+ status = "disabled";
+ };
+
+ usb2-3 {
+ status = "disabled";
+ };
+
+ usb3-0 {
+ status = "disabled";
+ };
+
+ usb3-1 {
+ status = "disabled";
+ };
+
+ usb3-2 {
+ status = "disabled";
+ };
+
+ usb3-3 {
+ status = "disabled";
+ };
+ };
+ };
+
+ usb@3550000 {
+ compatible = "nvidia,tegra234-xudc";
+ reg = <0x03550000 0x8000>,
+ <0x03558000 0x8000>;
+ reg-names = "base", "fpci";
+ interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&bpmp TEGRA234_CLK_XUSB_CORE_DEV>,
+ <&bpmp TEGRA234_CLK_XUSB_CORE_SS>,
+ <&bpmp TEGRA234_CLK_XUSB_SS>,
+ <&bpmp TEGRA234_CLK_XUSB_FS>;
+ clock-names = "dev", "ss", "ss_src", "fs_src";
+ interconnects = <&mc TEGRA234_MEMORY_CLIENT_XUSB_DEVR &emc>,
+ <&mc TEGRA234_MEMORY_CLIENT_XUSB_DEVW &emc>;
+ interconnect-names = "dma-mem", "write";
+ iommus = <&smmu_niso1 TEGRA234_SID_XUSB_DEV>;
+ power-domains = <&bpmp TEGRA234_POWER_DOMAIN_XUSBB>,
+ <&bpmp TEGRA234_POWER_DOMAIN_XUSBA>;
+ power-domain-names = "dev", "ss";
+ nvidia,xusb-padctl = <&xusb_padctl>;
+ dma-coherent;
+ status = "disabled";
+ };
+
+ usb@3610000 {
+ compatible = "nvidia,tegra234-xusb";
+ reg = <0x03610000 0x40000>,
+ <0x03600000 0x10000>,
+ <0x03650000 0x10000>;
+ reg-names = "hcd", "fpci", "bar2";
+
+ interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
+
+ clocks = <&bpmp TEGRA234_CLK_XUSB_CORE_HOST>,
+ <&bpmp TEGRA234_CLK_XUSB_FALCON>,
+ <&bpmp TEGRA234_CLK_XUSB_CORE_SS>,
+ <&bpmp TEGRA234_CLK_XUSB_SS>,
+ <&bpmp TEGRA234_CLK_CLK_M>,
+ <&bpmp TEGRA234_CLK_XUSB_FS>,
+ <&bpmp TEGRA234_CLK_UTMIP_PLL>,
+ <&bpmp TEGRA234_CLK_CLK_M>,
+ <&bpmp TEGRA234_CLK_PLLE>;
+ clock-names = "xusb_host", "xusb_falcon_src",
+ "xusb_ss", "xusb_ss_src", "xusb_hs_src",
+ "xusb_fs_src", "pll_u_480m", "clk_m",
+ "pll_e";
+ interconnects = <&mc TEGRA234_MEMORY_CLIENT_XUSB_HOSTR &emc>,
+ <&mc TEGRA234_MEMORY_CLIENT_XUSB_HOSTW &emc>;
+ interconnect-names = "dma-mem", "write";
+ iommus = <&smmu_niso1 TEGRA234_SID_XUSB_HOST>;
+
+ power-domains = <&bpmp TEGRA234_POWER_DOMAIN_XUSBC>,
+ <&bpmp TEGRA234_POWER_DOMAIN_XUSBA>;
+ power-domain-names = "xusb_host", "xusb_ss";
+
+ nvidia,xusb-padctl = <&xusb_padctl>;
+ dma-coherent;
+ status = "disabled";
+ };
+
fuse@3810000 {
compatible = "nvidia,tegra234-efuse";
reg = <0x03810000 0x10000>;
@@ -1470,6 +1638,8 @@ gen2_i2c: i2c@c240000 {
compatible = "nvidia,tegra194-i2c";
reg = <0xc240000 0x100>;
interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
status = "disabled";
clock-frequency = <100000>;
clocks = <&bpmp TEGRA234_CLK_I2C2
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin
2022-10-24 7:41 ` [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin Wayne Chang
@ 2022-10-28 2:23 ` Krzysztof Kozlowski
2022-10-28 9:33 ` Jon Hunter
0 siblings, 1 reply; 50+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-28 2:23 UTC (permalink / raw)
To: Wayne Chang, gregkh, robh+dt, krzysztof.kozlowski+dt, treding,
jonathanh, thierry.reding, heikki.krogerus, ajayg, kishon, vkoul,
p.zabel, balbi, mathias.nyman, jckuo
Cc: linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
On 24/10/2022 03:41, Wayne Chang wrote:
> This commit enables XUSB host, device, and pad controller on
> Jetson AGX Orin.
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> .../boot/dts/nvidia/tegra234-p3701-0000.dtsi | 48 +++++
> .../nvidia/tegra234-p3737-0000+p3701-0000.dts | 184 ++++++++++++++++++
> arch/arm64/boot/dts/nvidia/tegra234.dtsi | 170 ++++++++++++++++
> 3 files changed, 402 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
> index 9e4d72cfa69f..8acef87a5398 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
> @@ -61,6 +61,29 @@ mmc@3460000 {
> non-removable;
> };
>
> + padctl@3520000 {
> + vclamp-usb-supply = <&vdd_ao_1v8>;
> + avdd-usb-supply = <&vdd_ao_3v3>;
> +
> + ports {
> + usb2-0 {
> + vbus-supply = <&vdd_5v0_sys>;
> + };
> +
> + usb2-1 {
> + vbus-supply = <&vdd_5v0_sys>;
> + };
> +
> + usb2-2 {
> + vbus-supply = <&vdd_5v0_sys>;
> + };
> +
> + usb2-3 {
> + vbus-supply = <&vdd_5v0_sys>;
> + };
> + };
> + };
> +
> rtc@c2a0000 {
> status = "okay";
> };
> @@ -69,4 +92,29 @@ pmc@c360000 {
> nvidia,invert-interrupt;
> };
> };
> +
> + vdd_5v0_sys: regulator@0 {
> + compatible = "regulator-fixed";
> + regulator-name = "VIN_SYS_5V0";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + vdd_ao_1v8: regulator@1 {
> + compatible = "regulator-fixed";
> + regulator-name = "vdd-AO-1v8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + };
> +
> + vdd_ao_3v3: regulator@2 {
> + compatible = "regulator-fixed";
> + regulator-name = "vdd-AO-3v3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };
> };
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
> index 57ab75328814..b4630280bb32 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
> +++ b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
> @@ -2011,6 +2011,190 @@ hda@3510000 {
> nvidia,model = "NVIDIA Jetson AGX Orin HDA";
> status = "okay";
> };
> +
> + padctl@3520000 {
> + status = "okay";
> +
> + pads {
> + usb2 {
> + lanes {
> + usb2-0 {
> + status = "okay";
> + };
> +
> + usb2-1 {
> + status = "okay";
> + };
> +
> + usb2-2 {
> + status = "okay";
> + };
> +
> + usb2-3 {
> + status = "okay";
> + };
> + };
> + };
> +
> + usb3 {
> + lanes {
> + usb3-0 {
> + status = "okay";
> + };
> +
> + usb3-1 {
> + status = "okay";
> + };
> +
> + usb3-2 {
> + status = "okay";
> + };
> + };
> + };
> + };
> +
> + ports {
> + usb2-0 {
> + mode = "otg";
> + usb-role-switch;
> + status = "okay";
> + port {
> + hs_typec_p1: endpoint {
> + remote-endpoint = <&hs_ucsi_ccg_p1>;
> + };
> + };
> + };
> +
> + usb2-1 {
> + mode = "host";
> + status = "okay";
> + port {
> + hs_typec_p0: endpoint {
> + remote-endpoint = <&hs_ucsi_ccg_p0>;
> + };
> + };
> + };
> +
> + usb2-2 {
> + mode = "host";
> + status = "okay";
> + };
> +
> + usb2-3 {
> + mode = "host";
> + status = "okay";
> + };
> +
> + usb3-0 {
> + nvidia,usb2-companion = <1>;
> + status = "okay";
> + port {
> + ss_typec_p0: endpoint {
> + remote-endpoint = <&ss_ucsi_ccg_p0>;
> + };
> + };
> + };
> +
> + usb3-1 {
> + nvidia,usb2-companion = <0>;
> + status = "okay";
> + port {
> + ss_typec_p1: endpoint {
> + remote-endpoint = <&ss_ucsi_ccg_p1>;
> + };
> + };
> + };
> +
> + usb3-2 {
> + nvidia,usb2-companion = <3>;
> + status = "okay";
> + };
> + };
> + };
> +
> + usb@3550000 {
> + status = "okay";
> +
> + phys = <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-0}>,
> + <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-1}>;
> + phy-names = "usb2-0", "usb3-1";
> + };
> +
> + usb@3610000 {
> + status = "okay";
> +
> + phys = <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-0}>,
> + <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-1}>,
> + <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-2}>,
> + <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-3}>,
> + <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-0}>,
> + <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-1}>,
> + <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-2}>;
> + phy-names = "usb2-0", "usb2-1", "usb2-2", "usb2-3",
> + "usb3-0", "usb3-1", "usb3-2";
> + };
> +
> + i2c@c240000 {
> + status = "okay";
> + ucsi_ccg: ucsi_ccg@8 {
No underscores in node names.
> + compatible = "cypress,cypd4226";
> + cypress,firmware-build = "gn";
> + interrupt-parent = <&gpio>;
> + interrupts = <TEGRA234_MAIN_GPIO(Y, 4) IRQ_TYPE_LEVEL_LOW>;
> + reg = <0x08>;
> + status = "okay";
The pattern of redefining full path in Tegra is confusing - I have no
clue which of these status=okay are correct which are redundant.
Do you?
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ccg_typec_con0: connector@0 {
> + compatible = "usb-c-connector";
> + reg = <0>;
> + label = "USB-C";
> + data-role = "host";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
Hm, why do you have here cells?
Did you test the DTS with dtbs_check?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin
2022-10-28 2:23 ` Krzysztof Kozlowski
@ 2022-10-28 9:33 ` Jon Hunter
2022-10-28 11:27 ` Krzysztof Kozlowski
0 siblings, 1 reply; 50+ messages in thread
From: Jon Hunter @ 2022-10-28 9:33 UTC (permalink / raw)
To: Krzysztof Kozlowski, Wayne Chang, gregkh, robh+dt,
krzysztof.kozlowski+dt, treding, thierry.reding, heikki.krogerus,
ajayg, kishon, vkoul, p.zabel, balbi, mathias.nyman, jckuo
Cc: linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
On 28/10/2022 03:23, Krzysztof Kozlowski wrote:
> On 24/10/2022 03:41, Wayne Chang wrote:
>> This commit enables XUSB host, device, and pad controller on
>> Jetson AGX Orin.
>>
>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>> ---
>> .../boot/dts/nvidia/tegra234-p3701-0000.dtsi | 48 +++++
>> .../nvidia/tegra234-p3737-0000+p3701-0000.dts | 184 ++++++++++++++++++
>> arch/arm64/boot/dts/nvidia/tegra234.dtsi | 170 ++++++++++++++++
>> 3 files changed, 402 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
>> index 9e4d72cfa69f..8acef87a5398 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra234-p3701-0000.dtsi
>> @@ -61,6 +61,29 @@ mmc@3460000 {
>> non-removable;
>> };
>>
>> + padctl@3520000 {
>> + vclamp-usb-supply = <&vdd_ao_1v8>;
>> + avdd-usb-supply = <&vdd_ao_3v3>;
>> +
>> + ports {
>> + usb2-0 {
>> + vbus-supply = <&vdd_5v0_sys>;
>> + };
>> +
>> + usb2-1 {
>> + vbus-supply = <&vdd_5v0_sys>;
>> + };
>> +
>> + usb2-2 {
>> + vbus-supply = <&vdd_5v0_sys>;
>> + };
>> +
>> + usb2-3 {
>> + vbus-supply = <&vdd_5v0_sys>;
>> + };
>> + };
>> + };
>> +
>> rtc@c2a0000 {
>> status = "okay";
>> };
>> @@ -69,4 +92,29 @@ pmc@c360000 {
>> nvidia,invert-interrupt;
>> };
>> };
>> +
>> + vdd_5v0_sys: regulator@0 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "VIN_SYS_5V0";
>> + regulator-min-microvolt = <5000000>;
>> + regulator-max-microvolt = <5000000>;
>> + regulator-always-on;
>> + regulator-boot-on;
>> + };
>> +
>> + vdd_ao_1v8: regulator@1 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vdd-AO-1v8";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + regulator-always-on;
>> + };
>> +
>> + vdd_ao_3v3: regulator@2 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vdd-AO-3v3";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-always-on;
>> + };
>> };
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
>> index 57ab75328814..b4630280bb32 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
>> +++ b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
>> @@ -2011,6 +2011,190 @@ hda@3510000 {
>> nvidia,model = "NVIDIA Jetson AGX Orin HDA";
>> status = "okay";
>> };
>> +
>> + padctl@3520000 {
>> + status = "okay";
>> +
>> + pads {
>> + usb2 {
>> + lanes {
>> + usb2-0 {
>> + status = "okay";
>> + };
>> +
>> + usb2-1 {
>> + status = "okay";
>> + };
>> +
>> + usb2-2 {
>> + status = "okay";
>> + };
>> +
>> + usb2-3 {
>> + status = "okay";
>> + };
>> + };
>> + };
>> +
>> + usb3 {
>> + lanes {
>> + usb3-0 {
>> + status = "okay";
>> + };
>> +
>> + usb3-1 {
>> + status = "okay";
>> + };
>> +
>> + usb3-2 {
>> + status = "okay";
>> + };
>> + };
>> + };
>> + };
>> +
>> + ports {
>> + usb2-0 {
>> + mode = "otg";
>> + usb-role-switch;
>> + status = "okay";
>> + port {
>> + hs_typec_p1: endpoint {
>> + remote-endpoint = <&hs_ucsi_ccg_p1>;
>> + };
>> + };
>> + };
>> +
>> + usb2-1 {
>> + mode = "host";
>> + status = "okay";
>> + port {
>> + hs_typec_p0: endpoint {
>> + remote-endpoint = <&hs_ucsi_ccg_p0>;
>> + };
>> + };
>> + };
>> +
>> + usb2-2 {
>> + mode = "host";
>> + status = "okay";
>> + };
>> +
>> + usb2-3 {
>> + mode = "host";
>> + status = "okay";
>> + };
>> +
>> + usb3-0 {
>> + nvidia,usb2-companion = <1>;
>> + status = "okay";
>> + port {
>> + ss_typec_p0: endpoint {
>> + remote-endpoint = <&ss_ucsi_ccg_p0>;
>> + };
>> + };
>> + };
>> +
>> + usb3-1 {
>> + nvidia,usb2-companion = <0>;
>> + status = "okay";
>> + port {
>> + ss_typec_p1: endpoint {
>> + remote-endpoint = <&ss_ucsi_ccg_p1>;
>> + };
>> + };
>> + };
>> +
>> + usb3-2 {
>> + nvidia,usb2-companion = <3>;
>> + status = "okay";
>> + };
>> + };
>> + };
>> +
>> + usb@3550000 {
>> + status = "okay";
>> +
>> + phys = <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-0}>,
>> + <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-1}>;
>> + phy-names = "usb2-0", "usb3-1";
>> + };
>> +
>> + usb@3610000 {
>> + status = "okay";
>> +
>> + phys = <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-0}>,
>> + <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-1}>,
>> + <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-2}>,
>> + <&{/bus@0/padctl@3520000/pads/usb2/lanes/usb2-3}>,
>> + <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-0}>,
>> + <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-1}>,
>> + <&{/bus@0/padctl@3520000/pads/usb3/lanes/usb3-2}>;
>> + phy-names = "usb2-0", "usb2-1", "usb2-2", "usb2-3",
>> + "usb3-0", "usb3-1", "usb3-2";
>> + };
>> +
>> + i2c@c240000 {
>> + status = "okay";
>> + ucsi_ccg: ucsi_ccg@8 {
>
> No underscores in node names.
>
>> + compatible = "cypress,cypd4226";
>> + cypress,firmware-build = "gn";
>> + interrupt-parent = <&gpio>;
>> + interrupts = <TEGRA234_MAIN_GPIO(Y, 4) IRQ_TYPE_LEVEL_LOW>;
>> + reg = <0x08>;
>> + status = "okay";
>
> The pattern of redefining full path in Tegra is confusing - I have no
> clue which of these status=okay are correct which are redundant.
>
> Do you?
I understand you may not like this approach, however, this comment is
not really relevant to just this patch, but a general comment. But yes
we will ensure that this is correct.
>
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + ccg_typec_con0: connector@0 {
>> + compatible = "usb-c-connector";
>> + reg = <0>;
>> + label = "USB-C";
>> + data-role = "host";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>
> Hm, why do you have here cells?
>
> Did you test the DTS with dtbs_check?
That does not look correct and so we will correct.
Thanks!
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin
2022-10-28 9:33 ` Jon Hunter
@ 2022-10-28 11:27 ` Krzysztof Kozlowski
2022-10-28 11:34 ` Jon Hunter
2022-10-28 12:38 ` Thierry Reding
0 siblings, 2 replies; 50+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-28 11:27 UTC (permalink / raw)
To: Jon Hunter, Wayne Chang, gregkh, robh+dt, krzysztof.kozlowski+dt,
treding, thierry.reding, heikki.krogerus, ajayg, kishon, vkoul,
p.zabel, balbi, mathias.nyman, jckuo
Cc: linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
On 28/10/2022 05:33, Jon Hunter wrote:
>>> + ucsi_ccg: ucsi_ccg@8 {
>>
>> No underscores in node names.
>>
>>> + compatible = "cypress,cypd4226";
>>> + cypress,firmware-build = "gn";
>>> + interrupt-parent = <&gpio>;
>>> + interrupts = <TEGRA234_MAIN_GPIO(Y, 4) IRQ_TYPE_LEVEL_LOW>;
>>> + reg = <0x08>;
>>> + status = "okay";
>>
>> The pattern of redefining full path in Tegra is confusing - I have no
>> clue which of these status=okay are correct which are redundant.
>>
>> Do you?
>
> I understand you may not like this approach, however, this comment is
> not really relevant to just this patch, but a general comment. But yes
> we will ensure that this is correct.
>
Just to clarify - this status looks redundant, but I have no way to tell
for sure...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin
2022-10-28 11:27 ` Krzysztof Kozlowski
@ 2022-10-28 11:34 ` Jon Hunter
2022-10-28 12:38 ` Thierry Reding
1 sibling, 0 replies; 50+ messages in thread
From: Jon Hunter @ 2022-10-28 11:34 UTC (permalink / raw)
To: Krzysztof Kozlowski, Wayne Chang, gregkh, robh+dt,
krzysztof.kozlowski+dt, treding, thierry.reding, heikki.krogerus,
ajayg, kishon, vkoul, p.zabel, balbi, mathias.nyman, jckuo
Cc: linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
On 28/10/2022 12:27, Krzysztof Kozlowski wrote:
> On 28/10/2022 05:33, Jon Hunter wrote:
>>>> + ucsi_ccg: ucsi_ccg@8 {
>>>
>>> No underscores in node names.
>>>
>>>> + compatible = "cypress,cypd4226";
>>>> + cypress,firmware-build = "gn";
>>>> + interrupt-parent = <&gpio>;
>>>> + interrupts = <TEGRA234_MAIN_GPIO(Y, 4) IRQ_TYPE_LEVEL_LOW>;
>>>> + reg = <0x08>;
>>>> + status = "okay";
>>>
>>> The pattern of redefining full path in Tegra is confusing - I have no
>>> clue which of these status=okay are correct which are redundant.
>>>
>>> Do you?
>>
>> I understand you may not like this approach, however, this comment is
>> not really relevant to just this patch, but a general comment. But yes
>> we will ensure that this is correct.
>>
>
> Just to clarify - this status looks redundant, but I have no way to tell
> for sure...
I see. This is the only place where this device appears. I always forget
if we are meant to explicitly set status to okay or just omit.
Personally, I always prefer to be explicit.
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin
2022-10-28 11:27 ` Krzysztof Kozlowski
2022-10-28 11:34 ` Jon Hunter
@ 2022-10-28 12:38 ` Thierry Reding
2022-10-28 21:48 ` Krzysztof Kozlowski
1 sibling, 1 reply; 50+ messages in thread
From: Thierry Reding @ 2022-10-28 12:38 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jon Hunter, Wayne Chang, gregkh, robh+dt, krzysztof.kozlowski+dt,
treding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]
On Fri, Oct 28, 2022 at 07:27:09AM -0400, Krzysztof Kozlowski wrote:
> On 28/10/2022 05:33, Jon Hunter wrote:
> >>> + ucsi_ccg: ucsi_ccg@8 {
> >>
> >> No underscores in node names.
> >>
> >>> + compatible = "cypress,cypd4226";
> >>> + cypress,firmware-build = "gn";
> >>> + interrupt-parent = <&gpio>;
> >>> + interrupts = <TEGRA234_MAIN_GPIO(Y, 4) IRQ_TYPE_LEVEL_LOW>;
> >>> + reg = <0x08>;
> >>> + status = "okay";
> >>
> >> The pattern of redefining full path in Tegra is confusing - I have no
> >> clue which of these status=okay are correct which are redundant.
> >>
> >> Do you?
> >
> > I understand you may not like this approach, however, this comment is
> > not really relevant to just this patch, but a general comment. But yes
> > we will ensure that this is correct.
> >
>
> Just to clarify - this status looks redundant, but I have no way to tell
> for sure...
But that's independent of whether we specify this using the full path or
reference the node by label, isn't it? The only way to make sure that a
status = "okay" is not redundant is by manual inspection. I don't know
of an automated way to do that. Perhaps it's something that could be
added as a check to DTC?
In this particular case I don't think the status is needed. As Jon
mentioned, this device is first defined here and status = "okay" is the
default, so this is redundant.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin
2022-10-28 12:38 ` Thierry Reding
@ 2022-10-28 21:48 ` Krzysztof Kozlowski
0 siblings, 0 replies; 50+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-28 21:48 UTC (permalink / raw)
To: Thierry Reding
Cc: Jon Hunter, Wayne Chang, gregkh, robh+dt, krzysztof.kozlowski+dt,
treding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On 28/10/2022 08:38, Thierry Reding wrote:
>>>
>>> I understand you may not like this approach, however, this comment is
>>> not really relevant to just this patch, but a general comment. But yes
>>> we will ensure that this is correct.
>>>
>>
>> Just to clarify - this status looks redundant, but I have no way to tell
>> for sure...
>
> But that's independent of whether we specify this using the full path or
> reference the node by label, isn't it? The only way to make sure that a
> status = "okay" is not redundant is by manual inspection. I don't know
> of an automated way to do that. Perhaps it's something that could be
> added as a check to DTC?
With overrides/extends pattern it is easy to spot one case of mistakes -
you see override, then status might be needed might not. You see new
node (like here!) - then status=okay is redundant.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 05/11] usb: typec: ucsi_ccg: Add OF support
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
` (3 preceding siblings ...)
2022-10-24 7:41 ` [PATCH 04/11] arm64: tegra: Enable XUSB host and device on Jetson AGX Orin Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-10-24 7:41 ` [PATCH 06/11] usb: typec: ucsi_ccg: Replace ccgx to well-known regex Wayne Chang
` (5 subsequent siblings)
10 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
The change enables the device tree infrastructure support.
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
drivers/usb/typec/ucsi/ucsi_ccg.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 835f1c4372ba..139707a2f3d6 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -643,7 +643,7 @@ static int ccg_request_irq(struct ucsi_ccg *uc)
{
unsigned long flags = IRQF_ONESHOT;
- if (!has_acpi_companion(uc->dev))
+ if (!dev_fwnode(uc->dev))
flags |= IRQF_TRIGGER_HIGH;
return request_threaded_irq(uc->irq, NULL, ccg_irq_handler, flags, dev_name(uc->dev), uc);
@@ -1427,6 +1427,12 @@ static void ucsi_ccg_remove(struct i2c_client *client)
free_irq(uc->irq, uc);
}
+static const struct of_device_id ucsi_ccg_of_match_table[] = {
+ { .compatible = "cypress,cypd4226", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ucsi_ccg_of_match_table);
+
static const struct i2c_device_id ucsi_ccg_device_id[] = {
{"ccgx-ucsi", 0},
{}
@@ -1481,6 +1487,7 @@ static struct i2c_driver ucsi_ccg_driver = {
.pm = &ucsi_ccg_pm,
.dev_groups = ucsi_ccg_groups,
.acpi_match_table = amd_i2c_ucsi_match,
+ .of_match_table = ucsi_ccg_of_match_table,
},
.probe = ucsi_ccg_probe,
.remove = ucsi_ccg_remove,
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 06/11] usb: typec: ucsi_ccg: Replace ccgx to well-known regex
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
` (4 preceding siblings ...)
2022-10-24 7:41 ` [PATCH 05/11] usb: typec: ucsi_ccg: Add OF support Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-10-24 8:01 ` Heikki Krogerus
2022-10-24 7:41 ` [PATCH 07/11] i2c: nvidia-gpu: " Wayne Chang
` (4 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
ccgx is refer to the cypress cypd4226 typec controller.
Replace ccgx to well-known regex "cypress".
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 139707a2f3d6..5d3099e6eb77 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -1358,7 +1358,7 @@ static int ucsi_ccg_probe(struct i2c_client *client,
INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
/* Only fail FW flashing when FW build information is not provided */
- status = device_property_read_u16(dev, "ccgx,firmware-build",
+ status = device_property_read_u16(dev, "cypress,firmware-build",
&uc->fw_build);
if (status)
dev_err(uc->dev, "failed to get FW build information\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 06/11] usb: typec: ucsi_ccg: Replace ccgx to well-known regex
2022-10-24 7:41 ` [PATCH 06/11] usb: typec: ucsi_ccg: Replace ccgx to well-known regex Wayne Chang
@ 2022-10-24 8:01 ` Heikki Krogerus
2022-10-24 8:29 ` Felipe Balbi
0 siblings, 1 reply; 50+ messages in thread
From: Heikki Krogerus @ 2022-10-24 8:01 UTC (permalink / raw)
To: Wayne Chang
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, ajayg, kishon, vkoul, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On Mon, Oct 24, 2022 at 03:41:23PM +0800, Wayne Chang wrote:
> ccgx is refer to the cypress cypd4226 typec controller.
> Replace ccgx to well-known regex "cypress".
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
> index 139707a2f3d6..5d3099e6eb77 100644
> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -1358,7 +1358,7 @@ static int ucsi_ccg_probe(struct i2c_client *client,
> INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
>
> /* Only fail FW flashing when FW build information is not provided */
> - status = device_property_read_u16(dev, "ccgx,firmware-build",
> + status = device_property_read_u16(dev, "cypress,firmware-build",
> &uc->fw_build);
> if (status)
> dev_err(uc->dev, "failed to get FW build information\n");
This will break bisectability. You need to first add that
"cyppress,firmware-build" identifier without removing the old
"ccgx,firmware-build" identifier, and then introduce a separate
clean-up patch where you remove it when it's safe to remove:
1. Add new - This patch.
2. Modify users - PATCH 7/11.
3. Remove old - *missing*.
thanks,
--
heikki
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/11] usb: typec: ucsi_ccg: Replace ccgx to well-known regex
2022-10-24 8:01 ` Heikki Krogerus
@ 2022-10-24 8:29 ` Felipe Balbi
2022-10-24 8:46 ` Heikki Krogerus
0 siblings, 1 reply; 50+ messages in thread
From: Felipe Balbi @ 2022-10-24 8:29 UTC (permalink / raw)
To: Heikki Krogerus, Wayne Chang
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, ajayg, kishon, vkoul, p.zabel, mathias.nyman,
jckuo, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]
Heikki Krogerus <heikki.krogerus@linux.intel.com> writes:
> On Mon, Oct 24, 2022 at 03:41:23PM +0800, Wayne Chang wrote:
>> ccgx is refer to the cypress cypd4226 typec controller.
>> Replace ccgx to well-known regex "cypress".
>>
>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>> ---
>> drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
>> index 139707a2f3d6..5d3099e6eb77 100644
>> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
>> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
>> @@ -1358,7 +1358,7 @@ static int ucsi_ccg_probe(struct i2c_client *client,
>> INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
>>
>> /* Only fail FW flashing when FW build information is not provided */
>> - status = device_property_read_u16(dev, "ccgx,firmware-build",
>> + status = device_property_read_u16(dev, "cypress,firmware-build",
>> &uc->fw_build);
>> if (status)
>> dev_err(uc->dev, "failed to get FW build information\n");
>
> This will break bisectability. You need to first add that
> "cyppress,firmware-build" identifier without removing the old
> "ccgx,firmware-build" identifier, and then introduce a separate
> clean-up patch where you remove it when it's safe to remove:
>
> 1. Add new - This patch.
> 2. Modify users - PATCH 7/11.
> 3. Remove old - *missing*.
will it ever be safe to remove? What about potential products in the
market with little to no upgrade path? There are likely to be products
with a DTB that will never be updated, no?
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/11] usb: typec: ucsi_ccg: Replace ccgx to well-known regex
2022-10-24 8:29 ` Felipe Balbi
@ 2022-10-24 8:46 ` Heikki Krogerus
2022-10-25 7:26 ` Wayne Chang
0 siblings, 1 reply; 50+ messages in thread
From: Heikki Krogerus @ 2022-10-24 8:46 UTC (permalink / raw)
To: Felipe Balbi
Cc: Wayne Chang, gregkh, robh+dt, krzysztof.kozlowski+dt, treding,
jonathanh, thierry.reding, ajayg, kishon, vkoul, p.zabel,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On Mon, Oct 24, 2022 at 11:29:27AM +0300, Felipe Balbi wrote:
> Heikki Krogerus <heikki.krogerus@linux.intel.com> writes:
>
> > On Mon, Oct 24, 2022 at 03:41:23PM +0800, Wayne Chang wrote:
> >> ccgx is refer to the cypress cypd4226 typec controller.
> >> Replace ccgx to well-known regex "cypress".
> >>
> >> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> >> ---
> >> drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
> >> index 139707a2f3d6..5d3099e6eb77 100644
> >> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> >> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> >> @@ -1358,7 +1358,7 @@ static int ucsi_ccg_probe(struct i2c_client *client,
> >> INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
> >>
> >> /* Only fail FW flashing when FW build information is not provided */
> >> - status = device_property_read_u16(dev, "ccgx,firmware-build",
> >> + status = device_property_read_u16(dev, "cypress,firmware-build",
> >> &uc->fw_build);
> >> if (status)
> >> dev_err(uc->dev, "failed to get FW build information\n");
> >
> > This will break bisectability. You need to first add that
> > "cyppress,firmware-build" identifier without removing the old
> > "ccgx,firmware-build" identifier, and then introduce a separate
> > clean-up patch where you remove it when it's safe to remove:
> >
> > 1. Add new - This patch.
> > 2. Modify users - PATCH 7/11.
> > 3. Remove old - *missing*.
>
> will it ever be safe to remove? What about potential products in the
> market with little to no upgrade path? There are likely to be products
> with a DTB that will never be updated, no?
Not the case here. OF support is only just added to this driver in
this series. That old identifier has been used as a build-in property
only.
thanks,
--
heikki
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/11] usb: typec: ucsi_ccg: Replace ccgx to well-known regex
2022-10-24 8:46 ` Heikki Krogerus
@ 2022-10-25 7:26 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-10-25 7:26 UTC (permalink / raw)
To: Heikki Krogerus, Felipe Balbi
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, Thierry Reding,
Jonathan Hunter, thierry.reding, Ajay Gupta, kishon, vkoul,
p.zabel, mathias.nyman, Jui Chang Kuo, linux-usb, devicetree,
linux-kernel, Sing-Han Chen, linux-i2c, linux-phy, linux-tegra
Thanks for the review.
On 10/24/22 16:46, Heikki Krogerus wrote:
> External email: Use caution opening links or attachments
>
>
> On Mon, Oct 24, 2022 at 11:29:27AM +0300, Felipe Balbi wrote:
>> Heikki Krogerus <heikki.krogerus@linux.intel.com> writes:
>>
>>> On Mon, Oct 24, 2022 at 03:41:23PM +0800, Wayne Chang wrote:
>>>> ccgx is refer to the cypress cypd4226 typec controller.
>>>> Replace ccgx to well-known regex "cypress".
>>>>
>>>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>>>> ---
>>>> drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
>>>> index 139707a2f3d6..5d3099e6eb77 100644
>>>> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
>>>> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
>>>> @@ -1358,7 +1358,7 @@ static int ucsi_ccg_probe(struct i2c_client *client,
>>>> INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
>>>>
>>>> /* Only fail FW flashing when FW build information is not provided */
>>>> - status = device_property_read_u16(dev, "ccgx,firmware-build",
>>>> + status = device_property_read_u16(dev, "cypress,firmware-build",
>>>> &uc->fw_build);
>>>> if (status)
>>>> dev_err(uc->dev, "failed to get FW build information\n");
>>>
>>> This will break bisectability. You need to first add that
>>> "cyppress,firmware-build" identifier without removing the old
>>> "ccgx,firmware-build" identifier, and then introduce a separate
>>> clean-up patch where you remove it when it's safe to remove:
>>>
>>> 1. Add new - This patch.
>>> 2. Modify users - PATCH 7/11.
>>> 3. Remove old - *missing*.
>>
thanks for the guidance.
will update the changes in the next patchset.
thanks,
Wayne.
>> will it ever be safe to remove? What about potential products in the
>> market with little to no upgrade path? There are likely to be products
>> with a DTB that will never be updated, no?
>
> Not the case here. OF support is only just added to this driver in
> this series. That old identifier has been used as a build-in property
> only.
>
> thanks,
>
> --
> heikki
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 07/11] i2c: nvidia-gpu: Replace ccgx to well-known regex
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
` (5 preceding siblings ...)
2022-10-24 7:41 ` [PATCH 06/11] usb: typec: ucsi_ccg: Replace ccgx to well-known regex Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-11-01 15:07 ` Jon Hunter
2022-10-24 7:41 ` [PATCH 08/11] phy: tegra: xusb: Disable trk clk when not using Wayne Chang
` (3 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
ccgx is refer to the cypress cypd4226 typec controller.
Replace ccgx to well-known regex "cypress".
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
drivers/i2c/busses/i2c-nvidia-gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c
index 12e330cd7635..0934f8ad7f49 100644
--- a/drivers/i2c/busses/i2c-nvidia-gpu.c
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -260,7 +260,7 @@ MODULE_DEVICE_TABLE(pci, gpu_i2c_ids);
static const struct property_entry ccgx_props[] = {
/* Use FW built for NVIDIA (nv) only */
- PROPERTY_ENTRY_U16("ccgx,firmware-build", ('n' << 8) | 'v'),
+ PROPERTY_ENTRY_U16("cypress,firmware-build", ('n' << 8) | 'v'),
{ }
};
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 07/11] i2c: nvidia-gpu: Replace ccgx to well-known regex
2022-10-24 7:41 ` [PATCH 07/11] i2c: nvidia-gpu: " Wayne Chang
@ 2022-11-01 15:07 ` Jon Hunter
2022-11-03 11:36 ` Wayne Chang
0 siblings, 1 reply; 50+ messages in thread
From: Jon Hunter @ 2022-11-01 15:07 UTC (permalink / raw)
To: Wayne Chang, gregkh, robh+dt, krzysztof.kozlowski+dt, treding,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
On 24/10/2022 08:41, Wayne Chang wrote:
> ccgx is refer to the cypress cypd4226 typec controller.
> Replace ccgx to well-known regex "cypress".
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> drivers/i2c/busses/i2c-nvidia-gpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c
> index 12e330cd7635..0934f8ad7f49 100644
> --- a/drivers/i2c/busses/i2c-nvidia-gpu.c
> +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
> @@ -260,7 +260,7 @@ MODULE_DEVICE_TABLE(pci, gpu_i2c_ids);
>
> static const struct property_entry ccgx_props[] = {
> /* Use FW built for NVIDIA (nv) only */
> - PROPERTY_ENTRY_U16("ccgx,firmware-build", ('n' << 8) | 'v'),
> + PROPERTY_ENTRY_U16("cypress,firmware-build", ('n' << 8) | 'v'),
> { }
> };
>
Could we change this to be PROPERTY_ENTRY_STRING instead of
PROPERTY_ENTRY_U16? The benefit of this would be to allow us to use a
proper string identifier in device-tree for the Tegra devices which is
more flexible and descriptive.
So given that this is for NVIDIA GPUs, we could simply make this ...
PROPERTY_ENTRY_STRING("cypress,firmware-build", "nvidia,gpu"),
Then in the ucsi_ccg.c driver, when we read the device property
"cypress,firmware-build", if it is 'nvidia,gpu' then we set the
uc->fw_build variable to CCG_FW_BUILD_NVIDIA.
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 07/11] i2c: nvidia-gpu: Replace ccgx to well-known regex
2022-11-01 15:07 ` Jon Hunter
@ 2022-11-03 11:36 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-11-03 11:36 UTC (permalink / raw)
To: Jonathan Hunter, gregkh, robh+dt, krzysztof.kozlowski+dt,
Thierry Reding, thierry.reding, heikki.krogerus, Ajay Gupta,
kishon, vkoul, p.zabel, balbi, mathias.nyman, Jui Chang Kuo
Cc: linux-usb, devicetree, linux-kernel, Sing-Han Chen, linux-i2c,
linux-phy, linux-tegra
On 11/1/22 23:07, Jon Hunter wrote:
>
> On 24/10/2022 08:41, Wayne Chang wrote:
>> ccgx is refer to the cypress cypd4226 typec controller.
>> Replace ccgx to well-known regex "cypress".
>>
>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>> ---
>> drivers/i2c/busses/i2c-nvidia-gpu.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c
>> b/drivers/i2c/busses/i2c-nvidia-gpu.c
>> index 12e330cd7635..0934f8ad7f49 100644
>> --- a/drivers/i2c/busses/i2c-nvidia-gpu.c
>> +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
>> @@ -260,7 +260,7 @@ MODULE_DEVICE_TABLE(pci, gpu_i2c_ids);
>> static const struct property_entry ccgx_props[] = {
>> /* Use FW built for NVIDIA (nv) only */
>> - PROPERTY_ENTRY_U16("ccgx,firmware-build", ('n' << 8) | 'v'),
>> + PROPERTY_ENTRY_U16("cypress,firmware-build", ('n' << 8) | 'v'),
>> { }
>> };
>
>
> Could we change this to be PROPERTY_ENTRY_STRING instead of
> PROPERTY_ENTRY_U16? The benefit of this would be to allow us to use a
> proper string identifier in device-tree for the Tegra devices which is
> more flexible and descriptive.
>
> So given that this is for NVIDIA GPUs, we could simply make this ...
>
> PROPERTY_ENTRY_STRING("cypress,firmware-build", "nvidia,gpu"),
>
> Then in the ucsi_ccg.c driver, when we read the device property
> "cypress,firmware-build", if it is 'nvidia,gpu' then we set the
> uc->fw_build variable to CCG_FW_BUILD_NVIDIA.
>
Yes, it should be nice to have the change.
Thanks for the review.
I'll make the changes and using string instead of u16 in the next patch
series.
thanks,
Wayne.
> Jon
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 08/11] phy: tegra: xusb: Disable trk clk when not using
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
` (6 preceding siblings ...)
2022-10-24 7:41 ` [PATCH 07/11] i2c: nvidia-gpu: " Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-11-05 14:58 ` Vinod Koul
2022-10-24 7:41 ` [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support Wayne Chang
` (2 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
The change fixes an issue that the pad tracking is a one-time calibration
for Tegra186 and Tegra194. We should disable the clk when it is done.
The 100us delay is for HW recording the calibration value.
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
drivers/phy/tegra/xusb-tegra186.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
index 0996ede63387..f121b4ffbbfd 100644
--- a/drivers/phy/tegra/xusb-tegra186.c
+++ b/drivers/phy/tegra/xusb-tegra186.c
@@ -609,6 +609,10 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
value &= ~USB2_PD_TRK;
padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+ udelay(100);
+
+ clk_disable_unprepare(priv->usb2_trk_clk);
+
mutex_unlock(&padctl->lock);
}
@@ -633,8 +637,6 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
value |= USB2_PD_TRK;
padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
- clk_disable_unprepare(priv->usb2_trk_clk);
-
mutex_unlock(&padctl->lock);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 08/11] phy: tegra: xusb: Disable trk clk when not using
2022-10-24 7:41 ` [PATCH 08/11] phy: tegra: xusb: Disable trk clk when not using Wayne Chang
@ 2022-11-05 14:58 ` Vinod Koul
2022-11-07 10:37 ` Wayne Chang
0 siblings, 1 reply; 50+ messages in thread
From: Vinod Koul @ 2022-11-05 14:58 UTC (permalink / raw)
To: Wayne Chang
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On 24-10-22, 15:41, Wayne Chang wrote:
Consider revision of title to: "Disable trk clk when not in use"
> The change fixes an issue that the pad tracking is a one-time calibration
> for Tegra186 and Tegra194. We should disable the clk when it is done.
> The 100us delay is for HW recording the calibration value.
Consider:
"pad tracking is a one-time calibration for Tegra186 and Tegra194. clk
should be disabled after calibration.
Disable clk after claibration.
While at it add 100us delay HW recording the calibration
>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> drivers/phy/tegra/xusb-tegra186.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
> index 0996ede63387..f121b4ffbbfd 100644
> --- a/drivers/phy/tegra/xusb-tegra186.c
> +++ b/drivers/phy/tegra/xusb-tegra186.c
> @@ -609,6 +609,10 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
> value &= ~USB2_PD_TRK;
> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>
> + udelay(100);
> +
> + clk_disable_unprepare(priv->usb2_trk_clk);
> +
> mutex_unlock(&padctl->lock);
> }
>
> @@ -633,8 +637,6 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
> value |= USB2_PD_TRK;
> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>
> - clk_disable_unprepare(priv->usb2_trk_clk);
> -
> mutex_unlock(&padctl->lock);
> }
>
> --
> 2.25.1
--
~Vinod
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 08/11] phy: tegra: xusb: Disable trk clk when not using
2022-11-05 14:58 ` Vinod Koul
@ 2022-11-07 10:37 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-11-07 10:37 UTC (permalink / raw)
To: Vinod Koul
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, Thierry Reding,
Jonathan Hunter, thierry.reding, heikki.krogerus, Ajay Gupta,
kishon, p.zabel, balbi, mathias.nyman, Jui Chang Kuo, linux-usb,
devicetree, linux-kernel, Sing-Han Chen, linux-i2c, linux-phy,
linux-tegra
On 11/5/22 22:58, Vinod Koul wrote:
> External email: Use caution opening links or attachments
>
>
> On 24-10-22, 15:41, Wayne Chang wrote:
>
> Consider revision of title to: "Disable trk clk when not in use"
>
>> The change fixes an issue that the pad tracking is a one-time calibration
>> for Tegra186 and Tegra194. We should disable the clk when it is done.
>> The 100us delay is for HW recording the calibration value.
>
> Consider:
>
> "pad tracking is a one-time calibration for Tegra186 and Tegra194. clk
> should be disabled after calibration.
>
> Disable clk after claibration.
>
> While at it add 100us delay HW recording the calibration
>
Thanks for the review.
I'll update the title and commit message in the next patch series.
thanks,
Wayne.
>>
>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>> ---
>> drivers/phy/tegra/xusb-tegra186.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
>> index 0996ede63387..f121b4ffbbfd 100644
>> --- a/drivers/phy/tegra/xusb-tegra186.c
>> +++ b/drivers/phy/tegra/xusb-tegra186.c
>> @@ -609,6 +609,10 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
>> value &= ~USB2_PD_TRK;
>> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>>
>> + udelay(100);
>> +
>> + clk_disable_unprepare(priv->usb2_trk_clk);
>> +
>> mutex_unlock(&padctl->lock);
>> }
>>
>> @@ -633,8 +637,6 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
>> value |= USB2_PD_TRK;
>> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>>
>> - clk_disable_unprepare(priv->usb2_trk_clk);
>> -
>> mutex_unlock(&padctl->lock);
>> }
>>
>> --
>> 2.25.1
>
> --
> ~Vinod
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
` (7 preceding siblings ...)
2022-10-24 7:41 ` [PATCH 08/11] phy: tegra: xusb: Disable trk clk when not using Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-10-28 12:56 ` Thierry Reding
2022-11-05 15:01 ` Vinod Koul
2022-10-24 7:41 ` [PATCH 10/11] usb: host: xhci-tegra: Add Tegra234 XHCI support Wayne Chang
2022-10-24 7:41 ` [PATCH 11/11] usb: gadget: tegra-xudc: Add Tegra234 support Wayne Chang
10 siblings, 2 replies; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
From: Sing-Han Chen <singhanc@nvidia.com>
Add support for the XUSB pad controller found on Tegra234 SoCs. It is
mostly similar to the same IP found on Tegra194, because most of
the Tegra234 XUSB PADCTL registers definition and programming sequence
are the same as Tegra194, Tegra234 XUSB PADCTL can share the same
driver with Tegra186 and Tegra194 XUSB PADCTL.
Introduce a new feature, USB2 HW tracking, for Tegra234.
The feature is to enable HW periodical PAD tracking which measure
and capture the electric parameters of USB2.0 PAD.
Signed-off-by: Sing-Han Chen <singhanc@nvidia.com>
Co-developed-by: Wayne Chang <waynec@nvidia.com>
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
drivers/phy/tegra/Makefile | 1 +
drivers/phy/tegra/xusb-tegra186.c | 65 +++++++++++++++++++++++++++++--
drivers/phy/tegra/xusb.c | 6 +++
drivers/phy/tegra/xusb.h | 23 +++++++++++
4 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile
index 89b84067cb4c..eeeea72de117 100644
--- a/drivers/phy/tegra/Makefile
+++ b/drivers/phy/tegra/Makefile
@@ -7,4 +7,5 @@ phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o
phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o
phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_186_SOC) += xusb-tegra186.o
phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_194_SOC) += xusb-tegra186.o
+phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_234_SOC) += xusb-tegra186.o
obj-$(CONFIG_PHY_TEGRA194_P2U) += phy-tegra194-p2u.o
diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
index f121b4ffbbfd..cc02cea65a21 100644
--- a/drivers/phy/tegra/xusb-tegra186.c
+++ b/drivers/phy/tegra/xusb-tegra186.c
@@ -89,6 +89,11 @@
#define USB2_TRK_START_TIMER(x) (((x) & 0x7f) << 12)
#define USB2_TRK_DONE_RESET_TIMER(x) (((x) & 0x7f) << 19)
#define USB2_PD_TRK BIT(26)
+#define USB2_TRK_COMPLETED BIT(31)
+
+#define XUSB_PADCTL_USB2_BIAS_PAD_CTL2 0x28c
+#define USB2_TRK_HW_MODE BIT(0)
+#define CYA_TRK_CODE_UPDATE_ON_IDLE BIT(31)
#define XUSB_PADCTL_HSIC_PADX_CTL0(x) (0x300 + (x) * 0x20)
#define HSIC_PD_TX_DATA0 BIT(1)
@@ -609,9 +614,32 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
value &= ~USB2_PD_TRK;
padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
- udelay(100);
+ if (padctl->soc->poll_trk_completed) {
+ err = padctl_readl_poll(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1,
+ USB2_TRK_COMPLETED, USB2_TRK_COMPLETED, 100);
+ if (err) {
+ /* The failure with polling on trk complete will not
+ * cause the failure of powering on the bias pad.
+ */
+ dev_warn(dev, "failed to poll USB2 trk completed: %d\n",
+ err);
+ }
- clk_disable_unprepare(priv->usb2_trk_clk);
+ value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+ value |= USB2_TRK_COMPLETED;
+ padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+ } else {
+ udelay(100);
+ }
+
+ if (padctl->soc->trk_hw_mode) {
+ value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
+ value |= USB2_TRK_HW_MODE;
+ value &= ~CYA_TRK_CODE_UPDATE_ON_IDLE;
+ padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
+ } else {
+ clk_disable_unprepare(priv->usb2_trk_clk);
+ }
mutex_unlock(&padctl->lock);
}
@@ -637,6 +665,13 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
value |= USB2_PD_TRK;
padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+ if (padctl->soc->trk_hw_mode) {
+ value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
+ value &= ~USB2_TRK_HW_MODE;
+ padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
+ clk_disable_unprepare(priv->usb2_trk_clk);
+ }
+
mutex_unlock(&padctl->lock);
}
@@ -1560,7 +1595,8 @@ const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc = {
EXPORT_SYMBOL_GPL(tegra186_xusb_padctl_soc);
#endif
-#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC) || \
+ IS_ENABLED(CONFIG_ARCH_TEGRA_234_SOC)
static const char * const tegra194_xusb_padctl_supply_names[] = {
"avdd-usb",
"vclamp-usb",
@@ -1616,8 +1652,31 @@ const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc = {
.supply_names = tegra194_xusb_padctl_supply_names,
.num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
.supports_gen2 = true,
+ .poll_trk_completed = true,
};
EXPORT_SYMBOL_GPL(tegra194_xusb_padctl_soc);
+
+const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc = {
+ .num_pads = ARRAY_SIZE(tegra194_pads),
+ .pads = tegra194_pads,
+ .ports = {
+ .usb2 = {
+ .ops = &tegra186_usb2_port_ops,
+ .count = 4,
+ },
+ .usb3 = {
+ .ops = &tegra186_usb3_port_ops,
+ .count = 4,
+ },
+ },
+ .ops = &tegra186_xusb_padctl_ops,
+ .supply_names = tegra194_xusb_padctl_supply_names,
+ .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
+ .supports_gen2 = true,
+ .poll_trk_completed = true,
+ .trk_hw_mode = true,
+};
+EXPORT_SYMBOL_GPL(tegra234_xusb_padctl_soc);
#endif
MODULE_AUTHOR("JC Kuo <jckuo@nvidia.com>");
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 95091876c422..23d179b1a5b5 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -71,6 +71,12 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
.compatible = "nvidia,tegra194-xusb-padctl",
.data = &tegra194_xusb_padctl_soc,
},
+#endif
+#if defined(CONFIG_ARCH_TEGRA_234_SOC)
+ {
+ .compatible = "nvidia,tegra234-xusb-padctl",
+ .data = &tegra234_xusb_padctl_soc,
+ },
#endif
{ }
};
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index 8cfbbdbd6e0c..ec0b5b023ad1 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -8,6 +8,7 @@
#define __PHY_TEGRA_XUSB_H
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/mutex.h>
#include <linux/workqueue.h>
@@ -433,6 +434,8 @@ struct tegra_xusb_padctl_soc {
unsigned int num_supplies;
bool supports_gen2;
bool need_fake_usb3_port;
+ bool poll_trk_completed;
+ bool trk_hw_mode;
};
struct tegra_xusb_padctl {
@@ -475,6 +478,23 @@ static inline u32 padctl_readl(struct tegra_xusb_padctl *padctl,
return value;
}
+static inline u32 padctl_readl_poll(struct tegra_xusb_padctl *padctl,
+ unsigned long offset, u32 val, u32 mask, int us)
+{
+ u32 regval;
+ int err;
+
+ err = readl_poll_timeout_atomic(padctl->regs + offset, regval,
+ (regval & mask) == val, 1, us);
+ dev_dbg(padctl->dev, "%08lx poll > %08x\n", offset, regval);
+ if (err) {
+ dev_err(padctl->dev, "%08lx poll timeout > %08x\n", offset,
+ regval);
+ }
+
+ return err;
+}
+
struct tegra_xusb_lane *tegra_xusb_find_lane(struct tegra_xusb_padctl *padctl,
const char *name,
unsigned int index);
@@ -491,5 +511,8 @@ extern const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc;
#if defined(CONFIG_ARCH_TEGRA_194_SOC)
extern const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc;
#endif
+#if defined(CONFIG_ARCH_TEGRA_234_SOC)
+extern const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc;
+#endif
#endif /* __PHY_TEGRA_XUSB_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support
2022-10-24 7:41 ` [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support Wayne Chang
@ 2022-10-28 12:56 ` Thierry Reding
2022-11-03 11:42 ` Wayne Chang
2022-11-05 15:01 ` Vinod Koul
1 sibling, 1 reply; 50+ messages in thread
From: Thierry Reding @ 2022-10-28 12:56 UTC (permalink / raw)
To: Wayne Chang
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
heikki.krogerus, ajayg, kishon, vkoul, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 8515 bytes --]
On Mon, Oct 24, 2022 at 03:41:26PM +0800, Wayne Chang wrote:
> From: Sing-Han Chen <singhanc@nvidia.com>
>
> Add support for the XUSB pad controller found on Tegra234 SoCs. It is
> mostly similar to the same IP found on Tegra194, because most of
> the Tegra234 XUSB PADCTL registers definition and programming sequence
> are the same as Tegra194, Tegra234 XUSB PADCTL can share the same
> driver with Tegra186 and Tegra194 XUSB PADCTL.
>
> Introduce a new feature, USB2 HW tracking, for Tegra234.
> The feature is to enable HW periodical PAD tracking which measure
> and capture the electric parameters of USB2.0 PAD.
>
> Signed-off-by: Sing-Han Chen <singhanc@nvidia.com>
> Co-developed-by: Wayne Chang <waynec@nvidia.com>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> drivers/phy/tegra/Makefile | 1 +
> drivers/phy/tegra/xusb-tegra186.c | 65 +++++++++++++++++++++++++++++--
> drivers/phy/tegra/xusb.c | 6 +++
> drivers/phy/tegra/xusb.h | 23 +++++++++++
> 4 files changed, 92 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile
> index 89b84067cb4c..eeeea72de117 100644
> --- a/drivers/phy/tegra/Makefile
> +++ b/drivers/phy/tegra/Makefile
> @@ -7,4 +7,5 @@ phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o
> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o
> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_186_SOC) += xusb-tegra186.o
> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_194_SOC) += xusb-tegra186.o
> +phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_234_SOC) += xusb-tegra186.o
> obj-$(CONFIG_PHY_TEGRA194_P2U) += phy-tegra194-p2u.o
> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
> index f121b4ffbbfd..cc02cea65a21 100644
> --- a/drivers/phy/tegra/xusb-tegra186.c
> +++ b/drivers/phy/tegra/xusb-tegra186.c
> @@ -89,6 +89,11 @@
> #define USB2_TRK_START_TIMER(x) (((x) & 0x7f) << 12)
> #define USB2_TRK_DONE_RESET_TIMER(x) (((x) & 0x7f) << 19)
> #define USB2_PD_TRK BIT(26)
> +#define USB2_TRK_COMPLETED BIT(31)
> +
> +#define XUSB_PADCTL_USB2_BIAS_PAD_CTL2 0x28c
> +#define USB2_TRK_HW_MODE BIT(0)
> +#define CYA_TRK_CODE_UPDATE_ON_IDLE BIT(31)
>
> #define XUSB_PADCTL_HSIC_PADX_CTL0(x) (0x300 + (x) * 0x20)
> #define HSIC_PD_TX_DATA0 BIT(1)
> @@ -609,9 +614,32 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
> value &= ~USB2_PD_TRK;
> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>
> - udelay(100);
> + if (padctl->soc->poll_trk_completed) {
> + err = padctl_readl_poll(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1,
> + USB2_TRK_COMPLETED, USB2_TRK_COMPLETED, 100);
> + if (err) {
> + /* The failure with polling on trk complete will not
> + * cause the failure of powering on the bias pad.
> + */
> + dev_warn(dev, "failed to poll USB2 trk completed: %d\n",
> + err);
> + }
>
> - clk_disable_unprepare(priv->usb2_trk_clk);
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> + value |= USB2_TRK_COMPLETED;
> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> + } else {
> + udelay(100);
> + }
> +
> + if (padctl->soc->trk_hw_mode) {
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
> + value |= USB2_TRK_HW_MODE;
> + value &= ~CYA_TRK_CODE_UPDATE_ON_IDLE;
> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
> + } else {
> + clk_disable_unprepare(priv->usb2_trk_clk);
> + }
>
> mutex_unlock(&padctl->lock);
> }
> @@ -637,6 +665,13 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
> value |= USB2_PD_TRK;
> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>
> + if (padctl->soc->trk_hw_mode) {
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
> + value &= ~USB2_TRK_HW_MODE;
> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
> + clk_disable_unprepare(priv->usb2_trk_clk);
> + }
> +
> mutex_unlock(&padctl->lock);
> }
>
> @@ -1560,7 +1595,8 @@ const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc = {
> EXPORT_SYMBOL_GPL(tegra186_xusb_padctl_soc);
> #endif
>
> -#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC) || \
> + IS_ENABLED(CONFIG_ARCH_TEGRA_234_SOC)
> static const char * const tegra194_xusb_padctl_supply_names[] = {
> "avdd-usb",
> "vclamp-usb",
> @@ -1616,8 +1652,31 @@ const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc = {
> .supply_names = tegra194_xusb_padctl_supply_names,
> .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
> .supports_gen2 = true,
> + .poll_trk_completed = true,
> };
> EXPORT_SYMBOL_GPL(tegra194_xusb_padctl_soc);
> +
> +const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc = {
> + .num_pads = ARRAY_SIZE(tegra194_pads),
> + .pads = tegra194_pads,
> + .ports = {
> + .usb2 = {
> + .ops = &tegra186_usb2_port_ops,
> + .count = 4,
> + },
> + .usb3 = {
> + .ops = &tegra186_usb3_port_ops,
> + .count = 4,
> + },
> + },
> + .ops = &tegra186_xusb_padctl_ops,
> + .supply_names = tegra194_xusb_padctl_supply_names,
> + .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
> + .supports_gen2 = true,
> + .poll_trk_completed = true,
> + .trk_hw_mode = true,
> +};
> +EXPORT_SYMBOL_GPL(tegra234_xusb_padctl_soc);
I'm beginning to wonder if we perhaps went a bit overboard with this.
These symbols are used exclusively by drivers/phy/tegra/xusb.c, which
ends up in the same link unit as xusb-tegra186.c, so the export should
not be necessary.
Not necessarily something that needs fixing right now, but certainly
something to circle back to eventually.
> #endif
>
> MODULE_AUTHOR("JC Kuo <jckuo@nvidia.com>");
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 95091876c422..23d179b1a5b5 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -71,6 +71,12 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
> .compatible = "nvidia,tegra194-xusb-padctl",
> .data = &tegra194_xusb_padctl_soc,
> },
> +#endif
> +#if defined(CONFIG_ARCH_TEGRA_234_SOC)
> + {
> + .compatible = "nvidia,tegra234-xusb-padctl",
> + .data = &tegra234_xusb_padctl_soc,
> + },
> #endif
> { }
> };
> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
> index 8cfbbdbd6e0c..ec0b5b023ad1 100644
> --- a/drivers/phy/tegra/xusb.h
> +++ b/drivers/phy/tegra/xusb.h
> @@ -8,6 +8,7 @@
> #define __PHY_TEGRA_XUSB_H
>
> #include <linux/io.h>
> +#include <linux/iopoll.h>
> #include <linux/mutex.h>
> #include <linux/workqueue.h>
>
> @@ -433,6 +434,8 @@ struct tegra_xusb_padctl_soc {
> unsigned int num_supplies;
> bool supports_gen2;
> bool need_fake_usb3_port;
> + bool poll_trk_completed;
> + bool trk_hw_mode;
> };
>
> struct tegra_xusb_padctl {
> @@ -475,6 +478,23 @@ static inline u32 padctl_readl(struct tegra_xusb_padctl *padctl,
> return value;
> }
>
> +static inline u32 padctl_readl_poll(struct tegra_xusb_padctl *padctl,
> + unsigned long offset, u32 val, u32 mask, int us)
> +{
> + u32 regval;
> + int err;
> +
> + err = readl_poll_timeout_atomic(padctl->regs + offset, regval,
> + (regval & mask) == val, 1, us);
Do we really need the atomic variant here? The function that calls this
already uses a mutex for protection, so it can already sleep anyway.
Also, do we really need the helper here? We use this exactly once and
this doesn't make the invocation more readable, either.
Thierry
> + dev_dbg(padctl->dev, "%08lx poll > %08x\n", offset, regval);
> + if (err) {
> + dev_err(padctl->dev, "%08lx poll timeout > %08x\n", offset,
> + regval);
> + }
> +
> + return err;
> +}
> +
> struct tegra_xusb_lane *tegra_xusb_find_lane(struct tegra_xusb_padctl *padctl,
> const char *name,
> unsigned int index);
> @@ -491,5 +511,8 @@ extern const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc;
> #if defined(CONFIG_ARCH_TEGRA_194_SOC)
> extern const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc;
> #endif
> +#if defined(CONFIG_ARCH_TEGRA_234_SOC)
> +extern const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc;
> +#endif
>
> #endif /* __PHY_TEGRA_XUSB_H */
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support
2022-10-28 12:56 ` Thierry Reding
@ 2022-11-03 11:42 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-11-03 11:42 UTC (permalink / raw)
To: Thierry Reding
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, Thierry Reding,
Jonathan Hunter, heikki.krogerus, Ajay Gupta, kishon, vkoul,
p.zabel, balbi, mathias.nyman, Jui Chang Kuo, linux-usb,
devicetree, linux-kernel, Sing-Han Chen, linux-i2c, linux-phy,
linux-tegra
On 10/28/22 20:56, Thierry Reding wrote:
> On Mon, Oct 24, 2022 at 03:41:26PM +0800, Wayne Chang wrote:
>> From: Sing-Han Chen<singhanc@nvidia.com>
>>
>> Add support for the XUSB pad controller found on Tegra234 SoCs. It is
>> mostly similar to the same IP found on Tegra194, because most of
>> the Tegra234 XUSB PADCTL registers definition and programming sequence
>> are the same as Tegra194, Tegra234 XUSB PADCTL can share the same
>> driver with Tegra186 and Tegra194 XUSB PADCTL.
>>
>> Introduce a new feature, USB2 HW tracking, for Tegra234.
>> The feature is to enable HW periodical PAD tracking which measure
>> and capture the electric parameters of USB2.0 PAD.
>>
>> Signed-off-by: Sing-Han Chen<singhanc@nvidia.com>
>> Co-developed-by: Wayne Chang<waynec@nvidia.com>
>> Signed-off-by: Wayne Chang<waynec@nvidia.com>
>> ---
>> drivers/phy/tegra/Makefile | 1 +
>> drivers/phy/tegra/xusb-tegra186.c | 65 +++++++++++++++++++++++++++++--
>> drivers/phy/tegra/xusb.c | 6 +++
>> drivers/phy/tegra/xusb.h | 23 +++++++++++
>> 4 files changed, 92 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile
>> index 89b84067cb4c..eeeea72de117 100644
>> --- a/drivers/phy/tegra/Makefile
>> +++ b/drivers/phy/tegra/Makefile
>> @@ -7,4 +7,5 @@ phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o
>> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o
>> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_186_SOC) += xusb-tegra186.o
>> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_194_SOC) += xusb-tegra186.o
>> +phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_234_SOC) += xusb-tegra186.o
>> obj-$(CONFIG_PHY_TEGRA194_P2U) += phy-tegra194-p2u.o
>> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
>> index f121b4ffbbfd..cc02cea65a21 100644
>> --- a/drivers/phy/tegra/xusb-tegra186.c
>> +++ b/drivers/phy/tegra/xusb-tegra186.c
>> @@ -89,6 +89,11 @@
>> #define USB2_TRK_START_TIMER(x) (((x) & 0x7f) << 12)
>> #define USB2_TRK_DONE_RESET_TIMER(x) (((x) & 0x7f) << 19)
>> #define USB2_PD_TRK BIT(26)
>> +#define USB2_TRK_COMPLETED BIT(31)
>> +
>> +#define XUSB_PADCTL_USB2_BIAS_PAD_CTL2 0x28c
>> +#define USB2_TRK_HW_MODE BIT(0)
>> +#define CYA_TRK_CODE_UPDATE_ON_IDLE BIT(31)
>>
>> #define XUSB_PADCTL_HSIC_PADX_CTL0(x) (0x300 + (x) * 0x20)
>> #define HSIC_PD_TX_DATA0 BIT(1)
>> @@ -609,9 +614,32 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
>> value &= ~USB2_PD_TRK;
>> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>>
>> - udelay(100);
>> + if (padctl->soc->poll_trk_completed) {
>> + err = padctl_readl_poll(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1,
>> + USB2_TRK_COMPLETED, USB2_TRK_COMPLETED, 100);
>> + if (err) {
>> + /* The failure with polling on trk complete will not
>> + * cause the failure of powering on the bias pad.
>> + */
>> + dev_warn(dev, "failed to poll USB2 trk completed: %d\n",
>> + err);
>> + }
>>
>> - clk_disable_unprepare(priv->usb2_trk_clk);
>> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> + value |= USB2_TRK_COMPLETED;
>> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> + } else {
>> + udelay(100);
>> + }
>> +
>> + if (padctl->soc->trk_hw_mode) {
>> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
>> + value |= USB2_TRK_HW_MODE;
>> + value &= ~CYA_TRK_CODE_UPDATE_ON_IDLE;
>> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
>> + } else {
>> + clk_disable_unprepare(priv->usb2_trk_clk);
>> + }
>>
>> mutex_unlock(&padctl->lock);
>> }
>> @@ -637,6 +665,13 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
>> value |= USB2_PD_TRK;
>> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>>
>> + if (padctl->soc->trk_hw_mode) {
>> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
>> + value &= ~USB2_TRK_HW_MODE;
>> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
>> + clk_disable_unprepare(priv->usb2_trk_clk);
>> + }
>> +
>> mutex_unlock(&padctl->lock);
>> }
>>
>> @@ -1560,7 +1595,8 @@ const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc = {
>> EXPORT_SYMBOL_GPL(tegra186_xusb_padctl_soc);
>> #endif
>>
>> -#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
>> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC) || \
>> + IS_ENABLED(CONFIG_ARCH_TEGRA_234_SOC)
>> static const char * const tegra194_xusb_padctl_supply_names[] = {
>> "avdd-usb",
>> "vclamp-usb",
>> @@ -1616,8 +1652,31 @@ const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc = {
>> .supply_names = tegra194_xusb_padctl_supply_names,
>> .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
>> .supports_gen2 = true,
>> + .poll_trk_completed = true,
>> };
>> EXPORT_SYMBOL_GPL(tegra194_xusb_padctl_soc);
>> +
>> +const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc = {
>> + .num_pads = ARRAY_SIZE(tegra194_pads),
>> + .pads = tegra194_pads,
>> + .ports = {
>> + .usb2 = {
>> + .ops = &tegra186_usb2_port_ops,
>> + .count = 4,
>> + },
>> + .usb3 = {
>> + .ops = &tegra186_usb3_port_ops,
>> + .count = 4,
>> + },
>> + },
>> + .ops = &tegra186_xusb_padctl_ops,
>> + .supply_names = tegra194_xusb_padctl_supply_names,
>> + .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
>> + .supports_gen2 = true,
>> + .poll_trk_completed = true,
>> + .trk_hw_mode = true,
>> +};
>> +EXPORT_SYMBOL_GPL(tegra234_xusb_padctl_soc);
> I'm beginning to wonder if we perhaps went a bit overboard with this.
> These symbols are used exclusively by drivers/phy/tegra/xusb.c, which
> ends up in the same link unit as xusb-tegra186.c, so the export should
> not be necessary.
>
> Not necessarily something that needs fixing right now, but certainly
> something to circle back to eventually.
Yes, exactly.
OK. We will refactor it the next time.
Thanks for the review.
>
>> #endif
>>
>> MODULE_AUTHOR("JC Kuo<jckuo@nvidia.com>");
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index 95091876c422..23d179b1a5b5 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -71,6 +71,12 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
>> .compatible = "nvidia,tegra194-xusb-padctl",
>> .data = &tegra194_xusb_padctl_soc,
>> },
>> +#endif
>> +#if defined(CONFIG_ARCH_TEGRA_234_SOC)
>> + {
>> + .compatible = "nvidia,tegra234-xusb-padctl",
>> + .data = &tegra234_xusb_padctl_soc,
>> + },
>> #endif
>> { }
>> };
>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index 8cfbbdbd6e0c..ec0b5b023ad1 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -8,6 +8,7 @@
>> #define __PHY_TEGRA_XUSB_H
>>
>> #include <linux/io.h>
>> +#include <linux/iopoll.h>
>> #include <linux/mutex.h>
>> #include <linux/workqueue.h>
>>
>> @@ -433,6 +434,8 @@ struct tegra_xusb_padctl_soc {
>> unsigned int num_supplies;
>> bool supports_gen2;
>> bool need_fake_usb3_port;
>> + bool poll_trk_completed;
>> + bool trk_hw_mode;
>> };
>>
>> struct tegra_xusb_padctl {
>> @@ -475,6 +478,23 @@ static inline u32 padctl_readl(struct tegra_xusb_padctl *padctl,
>> return value;
>> }
>>
>> +static inline u32 padctl_readl_poll(struct tegra_xusb_padctl *padctl,
>> + unsigned long offset, u32 val, u32 mask, int us)
>> +{
>> + u32 regval;
>> + int err;
>> +
>> + err = readl_poll_timeout_atomic(padctl->regs + offset, regval,
>> + (regval & mask) == val, 1, us);
> Do we really need the atomic variant here? The function that calls this
> already uses a mutex for protection, so it can already sleep anyway.
>
Thanks for the review. No, we don't need it to be atomic.
I'll update it in the next patch series.
> Also, do we really need the helper here? We use this exactly once and
> this doesn't make the invocation more readable, either.
>
Not at all. Removed. Thanks.
thanks,
Wayne.
> Thierry
>
>> + dev_dbg(padctl->dev, "%08lx poll > %08x\n", offset, regval);
>> + if (err) {
>> + dev_err(padctl->dev, "%08lx poll timeout > %08x\n", offset,
>> + regval);
>> + }
>> +
>> + return err;
>> +}
>> +
>> struct tegra_xusb_lane *tegra_xusb_find_lane(struct tegra_xusb_padctl *padctl,
>> const char *name,
>> unsigned int index);
>> @@ -491,5 +511,8 @@ extern const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc;
>> #if defined(CONFIG_ARCH_TEGRA_194_SOC)
>> extern const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc;
>> #endif
>> +#if defined(CONFIG_ARCH_TEGRA_234_SOC)
>> +extern const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc;
>> +#endif
>>
>> #endif /* __PHY_TEGRA_XUSB_H */
>> --
>> 2.25.1
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support
2022-10-24 7:41 ` [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support Wayne Chang
2022-10-28 12:56 ` Thierry Reding
@ 2022-11-05 15:01 ` Vinod Koul
2022-11-07 10:36 ` Wayne Chang
1 sibling, 1 reply; 50+ messages in thread
From: Vinod Koul @ 2022-11-05 15:01 UTC (permalink / raw)
To: Wayne Chang
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On 24-10-22, 15:41, Wayne Chang wrote:
> From: Sing-Han Chen <singhanc@nvidia.com>
>
> Add support for the XUSB pad controller found on Tegra234 SoCs. It is
> mostly similar to the same IP found on Tegra194, because most of
> the Tegra234 XUSB PADCTL registers definition and programming sequence
> are the same as Tegra194, Tegra234 XUSB PADCTL can share the same
> driver with Tegra186 and Tegra194 XUSB PADCTL.
>
> Introduce a new feature, USB2 HW tracking, for Tegra234.
> The feature is to enable HW periodical PAD tracking which measure
> and capture the electric parameters of USB2.0 PAD.
why cant this patch be sent separately, are phy patches dependent on
rest..? If not consider splitting per subsystem and sending
independently..
>
> Signed-off-by: Sing-Han Chen <singhanc@nvidia.com>
> Co-developed-by: Wayne Chang <waynec@nvidia.com>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> drivers/phy/tegra/Makefile | 1 +
> drivers/phy/tegra/xusb-tegra186.c | 65 +++++++++++++++++++++++++++++--
> drivers/phy/tegra/xusb.c | 6 +++
> drivers/phy/tegra/xusb.h | 23 +++++++++++
> 4 files changed, 92 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile
> index 89b84067cb4c..eeeea72de117 100644
> --- a/drivers/phy/tegra/Makefile
> +++ b/drivers/phy/tegra/Makefile
> @@ -7,4 +7,5 @@ phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o
> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o
> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_186_SOC) += xusb-tegra186.o
> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_194_SOC) += xusb-tegra186.o
> +phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_234_SOC) += xusb-tegra186.o
> obj-$(CONFIG_PHY_TEGRA194_P2U) += phy-tegra194-p2u.o
> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
> index f121b4ffbbfd..cc02cea65a21 100644
> --- a/drivers/phy/tegra/xusb-tegra186.c
> +++ b/drivers/phy/tegra/xusb-tegra186.c
> @@ -89,6 +89,11 @@
> #define USB2_TRK_START_TIMER(x) (((x) & 0x7f) << 12)
> #define USB2_TRK_DONE_RESET_TIMER(x) (((x) & 0x7f) << 19)
> #define USB2_PD_TRK BIT(26)
> +#define USB2_TRK_COMPLETED BIT(31)
> +
> +#define XUSB_PADCTL_USB2_BIAS_PAD_CTL2 0x28c
> +#define USB2_TRK_HW_MODE BIT(0)
> +#define CYA_TRK_CODE_UPDATE_ON_IDLE BIT(31)
>
> #define XUSB_PADCTL_HSIC_PADX_CTL0(x) (0x300 + (x) * 0x20)
> #define HSIC_PD_TX_DATA0 BIT(1)
> @@ -609,9 +614,32 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
> value &= ~USB2_PD_TRK;
> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>
> - udelay(100);
> + if (padctl->soc->poll_trk_completed) {
> + err = padctl_readl_poll(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1,
> + USB2_TRK_COMPLETED, USB2_TRK_COMPLETED, 100);
> + if (err) {
> + /* The failure with polling on trk complete will not
> + * cause the failure of powering on the bias pad.
> + */
> + dev_warn(dev, "failed to poll USB2 trk completed: %d\n",
> + err);
Single line.. should this be dev_err
> + }
>
> - clk_disable_unprepare(priv->usb2_trk_clk);
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> + value |= USB2_TRK_COMPLETED;
> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> + } else {
> + udelay(100);
> + }
> +
> + if (padctl->soc->trk_hw_mode) {
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
> + value |= USB2_TRK_HW_MODE;
> + value &= ~CYA_TRK_CODE_UPDATE_ON_IDLE;
> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
> + } else {
> + clk_disable_unprepare(priv->usb2_trk_clk);
> + }
>
> mutex_unlock(&padctl->lock);
> }
> @@ -637,6 +665,13 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
> value |= USB2_PD_TRK;
> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>
> + if (padctl->soc->trk_hw_mode) {
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
> + value &= ~USB2_TRK_HW_MODE;
> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
> + clk_disable_unprepare(priv->usb2_trk_clk);
> + }
> +
> mutex_unlock(&padctl->lock);
> }
>
> @@ -1560,7 +1595,8 @@ const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc = {
> EXPORT_SYMBOL_GPL(tegra186_xusb_padctl_soc);
> #endif
>
> -#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC) || \
> + IS_ENABLED(CONFIG_ARCH_TEGRA_234_SOC)
> static const char * const tegra194_xusb_padctl_supply_names[] = {
> "avdd-usb",
> "vclamp-usb",
> @@ -1616,8 +1652,31 @@ const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc = {
> .supply_names = tegra194_xusb_padctl_supply_names,
> .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
> .supports_gen2 = true,
> + .poll_trk_completed = true,
> };
> EXPORT_SYMBOL_GPL(tegra194_xusb_padctl_soc);
> +
> +const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc = {
> + .num_pads = ARRAY_SIZE(tegra194_pads),
> + .pads = tegra194_pads,
> + .ports = {
> + .usb2 = {
> + .ops = &tegra186_usb2_port_ops,
> + .count = 4,
> + },
> + .usb3 = {
> + .ops = &tegra186_usb3_port_ops,
> + .count = 4,
> + },
> + },
> + .ops = &tegra186_xusb_padctl_ops,
> + .supply_names = tegra194_xusb_padctl_supply_names,
> + .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
> + .supports_gen2 = true,
> + .poll_trk_completed = true,
> + .trk_hw_mode = true,
> +};
> +EXPORT_SYMBOL_GPL(tegra234_xusb_padctl_soc);
> #endif
>
> MODULE_AUTHOR("JC Kuo <jckuo@nvidia.com>");
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 95091876c422..23d179b1a5b5 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -71,6 +71,12 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
> .compatible = "nvidia,tegra194-xusb-padctl",
> .data = &tegra194_xusb_padctl_soc,
> },
> +#endif
> +#if defined(CONFIG_ARCH_TEGRA_234_SOC)
> + {
> + .compatible = "nvidia,tegra234-xusb-padctl",
> + .data = &tegra234_xusb_padctl_soc,
> + },
> #endif
> { }
> };
> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
> index 8cfbbdbd6e0c..ec0b5b023ad1 100644
> --- a/drivers/phy/tegra/xusb.h
> +++ b/drivers/phy/tegra/xusb.h
> @@ -8,6 +8,7 @@
> #define __PHY_TEGRA_XUSB_H
>
> #include <linux/io.h>
> +#include <linux/iopoll.h>
> #include <linux/mutex.h>
> #include <linux/workqueue.h>
>
> @@ -433,6 +434,8 @@ struct tegra_xusb_padctl_soc {
> unsigned int num_supplies;
> bool supports_gen2;
> bool need_fake_usb3_port;
> + bool poll_trk_completed;
> + bool trk_hw_mode;
> };
>
> struct tegra_xusb_padctl {
> @@ -475,6 +478,23 @@ static inline u32 padctl_readl(struct tegra_xusb_padctl *padctl,
> return value;
> }
>
> +static inline u32 padctl_readl_poll(struct tegra_xusb_padctl *padctl,
> + unsigned long offset, u32 val, u32 mask, int us)
> +{
> + u32 regval;
> + int err;
> +
> + err = readl_poll_timeout_atomic(padctl->regs + offset, regval,
> + (regval & mask) == val, 1, us);
> + dev_dbg(padctl->dev, "%08lx poll > %08x\n", offset, regval);
> + if (err) {
> + dev_err(padctl->dev, "%08lx poll timeout > %08x\n", offset,
> + regval);
> + }
> +
> + return err;
> +}
> +
> struct tegra_xusb_lane *tegra_xusb_find_lane(struct tegra_xusb_padctl *padctl,
> const char *name,
> unsigned int index);
> @@ -491,5 +511,8 @@ extern const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc;
> #if defined(CONFIG_ARCH_TEGRA_194_SOC)
> extern const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc;
> #endif
> +#if defined(CONFIG_ARCH_TEGRA_234_SOC)
> +extern const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc;
> +#endif
>
> #endif /* __PHY_TEGRA_XUSB_H */
> --
> 2.25.1
--
~Vinod
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support
2022-11-05 15:01 ` Vinod Koul
@ 2022-11-07 10:36 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-11-07 10:36 UTC (permalink / raw)
To: Vinod Koul
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, Thierry Reding,
Jonathan Hunter, thierry.reding, heikki.krogerus, Ajay Gupta,
kishon, p.zabel, balbi, mathias.nyman, Jui Chang Kuo, linux-usb,
devicetree, linux-kernel, Sing-Han Chen, linux-i2c, linux-phy,
linux-tegra
On 11/5/22 23:01, Vinod Koul wrote:
> External email: Use caution opening links or attachments
>
>
> On 24-10-22, 15:41, Wayne Chang wrote:
>> From: Sing-Han Chen <singhanc@nvidia.com>
>>
>> Add support for the XUSB pad controller found on Tegra234 SoCs. It is
>> mostly similar to the same IP found on Tegra194, because most of
>> the Tegra234 XUSB PADCTL registers definition and programming sequence
>> are the same as Tegra194, Tegra234 XUSB PADCTL can share the same
>> driver with Tegra186 and Tegra194 XUSB PADCTL.
>>
>> Introduce a new feature, USB2 HW tracking, for Tegra234.
>> The feature is to enable HW periodical PAD tracking which measure
>> and capture the electric parameters of USB2.0 PAD.
>
> why cant this patch be sent separately, are phy patches dependent on
> rest..? If not consider splitting per subsystem and sending
> independently..
>
Thanks for the review.
Yes, tegra234_xusb_padctl_soc is added to enable Tegra234 features.
If we do not enable USB2 HW tracking, then we shall not have this change
and reuse tegra194_xusb_padctl_soc as well.
The feature is new added for Tegra234 pad control against Tegra194.
>>
>> Signed-off-by: Sing-Han Chen <singhanc@nvidia.com>
>> Co-developed-by: Wayne Chang <waynec@nvidia.com>
>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>> ---
>> drivers/phy/tegra/Makefile | 1 +
>> drivers/phy/tegra/xusb-tegra186.c | 65 +++++++++++++++++++++++++++++--
>> drivers/phy/tegra/xusb.c | 6 +++
>> drivers/phy/tegra/xusb.h | 23 +++++++++++
>> 4 files changed, 92 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile
>> index 89b84067cb4c..eeeea72de117 100644
>> --- a/drivers/phy/tegra/Makefile
>> +++ b/drivers/phy/tegra/Makefile
>> @@ -7,4 +7,5 @@ phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o
>> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o
>> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_186_SOC) += xusb-tegra186.o
>> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_194_SOC) += xusb-tegra186.o
>> +phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_234_SOC) += xusb-tegra186.o
>> obj-$(CONFIG_PHY_TEGRA194_P2U) += phy-tegra194-p2u.o
>> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
>> index f121b4ffbbfd..cc02cea65a21 100644
>> --- a/drivers/phy/tegra/xusb-tegra186.c
>> +++ b/drivers/phy/tegra/xusb-tegra186.c
>> @@ -89,6 +89,11 @@
>> #define USB2_TRK_START_TIMER(x) (((x) & 0x7f) << 12)
>> #define USB2_TRK_DONE_RESET_TIMER(x) (((x) & 0x7f) << 19)
>> #define USB2_PD_TRK BIT(26)
>> +#define USB2_TRK_COMPLETED BIT(31)
>> +
>> +#define XUSB_PADCTL_USB2_BIAS_PAD_CTL2 0x28c
>> +#define USB2_TRK_HW_MODE BIT(0)
>> +#define CYA_TRK_CODE_UPDATE_ON_IDLE BIT(31)
>>
>> #define XUSB_PADCTL_HSIC_PADX_CTL0(x) (0x300 + (x) * 0x20)
>> #define HSIC_PD_TX_DATA0 BIT(1)
>> @@ -609,9 +614,32 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
>> value &= ~USB2_PD_TRK;
>> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>>
>> - udelay(100);
>> + if (padctl->soc->poll_trk_completed) {
>> + err = padctl_readl_poll(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1,
>> + USB2_TRK_COMPLETED, USB2_TRK_COMPLETED, 100);
>> + if (err) {
>> + /* The failure with polling on trk complete will not
>> + * cause the failure of powering on the bias pad.
>> + */
>> + dev_warn(dev, "failed to poll USB2 trk completed: %d\n",
>> + err);
>
> Single line.. should this be dev_err
Thanks, I'll make it a single line.
The failure with polling on trk complete will not cause the failure of
powering on the bias pad and thus we put it warning here.
thanks,
Wayne.
>
>> + }
>>
>> - clk_disable_unprepare(priv->usb2_trk_clk);
>> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> + value |= USB2_TRK_COMPLETED;
>> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> + } else {
>> + udelay(100);
>> + }
>> +
>> + if (padctl->soc->trk_hw_mode) {
>> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
>> + value |= USB2_TRK_HW_MODE;
>> + value &= ~CYA_TRK_CODE_UPDATE_ON_IDLE;
>> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
>> + } else {
>> + clk_disable_unprepare(priv->usb2_trk_clk);
>> + }
>>
>> mutex_unlock(&padctl->lock);
>> }
>> @@ -637,6 +665,13 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
>> value |= USB2_PD_TRK;
>> padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>>
>> + if (padctl->soc->trk_hw_mode) {
>> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
>> + value &= ~USB2_TRK_HW_MODE;
>> + padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL2);
>> + clk_disable_unprepare(priv->usb2_trk_clk);
>> + }
>> +
>> mutex_unlock(&padctl->lock);
>> }
>>
>> @@ -1560,7 +1595,8 @@ const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc = {
>> EXPORT_SYMBOL_GPL(tegra186_xusb_padctl_soc);
>> #endif
>>
>> -#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
>> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC) || \
>> + IS_ENABLED(CONFIG_ARCH_TEGRA_234_SOC)
>> static const char * const tegra194_xusb_padctl_supply_names[] = {
>> "avdd-usb",
>> "vclamp-usb",
>> @@ -1616,8 +1652,31 @@ const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc = {
>> .supply_names = tegra194_xusb_padctl_supply_names,
>> .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
>> .supports_gen2 = true,
>> + .poll_trk_completed = true,
>> };
>> EXPORT_SYMBOL_GPL(tegra194_xusb_padctl_soc);
>> +
>> +const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc = {
>> + .num_pads = ARRAY_SIZE(tegra194_pads),
>> + .pads = tegra194_pads,
>> + .ports = {
>> + .usb2 = {
>> + .ops = &tegra186_usb2_port_ops,
>> + .count = 4,
>> + },
>> + .usb3 = {
>> + .ops = &tegra186_usb3_port_ops,
>> + .count = 4,
>> + },
>> + },
>> + .ops = &tegra186_xusb_padctl_ops,
>> + .supply_names = tegra194_xusb_padctl_supply_names,
>> + .num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
>> + .supports_gen2 = true,
>> + .poll_trk_completed = true,
>> + .trk_hw_mode = true,
>> +};
>> +EXPORT_SYMBOL_GPL(tegra234_xusb_padctl_soc);
>> #endif
>>
>> MODULE_AUTHOR("JC Kuo <jckuo@nvidia.com>");
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index 95091876c422..23d179b1a5b5 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -71,6 +71,12 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
>> .compatible = "nvidia,tegra194-xusb-padctl",
>> .data = &tegra194_xusb_padctl_soc,
>> },
>> +#endif
>> +#if defined(CONFIG_ARCH_TEGRA_234_SOC)
>> + {
>> + .compatible = "nvidia,tegra234-xusb-padctl",
>> + .data = &tegra234_xusb_padctl_soc,
>> + },
>> #endif
>> { }
>> };
>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index 8cfbbdbd6e0c..ec0b5b023ad1 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -8,6 +8,7 @@
>> #define __PHY_TEGRA_XUSB_H
>>
>> #include <linux/io.h>
>> +#include <linux/iopoll.h>
>> #include <linux/mutex.h>
>> #include <linux/workqueue.h>
>>
>> @@ -433,6 +434,8 @@ struct tegra_xusb_padctl_soc {
>> unsigned int num_supplies;
>> bool supports_gen2;
>> bool need_fake_usb3_port;
>> + bool poll_trk_completed;
>> + bool trk_hw_mode;
>> };
>>
>> struct tegra_xusb_padctl {
>> @@ -475,6 +478,23 @@ static inline u32 padctl_readl(struct tegra_xusb_padctl *padctl,
>> return value;
>> }
>>
>> +static inline u32 padctl_readl_poll(struct tegra_xusb_padctl *padctl,
>> + unsigned long offset, u32 val, u32 mask, int us)
>> +{
>> + u32 regval;
>> + int err;
>> +
>> + err = readl_poll_timeout_atomic(padctl->regs + offset, regval,
>> + (regval & mask) == val, 1, us);
>> + dev_dbg(padctl->dev, "%08lx poll > %08x\n", offset, regval);
>> + if (err) {
>> + dev_err(padctl->dev, "%08lx poll timeout > %08x\n", offset,
>> + regval);
>> + }
>> +
>> + return err;
>> +}
>> +
>> struct tegra_xusb_lane *tegra_xusb_find_lane(struct tegra_xusb_padctl *padctl,
>> const char *name,
>> unsigned int index);
>> @@ -491,5 +511,8 @@ extern const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc;
>> #if defined(CONFIG_ARCH_TEGRA_194_SOC)
>> extern const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc;
>> #endif
>> +#if defined(CONFIG_ARCH_TEGRA_234_SOC)
>> +extern const struct tegra_xusb_padctl_soc tegra234_xusb_padctl_soc;
>> +#endif
>>
>> #endif /* __PHY_TEGRA_XUSB_H */
>> --
>> 2.25.1
>
> --
> ~Vinod
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 10/11] usb: host: xhci-tegra: Add Tegra234 XHCI support
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
` (8 preceding siblings ...)
2022-10-24 7:41 ` [PATCH 09/11] phy: tegra: xusb: Add Tegra234 support Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
2022-10-28 13:39 ` Thierry Reding
2022-10-24 7:41 ` [PATCH 11/11] usb: gadget: tegra-xudc: Add Tegra234 support Wayne Chang
10 siblings, 1 reply; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
From: Sing-Han Chen <singhanc@nvidia.com>
This change adds Tegra234 XUSB host mode controller support.
In Tegra234, some of the registers have moved to bar2 space.
The new soc variable has_bar2 indicates the chip with bar2
area. This patch adds new reg helper to let the driver reuse
the same code for those chips with bar2 support.
The new soc variables has_ifr indicates the chip with IFR FW
loading support. IFR registers would be configured in
MB1, and FW loading will be triggered in MB2.
Signed-off-by: Sing-Han Chen <singhanc@nvidia.com>
Co-developed-by: Wayne Chang <waynec@nvidia.com>
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
drivers/usb/host/xhci-tegra.c | 277 +++++++++++++++++++++++++++++-----
1 file changed, 237 insertions(+), 40 deletions(-)
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index bdb776553826..86036eeece43 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -44,6 +44,9 @@
#define XUSB_CFG_4 0x010
#define XUSB_BASE_ADDR_SHIFT 15
#define XUSB_BASE_ADDR_MASK 0x1ffff
+#define XUSB_CFG_7 0x01c
+#define XUSB_BASE2_ADDR_SHIFT 16
+#define XUSB_BASE2_ADDR_MASK 0xffff
#define XUSB_CFG_16 0x040
#define XUSB_CFG_24 0x060
#define XUSB_CFG_AXI_CFG 0x0f8
@@ -75,6 +78,20 @@
#define MBOX_SMI_INTR_FW_HANG BIT(1)
#define MBOX_SMI_INTR_EN BIT(3)
+/* BAR2 registers */
+#define XUSB_BAR2_ARU_MBOX_CMD 0x004
+#define XUSB_BAR2_ARU_MBOX_DATA_IN 0x008
+#define XUSB_BAR2_ARU_MBOX_DATA_OUT 0x00c
+#define XUSB_BAR2_ARU_MBOX_OWNER 0x010
+#define XUSB_BAR2_ARU_SMI_INTR 0x014
+#define XUSB_BAR2_ARU_SMI_ARU_FW_SCRATCH_DATA0 0x01c
+#define XUSB_BAR2_ARU_IFRDMA_CFG0 0x0e0
+#define XUSB_BAR2_ARU_IFRDMA_CFG1 0x0e4
+#define XUSB_BAR2_ARU_IFRDMA_STREAMID_FIELD 0x0e8
+#define XUSB_BAR2_ARU_C11_CSBRANGE 0x9c
+#define XUSB_BAR2_ARU_FW_SCRATCH 0x1000
+#define XUSB_BAR2_CSB_BASE_ADDR 0x2000
+
/* IPFS registers */
#define IPFS_XUSB_HOST_MSI_BAR_SZ_0 0x0c0
#define IPFS_XUSB_HOST_MSI_AXI_BAR_ST_0 0x0c4
@@ -111,6 +128,9 @@
#define IMFILLRNG1_TAG_HI_SHIFT 16
#define XUSB_FALC_IMFILLCTL 0x158
+/* CSB ARU registers */
+#define XUSB_CSB_ARU_SCRATCH0 0x100100
+
/* MP CSB registers */
#define XUSB_CSB_MP_ILOAD_ATTR 0x101a00
#define XUSB_CSB_MP_ILOAD_BASE_LO 0x101a04
@@ -131,6 +151,9 @@
#define IMEM_BLOCK_SIZE 256
+#define FW_IOCTL_TYPE_SHIFT (24)
+#define FW_IOCTL_CFGTBL_READ (17)
+
struct tegra_xusb_fw_header {
__le32 boot_loadaddr_in_imem;
__le32 boot_codedfi_offset;
@@ -175,6 +198,7 @@ struct tegra_xusb_mbox_regs {
u16 data_in;
u16 data_out;
u16 owner;
+ u16 smi_intr;
};
struct tegra_xusb_context_soc {
@@ -189,6 +213,7 @@ struct tegra_xusb_context_soc {
} fpci;
};
+struct tegra_xusb_soc_ops;
struct tegra_xusb_soc {
const char *firmware;
const char * const *supply_names;
@@ -205,11 +230,15 @@ struct tegra_xusb_soc {
} ports;
struct tegra_xusb_mbox_regs mbox;
+ struct tegra_xusb_soc_ops *ops;
bool scale_ss_clock;
bool has_ipfs;
bool lpm_support;
bool otg_reset_sspi;
+
+ bool has_bar2;
+ bool has_ifr;
};
struct tegra_xusb_context {
@@ -230,6 +259,8 @@ struct tegra_xusb {
void __iomem *ipfs_base;
void __iomem *fpci_base;
+ void __iomem *bar2_base;
+ resource_size_t bar2_start;
const struct tegra_xusb_soc *soc;
@@ -276,6 +307,17 @@ struct tegra_xusb {
struct tegra_xusb_context context;
};
+struct tegra_xusb_soc_ops {
+ u32 (*mbox_reg_readl)(struct tegra_xusb *tegra,
+ unsigned int offset);
+ void (*mbox_reg_writel)(struct tegra_xusb *tegra,
+ u32 value, unsigned int offset);
+ u32 (*csb_reg_readl)(struct tegra_xusb *tegra,
+ unsigned int offset);
+ void (*csb_reg_writel)(struct tegra_xusb *tegra,
+ u32 value, unsigned int offset);
+};
+
static struct hc_driver __read_mostly tegra_xhci_hc_driver;
static inline u32 fpci_readl(struct tegra_xusb *tegra, unsigned int offset)
@@ -300,7 +342,33 @@ static inline void ipfs_writel(struct tegra_xusb *tegra, u32 value,
writel(value, tegra->ipfs_base + offset);
}
+static inline u32 bar2_readl(struct tegra_xusb *tegra, unsigned int offset)
+{
+ return readl(tegra->bar2_base + offset);
+}
+
+static inline void bar2_writel(struct tegra_xusb *tegra, u32 value,
+ unsigned int offset)
+{
+ writel(value, tegra->bar2_base + offset);
+}
+
static u32 csb_readl(struct tegra_xusb *tegra, unsigned int offset)
+{
+ struct tegra_xusb_soc_ops *ops = tegra->soc->ops;
+
+ return ops->csb_reg_readl(tegra, offset);
+}
+
+static void csb_writel(struct tegra_xusb *tegra, u32 value,
+ unsigned int offset)
+{
+ struct tegra_xusb_soc_ops *ops = tegra->soc->ops;
+
+ ops->csb_reg_writel(tegra, value, offset);
+}
+
+static u32 fpci_csb_readl(struct tegra_xusb *tegra, unsigned int offset)
{
u32 page = CSB_PAGE_SELECT(offset);
u32 ofs = CSB_PAGE_OFFSET(offset);
@@ -310,7 +378,7 @@ static u32 csb_readl(struct tegra_xusb *tegra, unsigned int offset)
return fpci_readl(tegra, XUSB_CFG_CSB_BASE_ADDR + ofs);
}
-static void csb_writel(struct tegra_xusb *tegra, u32 value,
+static void fpci_csb_writel(struct tegra_xusb *tegra, u32 value,
unsigned int offset)
{
u32 page = CSB_PAGE_SELECT(offset);
@@ -320,6 +388,26 @@ static void csb_writel(struct tegra_xusb *tegra, u32 value,
fpci_writel(tegra, value, XUSB_CFG_CSB_BASE_ADDR + ofs);
}
+static u32 bar2_csb_readl(struct tegra_xusb *tegra, unsigned int offset)
+{
+ u32 page = CSB_PAGE_SELECT(offset);
+ u32 ofs = CSB_PAGE_OFFSET(offset);
+
+ bar2_writel(tegra, page, XUSB_BAR2_ARU_C11_CSBRANGE);
+
+ return bar2_readl(tegra, XUSB_BAR2_CSB_BASE_ADDR + ofs);
+}
+
+static void bar2_csb_writel(struct tegra_xusb *tegra, u32 value,
+ unsigned int offset)
+{
+ u32 page = CSB_PAGE_SELECT(offset);
+ u32 ofs = CSB_PAGE_OFFSET(offset);
+
+ bar2_writel(tegra, page, XUSB_BAR2_ARU_C11_CSBRANGE);
+ bar2_writel(tegra, value, XUSB_BAR2_CSB_BASE_ADDR + ofs);
+}
+
static int tegra_xusb_set_ss_clk(struct tegra_xusb *tegra,
unsigned long rate)
{
@@ -451,6 +539,7 @@ static bool tegra_xusb_mbox_cmd_requires_ack(enum tegra_xusb_mbox_cmd cmd)
static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
const struct tegra_xusb_mbox_msg *msg)
{
+ struct tegra_xusb_soc_ops *ops = tegra->soc->ops;
bool wait_for_idle = false;
u32 value;
@@ -459,15 +548,15 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
* ACK/NAK messages.
*/
if (!(msg->cmd == MBOX_CMD_ACK || msg->cmd == MBOX_CMD_NAK)) {
- value = fpci_readl(tegra, tegra->soc->mbox.owner);
+ value = ops->mbox_reg_readl(tegra, tegra->soc->mbox.owner);
if (value != MBOX_OWNER_NONE) {
dev_err(tegra->dev, "mailbox is busy\n");
return -EBUSY;
}
- fpci_writel(tegra, MBOX_OWNER_SW, tegra->soc->mbox.owner);
+ ops->mbox_reg_writel(tegra, MBOX_OWNER_SW, tegra->soc->mbox.owner);
- value = fpci_readl(tegra, tegra->soc->mbox.owner);
+ value = ops->mbox_reg_readl(tegra, tegra->soc->mbox.owner);
if (value != MBOX_OWNER_SW) {
dev_err(tegra->dev, "failed to acquire mailbox\n");
return -EBUSY;
@@ -477,17 +566,17 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
}
value = tegra_xusb_mbox_pack(msg);
- fpci_writel(tegra, value, tegra->soc->mbox.data_in);
+ ops->mbox_reg_writel(tegra, value, tegra->soc->mbox.data_in);
- value = fpci_readl(tegra, tegra->soc->mbox.cmd);
+ value = ops->mbox_reg_readl(tegra, tegra->soc->mbox.cmd);
value |= MBOX_INT_EN | MBOX_DEST_FALC;
- fpci_writel(tegra, value, tegra->soc->mbox.cmd);
+ ops->mbox_reg_writel(tegra, value, tegra->soc->mbox.cmd);
if (wait_for_idle) {
unsigned long timeout = jiffies + msecs_to_jiffies(250);
while (time_before(jiffies, timeout)) {
- value = fpci_readl(tegra, tegra->soc->mbox.owner);
+ value = ops->mbox_reg_readl(tegra, tegra->soc->mbox.owner);
if (value == MBOX_OWNER_NONE)
break;
@@ -495,7 +584,7 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
}
if (time_after(jiffies, timeout))
- value = fpci_readl(tegra, tegra->soc->mbox.owner);
+ value = ops->mbox_reg_readl(tegra, tegra->soc->mbox.owner);
if (value != MBOX_OWNER_NONE)
return -ETIMEDOUT;
@@ -507,11 +596,12 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
static irqreturn_t tegra_xusb_mbox_irq(int irq, void *data)
{
struct tegra_xusb *tegra = data;
+ struct tegra_xusb_soc_ops *ops = tegra->soc->ops;
u32 value;
/* clear mailbox interrupts */
- value = fpci_readl(tegra, XUSB_CFG_ARU_SMI_INTR);
- fpci_writel(tegra, value, XUSB_CFG_ARU_SMI_INTR);
+ value = ops->mbox_reg_readl(tegra, tegra->soc->mbox.smi_intr);
+ ops->mbox_reg_writel(tegra, value, tegra->soc->mbox.smi_intr);
if (value & MBOX_SMI_INTR_FW_HANG)
dev_err(tegra->dev, "controller firmware hang\n");
@@ -664,6 +754,7 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
static irqreturn_t tegra_xusb_mbox_thread(int irq, void *data)
{
struct tegra_xusb *tegra = data;
+ struct tegra_xusb_soc_ops *ops = tegra->soc->ops;
struct tegra_xusb_mbox_msg msg;
u32 value;
@@ -672,16 +763,16 @@ static irqreturn_t tegra_xusb_mbox_thread(int irq, void *data)
if (pm_runtime_suspended(tegra->dev) || tegra->suspended)
goto out;
- value = fpci_readl(tegra, tegra->soc->mbox.data_out);
+ value = ops->mbox_reg_readl(tegra, tegra->soc->mbox.data_out);
tegra_xusb_mbox_unpack(&msg, value);
- value = fpci_readl(tegra, tegra->soc->mbox.cmd);
+ value = ops->mbox_reg_readl(tegra, tegra->soc->mbox.cmd);
value &= ~MBOX_DEST_SMI;
- fpci_writel(tegra, value, tegra->soc->mbox.cmd);
+ ops->mbox_reg_writel(tegra, value, tegra->soc->mbox.cmd);
/* clear mailbox owner if no ACK/NAK is required */
if (!tegra_xusb_mbox_cmd_requires_ack(msg.cmd))
- fpci_writel(tegra, MBOX_OWNER_NONE, tegra->soc->mbox.owner);
+ ops->mbox_reg_writel(tegra, MBOX_OWNER_NONE, tegra->soc->mbox.owner);
tegra_xusb_mbox_handle(tegra, &msg);
@@ -709,6 +800,15 @@ static void tegra_xusb_config(struct tegra_xusb *tegra)
value |= regs & (XUSB_BASE_ADDR_MASK << XUSB_BASE_ADDR_SHIFT);
fpci_writel(tegra, value, XUSB_CFG_4);
+ /* Program BAR2 space */
+ if (tegra->soc->has_bar2) {
+ value = fpci_readl(tegra, XUSB_CFG_7);
+ value &= ~(XUSB_BASE2_ADDR_MASK << XUSB_BASE2_ADDR_SHIFT);
+ value |= tegra->bar2_start &
+ (XUSB_BASE2_ADDR_MASK << XUSB_BASE2_ADDR_SHIFT);
+ fpci_writel(tegra, value, XUSB_CFG_7);
+ }
+
usleep_range(100, 200);
/* Enable bus master */
@@ -881,21 +981,36 @@ static int tegra_xusb_request_firmware(struct tegra_xusb *tegra)
return 0;
}
+static int tegra_xusb_wait_for_falcon(struct tegra_xusb *tegra)
+{
+ struct xhci_cap_regs __iomem *cap_regs;
+ struct xhci_op_regs __iomem *op_regs;
+ int ret;
+ u32 val;
+
+ cap_regs = tegra->regs;
+ op_regs = tegra->regs + HC_LENGTH(readl(&cap_regs->hc_capbase));
+
+ ret = readl_poll_timeout(&op_regs->status, val, !(val & STS_CNR), 1000, 200000);
+
+ if (ret)
+ dev_err(tegra->dev, "XHCI Controller not ready. Falcon state: 0x%x\n",
+ csb_readl(tegra, XUSB_FALC_CPUCTL));
+
+ return ret;
+}
+
static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
{
unsigned int code_tag_blocks, code_size_blocks, code_blocks;
- struct xhci_cap_regs __iomem *cap = tegra->regs;
struct tegra_xusb_fw_header *header;
struct device *dev = tegra->dev;
- struct xhci_op_regs __iomem *op;
- unsigned long timeout;
time64_t timestamp;
u64 address;
u32 value;
int err;
header = (struct tegra_xusb_fw_header *)tegra->fw.virt;
- op = tegra->regs + HC_LENGTH(readl(&cap->hc_capbase));
if (csb_readl(tegra, XUSB_CSB_MP_ILOAD_BASE_LO) != 0) {
dev_info(dev, "Firmware already loaded, Falcon state %#x\n",
@@ -968,26 +1083,43 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
/* Boot Falcon CPU and wait for USBSTS_CNR to get cleared. */
csb_writel(tegra, CPUCTL_STARTCPU, XUSB_FALC_CPUCTL);
- timeout = jiffies + msecs_to_jiffies(200);
+ if (tegra_xusb_wait_for_falcon(tegra))
+ return -EIO;
- do {
- value = readl(&op->status);
- if ((value & STS_CNR) == 0)
- break;
+ timestamp = le32_to_cpu(header->fwimg_created_time);
- usleep_range(1000, 2000);
- } while (time_is_after_jiffies(timeout));
+ dev_info(dev, "Firmware timestamp: %ptTs UTC\n", ×tamp);
+
+ return 0;
+}
- value = readl(&op->status);
- if (value & STS_CNR) {
- value = csb_readl(tegra, XUSB_FALC_CPUCTL);
- dev_err(dev, "XHCI controller not read: %#010x\n", value);
+static u32 tegra_xusb_read_firmware_header(struct tegra_xusb *tegra, u32 offset)
+{
+ /*
+ * We only accept reading the firmware config table
+ * The offset should not exceed the fw header structure
+ */
+ if (offset >= sizeof(struct tegra_xusb_fw_header))
+ return 0;
+
+ bar2_writel(tegra, (FW_IOCTL_CFGTBL_READ << FW_IOCTL_TYPE_SHIFT) | offset,
+ XUSB_BAR2_ARU_FW_SCRATCH);
+ return bar2_readl(tegra, XUSB_BAR2_ARU_SMI_ARU_FW_SCRATCH_DATA0);
+}
+
+static int tegra_xusb_init_ifr_firmware(struct tegra_xusb *tegra)
+{
+ time64_t timestamp;
+
+ if (tegra_xusb_wait_for_falcon(tegra))
return -EIO;
- }
- timestamp = le32_to_cpu(header->fwimg_created_time);
+#define offsetof_32(X, Y) ((u8)(offsetof(X, Y) / sizeof(__le32)))
+ timestamp = tegra_xusb_read_firmware_header(tegra,
+ offsetof_32(struct tegra_xusb_fw_header,
+ fwimg_created_time) << 2);
- dev_info(dev, "Firmware timestamp: %ptTs UTC\n", ×tamp);
+ dev_info(tegra->dev, "Firmware timestamp: %ptTs UTC\n", ×tamp);
return 0;
}
@@ -1403,7 +1535,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
struct of_phandle_args args;
struct tegra_xusb *tegra;
struct device_node *np;
- struct resource *regs;
+ struct resource *res, *regs;
struct xhci_hcd *xhci;
unsigned int i, j, k;
struct phy *phy;
@@ -1435,6 +1567,11 @@ static int tegra_xusb_probe(struct platform_device *pdev)
tegra->ipfs_base = devm_platform_ioremap_resource(pdev, 2);
if (IS_ERR(tegra->ipfs_base))
return PTR_ERR(tegra->ipfs_base);
+ } else if (tegra->soc->has_bar2) {
+ tegra->bar2_base = devm_platform_get_and_ioremap_resource(pdev, 2, &res);
+ if (IS_ERR(tegra->bar2_base))
+ return PTR_ERR(tegra->bar2_base);
+ tegra->bar2_start = res->start;
}
tegra->xhci_irq = platform_get_irq(pdev, 0);
@@ -1651,10 +1788,13 @@ static int tegra_xusb_probe(struct platform_device *pdev)
goto disable_phy;
}
- err = tegra_xusb_request_firmware(tegra);
- if (err < 0) {
- dev_err(&pdev->dev, "failed to request firmware: %d\n", err);
- goto disable_phy;
+ if (!tegra->soc->has_ifr) {
+ err = tegra_xusb_request_firmware(tegra);
+ if (err < 0) {
+ dev_err(&pdev->dev,
+ "failed to request firmware: %d\n", err);
+ goto disable_phy;
+ }
}
err = tegra_xusb_unpowergate_partitions(tegra);
@@ -1663,7 +1803,10 @@ static int tegra_xusb_probe(struct platform_device *pdev)
tegra_xusb_config(tegra);
- err = tegra_xusb_load_firmware(tegra);
+ if (tegra->soc->has_ifr)
+ err = tegra_xusb_init_ifr_firmware(tegra);
+ else
+ err = tegra_xusb_load_firmware(tegra);
if (err < 0) {
dev_err(&pdev->dev, "failed to load firmware: %d\n", err);
goto powergate;
@@ -2070,7 +2213,10 @@ static int tegra_xusb_exit_elpg(struct tegra_xusb *tegra, bool runtime)
tegra_xusb_config(tegra);
tegra_xusb_restore_context(tegra);
- err = tegra_xusb_load_firmware(tegra);
+ if (tegra->soc->has_ifr)
+ err = tegra_xusb_init_ifr_firmware(tegra);
+ else
+ err = tegra_xusb_load_firmware(tegra);
if (err < 0) {
dev_err(tegra->dev, "failed to load firmware: %d\n", err);
goto disable_phy;
@@ -2271,6 +2417,13 @@ static const struct tegra_xusb_context_soc tegra124_xusb_context = {
},
};
+static struct tegra_xusb_soc_ops tegra124_ops = {
+ .mbox_reg_readl = &fpci_readl,
+ .mbox_reg_writel = &fpci_writel,
+ .csb_reg_readl = &fpci_csb_readl,
+ .csb_reg_writel = &fpci_csb_writel,
+};
+
static const struct tegra_xusb_soc tegra124_soc = {
.firmware = "nvidia/tegra124/xusb.bin",
.supply_names = tegra124_supply_names,
@@ -2286,11 +2439,13 @@ static const struct tegra_xusb_soc tegra124_soc = {
.scale_ss_clock = true,
.has_ipfs = true,
.otg_reset_sspi = false,
+ .ops = &tegra124_ops,
.mbox = {
.cmd = 0xe4,
.data_in = 0xe8,
.data_out = 0xec,
.owner = 0xf0,
+ .smi_intr = XUSB_CFG_ARU_SMI_INTR,
},
};
MODULE_FIRMWARE("nvidia/tegra124/xusb.bin");
@@ -2322,11 +2477,13 @@ static const struct tegra_xusb_soc tegra210_soc = {
.scale_ss_clock = false,
.has_ipfs = true,
.otg_reset_sspi = true,
+ .ops = &tegra124_ops,
.mbox = {
.cmd = 0xe4,
.data_in = 0xe8,
.data_out = 0xec,
.owner = 0xf0,
+ .smi_intr = XUSB_CFG_ARU_SMI_INTR,
},
};
MODULE_FIRMWARE("nvidia/tegra210/xusb.bin");
@@ -2363,11 +2520,13 @@ static const struct tegra_xusb_soc tegra186_soc = {
.scale_ss_clock = false,
.has_ipfs = false,
.otg_reset_sspi = false,
+ .ops = &tegra124_ops,
.mbox = {
.cmd = 0xe4,
.data_in = 0xe8,
.data_out = 0xec,
.owner = 0xf0,
+ .smi_intr = XUSB_CFG_ARU_SMI_INTR,
},
.lpm_support = true,
};
@@ -2394,21 +2553,59 @@ static const struct tegra_xusb_soc tegra194_soc = {
.scale_ss_clock = false,
.has_ipfs = false,
.otg_reset_sspi = false,
+ .ops = &tegra124_ops,
.mbox = {
.cmd = 0x68,
.data_in = 0x6c,
.data_out = 0x70,
.owner = 0x74,
+ .smi_intr = XUSB_CFG_ARU_SMI_INTR,
},
.lpm_support = true,
};
MODULE_FIRMWARE("nvidia/tegra194/xusb.bin");
+static struct tegra_xusb_soc_ops tegra234_ops = {
+ .mbox_reg_readl = &bar2_readl,
+ .mbox_reg_writel = &bar2_writel,
+ .csb_reg_readl = &bar2_csb_readl,
+ .csb_reg_writel = &bar2_csb_writel,
+};
+
+static const struct tegra_xusb_soc tegra234_soc = {
+ .firmware = "nvidia/tegra234/xusb.bin",
+ .supply_names = tegra194_supply_names,
+ .num_supplies = ARRAY_SIZE(tegra194_supply_names),
+ .phy_types = tegra194_phy_types,
+ .num_types = ARRAY_SIZE(tegra194_phy_types),
+ .context = &tegra186_xusb_context,
+ .ports = {
+ .usb3 = { .offset = 0, .count = 4, },
+ .usb2 = { .offset = 4, .count = 4, },
+ },
+ .scale_ss_clock = false,
+ .has_ipfs = false,
+ .otg_reset_sspi = false,
+ .ops = &tegra234_ops,
+ .mbox = {
+ .cmd = XUSB_BAR2_ARU_MBOX_CMD,
+ .data_in = XUSB_BAR2_ARU_MBOX_DATA_IN,
+ .data_out = XUSB_BAR2_ARU_MBOX_DATA_OUT,
+ .owner = XUSB_BAR2_ARU_MBOX_OWNER,
+ .smi_intr = XUSB_BAR2_ARU_SMI_INTR,
+ },
+ .lpm_support = true,
+ .has_bar2 = true,
+ .has_ifr = true,
+};
+MODULE_FIRMWARE("nvidia/tegra234/xusb.bin");
+
static const struct of_device_id tegra_xusb_of_match[] = {
{ .compatible = "nvidia,tegra124-xusb", .data = &tegra124_soc },
{ .compatible = "nvidia,tegra210-xusb", .data = &tegra210_soc },
{ .compatible = "nvidia,tegra186-xusb", .data = &tegra186_soc },
{ .compatible = "nvidia,tegra194-xusb", .data = &tegra194_soc },
+ { .compatible = "nvidia,tegra234-xusb", .data = &tegra234_soc },
{ },
};
MODULE_DEVICE_TABLE(of, tegra_xusb_of_match);
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 10/11] usb: host: xhci-tegra: Add Tegra234 XHCI support
2022-10-24 7:41 ` [PATCH 10/11] usb: host: xhci-tegra: Add Tegra234 XHCI support Wayne Chang
@ 2022-10-28 13:39 ` Thierry Reding
2022-11-01 14:53 ` Jon Hunter
0 siblings, 1 reply; 50+ messages in thread
From: Thierry Reding @ 2022-10-28 13:39 UTC (permalink / raw)
To: Wayne Chang
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
heikki.krogerus, ajayg, kishon, vkoul, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 15056 bytes --]
On Mon, Oct 24, 2022 at 03:41:27PM +0800, Wayne Chang wrote:
> From: Sing-Han Chen <singhanc@nvidia.com>
>
> This change adds Tegra234 XUSB host mode controller support.
>
> In Tegra234, some of the registers have moved to bar2 space.
> The new soc variable has_bar2 indicates the chip with bar2
> area. This patch adds new reg helper to let the driver reuse
> the same code for those chips with bar2 support.
>
> The new soc variables has_ifr indicates the chip with IFR FW
> loading support. IFR registers would be configured in
> MB1, and FW loading will be triggered in MB2.
>
> Signed-off-by: Sing-Han Chen <singhanc@nvidia.com>
> Co-developed-by: Wayne Chang <waynec@nvidia.com>
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
> drivers/usb/host/xhci-tegra.c | 277 +++++++++++++++++++++++++++++-----
> 1 file changed, 237 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index bdb776553826..86036eeece43 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -44,6 +44,9 @@
> #define XUSB_CFG_4 0x010
> #define XUSB_BASE_ADDR_SHIFT 15
> #define XUSB_BASE_ADDR_MASK 0x1ffff
> +#define XUSB_CFG_7 0x01c
> +#define XUSB_BASE2_ADDR_SHIFT 16
> +#define XUSB_BASE2_ADDR_MASK 0xffff
> #define XUSB_CFG_16 0x040
> #define XUSB_CFG_24 0x060
> #define XUSB_CFG_AXI_CFG 0x0f8
> @@ -75,6 +78,20 @@
> #define MBOX_SMI_INTR_FW_HANG BIT(1)
> #define MBOX_SMI_INTR_EN BIT(3)
>
> +/* BAR2 registers */
> +#define XUSB_BAR2_ARU_MBOX_CMD 0x004
> +#define XUSB_BAR2_ARU_MBOX_DATA_IN 0x008
> +#define XUSB_BAR2_ARU_MBOX_DATA_OUT 0x00c
> +#define XUSB_BAR2_ARU_MBOX_OWNER 0x010
> +#define XUSB_BAR2_ARU_SMI_INTR 0x014
> +#define XUSB_BAR2_ARU_SMI_ARU_FW_SCRATCH_DATA0 0x01c
> +#define XUSB_BAR2_ARU_IFRDMA_CFG0 0x0e0
> +#define XUSB_BAR2_ARU_IFRDMA_CFG1 0x0e4
> +#define XUSB_BAR2_ARU_IFRDMA_STREAMID_FIELD 0x0e8
> +#define XUSB_BAR2_ARU_C11_CSBRANGE 0x9c
> +#define XUSB_BAR2_ARU_FW_SCRATCH 0x1000
> +#define XUSB_BAR2_CSB_BASE_ADDR 0x2000
> +
> /* IPFS registers */
> #define IPFS_XUSB_HOST_MSI_BAR_SZ_0 0x0c0
> #define IPFS_XUSB_HOST_MSI_AXI_BAR_ST_0 0x0c4
> @@ -111,6 +128,9 @@
> #define IMFILLRNG1_TAG_HI_SHIFT 16
> #define XUSB_FALC_IMFILLCTL 0x158
>
> +/* CSB ARU registers */
Weird double-space between "ARU" and "registers".
> +#define XUSB_CSB_ARU_SCRATCH0 0x100100
> +
> /* MP CSB registers */
> #define XUSB_CSB_MP_ILOAD_ATTR 0x101a00
> #define XUSB_CSB_MP_ILOAD_BASE_LO 0x101a04
> @@ -131,6 +151,9 @@
>
> #define IMEM_BLOCK_SIZE 256
>
> +#define FW_IOCTL_TYPE_SHIFT (24)
This should use tabs for spacing, like all the other definitions. Also,
no need to wrap literal integers in parentheses.
> +#define FW_IOCTL_CFGTBL_READ (17)
No need for the parentheses.
> +
> struct tegra_xusb_fw_header {
> __le32 boot_loadaddr_in_imem;
> __le32 boot_codedfi_offset;
> @@ -175,6 +198,7 @@ struct tegra_xusb_mbox_regs {
> u16 data_in;
> u16 data_out;
> u16 owner;
> + u16 smi_intr;
> };
>
> struct tegra_xusb_context_soc {
> @@ -189,6 +213,7 @@ struct tegra_xusb_context_soc {
> } fpci;
> };
>
> +struct tegra_xusb_soc_ops;
Probably better to move the definition of the structure here and instead
predeclare struct tegra_xusb.
> struct tegra_xusb_soc {
> const char *firmware;
> const char * const *supply_names;
> @@ -205,11 +230,15 @@ struct tegra_xusb_soc {
> } ports;
>
> struct tegra_xusb_mbox_regs mbox;
> + struct tegra_xusb_soc_ops *ops;
const please.
>
> bool scale_ss_clock;
> bool has_ipfs;
> bool lpm_support;
> bool otg_reset_sspi;
> +
> + bool has_bar2;
> + bool has_ifr;
> };
>
> struct tegra_xusb_context {
> @@ -230,6 +259,8 @@ struct tegra_xusb {
>
> void __iomem *ipfs_base;
> void __iomem *fpci_base;
> + void __iomem *bar2_base;
> + resource_size_t bar2_start;
Maybe just store struct resource *bar2, here.
[...]
> @@ -664,6 +754,7 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
> static irqreturn_t tegra_xusb_mbox_thread(int irq, void *data)
> {
> struct tegra_xusb *tegra = data;
> + struct tegra_xusb_soc_ops *ops = tegra->soc->ops;
const
> @@ -709,6 +800,15 @@ static void tegra_xusb_config(struct tegra_xusb *tegra)
> value |= regs & (XUSB_BASE_ADDR_MASK << XUSB_BASE_ADDR_SHIFT);
> fpci_writel(tegra, value, XUSB_CFG_4);
>
> + /* Program BAR2 space */
> + if (tegra->soc->has_bar2) {
You could make this depend on tegra->bar2 if you make the change above.
> + value = fpci_readl(tegra, XUSB_CFG_7);
> + value &= ~(XUSB_BASE2_ADDR_MASK << XUSB_BASE2_ADDR_SHIFT);
> + value |= tegra->bar2_start &
> + (XUSB_BASE2_ADDR_MASK << XUSB_BASE2_ADDR_SHIFT);
> + fpci_writel(tegra, value, XUSB_CFG_7);
> + }
> +
> usleep_range(100, 200);
>
> /* Enable bus master */
> @@ -881,21 +981,36 @@ static int tegra_xusb_request_firmware(struct tegra_xusb *tegra)
> return 0;
> }
>
> +static int tegra_xusb_wait_for_falcon(struct tegra_xusb *tegra)
> +{
> + struct xhci_cap_regs __iomem *cap_regs;
> + struct xhci_op_regs __iomem *op_regs;
> + int ret;
> + u32 val;
Use "value" for consistency with the rest of the driver.
> +
> + cap_regs = tegra->regs;
> + op_regs = tegra->regs + HC_LENGTH(readl(&cap_regs->hc_capbase));
> +
> + ret = readl_poll_timeout(&op_regs->status, val, !(val & STS_CNR), 1000, 200000);
> +
> + if (ret)
> + dev_err(tegra->dev, "XHCI Controller not ready. Falcon state: 0x%x\n",
> + csb_readl(tegra, XUSB_FALC_CPUCTL));
> +
> + return ret;
> +}
This refactoring could be a separate patch. It makes the rest of the
changes harder to review. Not necessarily something that needs to be
addressed, though.
> +
> static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
> {
> unsigned int code_tag_blocks, code_size_blocks, code_blocks;
> - struct xhci_cap_regs __iomem *cap = tegra->regs;
> struct tegra_xusb_fw_header *header;
> struct device *dev = tegra->dev;
> - struct xhci_op_regs __iomem *op;
> - unsigned long timeout;
> time64_t timestamp;
> u64 address;
> u32 value;
> int err;
>
> header = (struct tegra_xusb_fw_header *)tegra->fw.virt;
> - op = tegra->regs + HC_LENGTH(readl(&cap->hc_capbase));
>
> if (csb_readl(tegra, XUSB_CSB_MP_ILOAD_BASE_LO) != 0) {
> dev_info(dev, "Firmware already loaded, Falcon state %#x\n",
> @@ -968,26 +1083,43 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
> /* Boot Falcon CPU and wait for USBSTS_CNR to get cleared. */
> csb_writel(tegra, CPUCTL_STARTCPU, XUSB_FALC_CPUCTL);
>
> - timeout = jiffies + msecs_to_jiffies(200);
> + if (tegra_xusb_wait_for_falcon(tegra))
> + return -EIO;
>
> - do {
> - value = readl(&op->status);
> - if ((value & STS_CNR) == 0)
> - break;
> + timestamp = le32_to_cpu(header->fwimg_created_time);
>
> - usleep_range(1000, 2000);
> - } while (time_is_after_jiffies(timeout));
> + dev_info(dev, "Firmware timestamp: %ptTs UTC\n", ×tamp);
> +
> + return 0;
> +}
>
> - value = readl(&op->status);
> - if (value & STS_CNR) {
> - value = csb_readl(tegra, XUSB_FALC_CPUCTL);
> - dev_err(dev, "XHCI controller not read: %#010x\n", value);
> +static u32 tegra_xusb_read_firmware_header(struct tegra_xusb *tegra, u32 offset)
> +{
> + /*
> + * We only accept reading the firmware config table
> + * The offset should not exceed the fw header structure
> + */
> + if (offset >= sizeof(struct tegra_xusb_fw_header))
> + return 0;
You technically still allow reading 3 bytes past the header structure.
Or does the firmware's CFGTL_READ IOCTL mask out the lower 2 bits of the
offset?
> +
> + bar2_writel(tegra, (FW_IOCTL_CFGTBL_READ << FW_IOCTL_TYPE_SHIFT) | offset,
> + XUSB_BAR2_ARU_FW_SCRATCH);
> + return bar2_readl(tegra, XUSB_BAR2_ARU_SMI_ARU_FW_SCRATCH_DATA0);
> +}
> +
> +static int tegra_xusb_init_ifr_firmware(struct tegra_xusb *tegra)
> +{
> + time64_t timestamp;
> +
> + if (tegra_xusb_wait_for_falcon(tegra))
> return -EIO;
> - }
>
> - timestamp = le32_to_cpu(header->fwimg_created_time);
> +#define offsetof_32(X, Y) ((u8)(offsetof(X, Y) / sizeof(__le32)))
> + timestamp = tegra_xusb_read_firmware_header(tegra,
> + offsetof_32(struct tegra_xusb_fw_header,
> + fwimg_created_time) << 2);
>
> - dev_info(dev, "Firmware timestamp: %ptTs UTC\n", ×tamp);
> + dev_info(tegra->dev, "Firmware timestamp: %ptTs UTC\n", ×tamp);
>
> return 0;
> }
> @@ -1403,7 +1535,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> struct of_phandle_args args;
> struct tegra_xusb *tegra;
> struct device_node *np;
> - struct resource *regs;
> + struct resource *res, *regs;
> struct xhci_hcd *xhci;
> unsigned int i, j, k;
> struct phy *phy;
> @@ -1435,6 +1567,11 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> tegra->ipfs_base = devm_platform_ioremap_resource(pdev, 2);
> if (IS_ERR(tegra->ipfs_base))
> return PTR_ERR(tegra->ipfs_base);
> + } else if (tegra->soc->has_bar2) {
> + tegra->bar2_base = devm_platform_get_and_ioremap_resource(pdev, 2, &res);
If you store struct resource *bar2 in tegra, you can pass &tegra->bar2
here and ...
> + if (IS_ERR(tegra->bar2_base))
> + return PTR_ERR(tegra->bar2_base);
> + tegra->bar2_start = res->start;
... skip this.
> }
>
> tegra->xhci_irq = platform_get_irq(pdev, 0);
> @@ -1651,10 +1788,13 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> goto disable_phy;
> }
>
> - err = tegra_xusb_request_firmware(tegra);
> - if (err < 0) {
> - dev_err(&pdev->dev, "failed to request firmware: %d\n", err);
> - goto disable_phy;
> + if (!tegra->soc->has_ifr) {
> + err = tegra_xusb_request_firmware(tegra);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "failed to request firmware: %d\n", err);
> + goto disable_phy;
> + }
> }
>
> err = tegra_xusb_unpowergate_partitions(tegra);
> @@ -1663,7 +1803,10 @@ static int tegra_xusb_probe(struct platform_device *pdev)
>
> tegra_xusb_config(tegra);
>
> - err = tegra_xusb_load_firmware(tegra);
> + if (tegra->soc->has_ifr)
> + err = tegra_xusb_init_ifr_firmware(tegra);
> + else
> + err = tegra_xusb_load_firmware(tegra);
> if (err < 0) {
> dev_err(&pdev->dev, "failed to load firmware: %d\n", err);
> goto powergate;
> @@ -2070,7 +2213,10 @@ static int tegra_xusb_exit_elpg(struct tegra_xusb *tegra, bool runtime)
> tegra_xusb_config(tegra);
> tegra_xusb_restore_context(tegra);
>
> - err = tegra_xusb_load_firmware(tegra);
> + if (tegra->soc->has_ifr)
> + err = tegra_xusb_init_ifr_firmware(tegra);
> + else
> + err = tegra_xusb_load_firmware(tegra);
Might be worth extracting this into a new function since you use this
twice now.
> if (err < 0) {
> dev_err(tegra->dev, "failed to load firmware: %d\n", err);
> goto disable_phy;
> @@ -2271,6 +2417,13 @@ static const struct tegra_xusb_context_soc tegra124_xusb_context = {
> },
> };
>
> +static struct tegra_xusb_soc_ops tegra124_ops = {
const
> + .mbox_reg_readl = &fpci_readl,
> + .mbox_reg_writel = &fpci_writel,
> + .csb_reg_readl = &fpci_csb_readl,
> + .csb_reg_writel = &fpci_csb_writel,
> +};
> +
> static const struct tegra_xusb_soc tegra124_soc = {
> .firmware = "nvidia/tegra124/xusb.bin",
> .supply_names = tegra124_supply_names,
> @@ -2286,11 +2439,13 @@ static const struct tegra_xusb_soc tegra124_soc = {
> .scale_ss_clock = true,
> .has_ipfs = true,
> .otg_reset_sspi = false,
> + .ops = &tegra124_ops,
> .mbox = {
> .cmd = 0xe4,
> .data_in = 0xe8,
> .data_out = 0xec,
> .owner = 0xf0,
> + .smi_intr = XUSB_CFG_ARU_SMI_INTR,
> },
> };
> MODULE_FIRMWARE("nvidia/tegra124/xusb.bin");
> @@ -2322,11 +2477,13 @@ static const struct tegra_xusb_soc tegra210_soc = {
> .scale_ss_clock = false,
> .has_ipfs = true,
> .otg_reset_sspi = true,
> + .ops = &tegra124_ops,
> .mbox = {
> .cmd = 0xe4,
> .data_in = 0xe8,
> .data_out = 0xec,
> .owner = 0xf0,
> + .smi_intr = XUSB_CFG_ARU_SMI_INTR,
> },
> };
> MODULE_FIRMWARE("nvidia/tegra210/xusb.bin");
> @@ -2363,11 +2520,13 @@ static const struct tegra_xusb_soc tegra186_soc = {
> .scale_ss_clock = false,
> .has_ipfs = false,
> .otg_reset_sspi = false,
> + .ops = &tegra124_ops,
> .mbox = {
> .cmd = 0xe4,
> .data_in = 0xe8,
> .data_out = 0xec,
> .owner = 0xf0,
> + .smi_intr = XUSB_CFG_ARU_SMI_INTR,
> },
> .lpm_support = true,
> };
> @@ -2394,21 +2553,59 @@ static const struct tegra_xusb_soc tegra194_soc = {
> .scale_ss_clock = false,
> .has_ipfs = false,
> .otg_reset_sspi = false,
> + .ops = &tegra124_ops,
> .mbox = {
> .cmd = 0x68,
> .data_in = 0x6c,
> .data_out = 0x70,
> .owner = 0x74,
> + .smi_intr = XUSB_CFG_ARU_SMI_INTR,
> },
> .lpm_support = true,
> };
> MODULE_FIRMWARE("nvidia/tegra194/xusb.bin");
>
> +static struct tegra_xusb_soc_ops tegra234_ops = {
const
> + .mbox_reg_readl = &bar2_readl,
> + .mbox_reg_writel = &bar2_writel,
> + .csb_reg_readl = &bar2_csb_readl,
> + .csb_reg_writel = &bar2_csb_writel,
> +};
> +
> +static const struct tegra_xusb_soc tegra234_soc = {
> + .firmware = "nvidia/tegra234/xusb.bin",
> + .supply_names = tegra194_supply_names,
> + .num_supplies = ARRAY_SIZE(tegra194_supply_names),
> + .phy_types = tegra194_phy_types,
> + .num_types = ARRAY_SIZE(tegra194_phy_types),
> + .context = &tegra186_xusb_context,
> + .ports = {
> + .usb3 = { .offset = 0, .count = 4, },
> + .usb2 = { .offset = 4, .count = 4, },
> + },
> + .scale_ss_clock = false,
> + .has_ipfs = false,
> + .otg_reset_sspi = false,
> + .ops = &tegra234_ops,
> + .mbox = {
> + .cmd = XUSB_BAR2_ARU_MBOX_CMD,
> + .data_in = XUSB_BAR2_ARU_MBOX_DATA_IN,
> + .data_out = XUSB_BAR2_ARU_MBOX_DATA_OUT,
> + .owner = XUSB_BAR2_ARU_MBOX_OWNER,
> + .smi_intr = XUSB_BAR2_ARU_SMI_INTR,
> + },
> + .lpm_support = true,
> + .has_bar2 = true,
> + .has_ifr = true,
> +};
> +MODULE_FIRMWARE("nvidia/tegra234/xusb.bin");
Can you prepare a patch to add this firmware to the linux-firmware
repository? I don't see it there yet.
Thierry
> +
> static const struct of_device_id tegra_xusb_of_match[] = {
> { .compatible = "nvidia,tegra124-xusb", .data = &tegra124_soc },
> { .compatible = "nvidia,tegra210-xusb", .data = &tegra210_soc },
> { .compatible = "nvidia,tegra186-xusb", .data = &tegra186_soc },
> { .compatible = "nvidia,tegra194-xusb", .data = &tegra194_soc },
> + { .compatible = "nvidia,tegra234-xusb", .data = &tegra234_soc },
> { },
> };
> MODULE_DEVICE_TABLE(of, tegra_xusb_of_match);
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 10/11] usb: host: xhci-tegra: Add Tegra234 XHCI support
2022-10-28 13:39 ` Thierry Reding
@ 2022-11-01 14:53 ` Jon Hunter
2022-11-03 11:35 ` Wayne Chang
0 siblings, 1 reply; 50+ messages in thread
From: Jon Hunter @ 2022-11-01 14:53 UTC (permalink / raw)
To: Thierry Reding, Wayne Chang
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, treding,
heikki.krogerus, ajayg, kishon, vkoul, p.zabel, balbi,
mathias.nyman, jckuo, linux-usb, devicetree, linux-kernel,
singhanc, linux-i2c, linux-phy, linux-tegra
On 28/10/2022 14:39, Thierry Reding wrote:
...
>> +static const struct tegra_xusb_soc tegra234_soc = {
>> + .firmware = "nvidia/tegra234/xusb.bin",
>> + .supply_names = tegra194_supply_names,
>> + .num_supplies = ARRAY_SIZE(tegra194_supply_names),
>> + .phy_types = tegra194_phy_types,
>> + .num_types = ARRAY_SIZE(tegra194_phy_types),
>> + .context = &tegra186_xusb_context,
>> + .ports = {
>> + .usb3 = { .offset = 0, .count = 4, },
>> + .usb2 = { .offset = 4, .count = 4, },
>> + },
>> + .scale_ss_clock = false,
>> + .has_ipfs = false,
>> + .otg_reset_sspi = false,
>> + .ops = &tegra234_ops,
>> + .mbox = {
>> + .cmd = XUSB_BAR2_ARU_MBOX_CMD,
>> + .data_in = XUSB_BAR2_ARU_MBOX_DATA_IN,
>> + .data_out = XUSB_BAR2_ARU_MBOX_DATA_OUT,
>> + .owner = XUSB_BAR2_ARU_MBOX_OWNER,
>> + .smi_intr = XUSB_BAR2_ARU_SMI_INTR,
>> + },
>> + .lpm_support = true,
>> + .has_bar2 = true,
>> + .has_ifr = true,
>> +};
>> +MODULE_FIRMWARE("nvidia/tegra234/xusb.bin");
>
> Can you prepare a patch to add this firmware to the linux-firmware
> repository? I don't see it there yet.
Actually, we should remove the MODULE_FIRMWARE completely for Tegra234.
Per the commit message the variable 'has_ifr' is used to indicate if the
firmware is loaded by calling request_firmware() or via these IFR
registers. I wonder if we need this 'has_ifr' variable if we should just
avoid setting the 'firmware' variable for Tegra234 and use this instead
of the 'has_ifr'?
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 10/11] usb: host: xhci-tegra: Add Tegra234 XHCI support
2022-11-01 14:53 ` Jon Hunter
@ 2022-11-03 11:35 ` Wayne Chang
0 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-11-03 11:35 UTC (permalink / raw)
To: Jonathan Hunter, Thierry Reding
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, Thierry Reding,
heikki.krogerus, Ajay Gupta, kishon, vkoul, p.zabel, balbi,
mathias.nyman, Jui Chang Kuo, linux-usb, devicetree,
linux-kernel, Sing-Han Chen, linux-i2c, linux-phy, linux-tegra
On 11/1/22 22:53, Jon Hunter wrote:
> On 28/10/2022 14:39, Thierry Reding wrote:
>
> ...
>
>>> +static const struct tegra_xusb_soc tegra234_soc = {
>>> + .firmware = "nvidia/tegra234/xusb.bin",
>>> + .supply_names = tegra194_supply_names,
>>> + .num_supplies = ARRAY_SIZE(tegra194_supply_names),
>>> + .phy_types = tegra194_phy_types,
>>> + .num_types = ARRAY_SIZE(tegra194_phy_types),
>>> + .context = &tegra186_xusb_context,
>>> + .ports = {
>>> + .usb3 = { .offset = 0, .count = 4, },
>>> + .usb2 = { .offset = 4, .count = 4, },
>>> + },
>>> + .scale_ss_clock = false,
>>> + .has_ipfs = false,
>>> + .otg_reset_sspi = false,
>>> + .ops = &tegra234_ops,
>>> + .mbox = {
>>> + .cmd = XUSB_BAR2_ARU_MBOX_CMD,
>>> + .data_in = XUSB_BAR2_ARU_MBOX_DATA_IN,
>>> + .data_out = XUSB_BAR2_ARU_MBOX_DATA_OUT,
>>> + .owner = XUSB_BAR2_ARU_MBOX_OWNER,
>>> + .smi_intr = XUSB_BAR2_ARU_SMI_INTR,
>>> + },
>>> + .lpm_support = true,
>>> + .has_bar2 = true,
>>> + .has_ifr = true,
>>> +};
>>> +MODULE_FIRMWARE("nvidia/tegra234/xusb.bin");
>>
>> Can you prepare a patch to add this firmware to the linux-firmware
>> repository? I don't see it there yet.
>
>
> Actually, we should remove the MODULE_FIRMWARE completely for Tegra234.
> Per the commit message the variable 'has_ifr' is used to indicate if the
> firmware is loaded by calling request_firmware() or via these IFR
> registers. I wonder if we need this 'has_ifr' variable if we should just
> avoid setting the 'firmware' variable for Tegra234 and use this instead
> of the 'has_ifr'?
>
Thanks for the review.
Yes, correct. The firmware loading is moved to MB2 and thus we do not
need it anymore. I'll remove it together with the .firmware in the soc data.
We could checking firmware in soc data instead of has_ifr as we now only
get two ways to load the firmware.
I'll make the change on it in the next patch series
thanks,
Wayne.
> Jon
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 11/11] usb: gadget: tegra-xudc: Add Tegra234 support
2022-10-24 7:41 [PATCH 00/11] Enable USB host and device functions on Jetson Wayne Chang
` (9 preceding siblings ...)
2022-10-24 7:41 ` [PATCH 10/11] usb: host: xhci-tegra: Add Tegra234 XHCI support Wayne Chang
@ 2022-10-24 7:41 ` Wayne Chang
10 siblings, 0 replies; 50+ messages in thread
From: Wayne Chang @ 2022-10-24 7:41 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, treding, jonathanh,
thierry.reding, heikki.krogerus, ajayg, kishon, vkoul, p.zabel,
balbi, mathias.nyman, jckuo
Cc: waynec, linux-usb, devicetree, linux-kernel, singhanc, linux-i2c,
linux-phy, linux-tegra
From: Sing-Han Chen <singhanc@nvidia.com>
This commit adds support for XUSB device mode controller support on
Tegra234 SoC. This is very similar to the existing Tegra194 XUDC.
Signed-off-by: Sing-Han Chen <singhanc@nvidia.com>
Signed-off-by: Wayne Chang <waynec@nvidia.com>
---
drivers/usb/gadget/udc/tegra-xudc.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
index 76919d7570d2..ff697190469b 100644
--- a/drivers/usb/gadget/udc/tegra-xudc.c
+++ b/drivers/usb/gadget/udc/tegra-xudc.c
@@ -3660,6 +3660,19 @@ static struct tegra_xudc_soc tegra194_xudc_soc_data = {
.has_ipfs = false,
};
+static struct tegra_xudc_soc tegra234_xudc_soc_data = {
+ .clock_names = tegra186_xudc_clock_names,
+ .num_clks = ARRAY_SIZE(tegra186_xudc_clock_names),
+ .num_phys = 4,
+ .u1_enable = true,
+ .u2_enable = true,
+ .lpm_enable = true,
+ .invalid_seq_num = false,
+ .pls_quirk = false,
+ .port_reset_quirk = false,
+ .has_ipfs = false,
+};
+
static const struct of_device_id tegra_xudc_of_match[] = {
{
.compatible = "nvidia,tegra210-xudc",
@@ -3673,6 +3686,10 @@ static const struct of_device_id tegra_xudc_of_match[] = {
.compatible = "nvidia,tegra194-xudc",
.data = &tegra194_xudc_soc_data
},
+ {
+ .compatible = "nvidia,tegra234-xudc",
+ .data = &tegra234_xudc_soc_data
+ },
{ }
};
MODULE_DEVICE_TABLE(of, tegra_xudc_of_match);
--
2.25.1
^ permalink raw reply related [flat|nested] 50+ messages in thread