* [PATCH v1 0/5] PCI: tegra: A couple of cleanups
@ 2025-09-26 7:27 Anand Moon
2025-09-26 7:27 ` [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema Anand Moon
` (4 more replies)
0 siblings, 5 replies; 22+ messages in thread
From: Anand Moon @ 2025-09-26 7:27 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, Thierry Reding,
Jonathan Hunter,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:TEGRA ARCHITECTURE SUPPORT, open list
Cc: Anand Moon
Hi All,
This small series provides two cleanup patches for the Tegra PCIe driver.
The overall goal is to replace custom, open-coded logic with standard
kernel helper functions.
These changes improve the driver's readability and maintainability by
everaging modern, well-tested APIs for clock management and register
polling.
v1 Added new devicetree binding nvidia,tegra-pcie.yaml file.
Switch from devm_clk_bulk_get_all() -> devm_clk_bulk_get() api.
Fixed checkpatch warnings.
Tested on Jetson Nano 4 GB ram.
RFC : https://lore.kernel.org/linux-tegra/20250831190055.7952-2-linux.amoon@gmail.com/
Thanks
-Anand
Anand Moon (5):
dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings
documentation into a YAML schema
PCI: tegra: Simplify clock handling by using clk_bulk*() functions
PCI: tegra: Use readl_poll_timeout() for link status polling
PCI: tegra: Use BIT() and GENMASK() macros for register definitions
PCI: tegra: Document map_lock and mask_lock usage
.../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++
.../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------
drivers/pci/controller/pci-tegra.c | 268 ++++---
3 files changed, 777 insertions(+), 812 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml
delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
base-commit: 4ff71af020ae59ae2d83b174646fc2ad9fcd4dc4
--
2.50.1
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-09-26 7:27 [PATCH v1 0/5] PCI: tegra: A couple of cleanups Anand Moon @ 2025-09-26 7:27 ` Anand Moon 2025-09-26 13:56 ` Rob Herring 2025-09-26 7:27 ` [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions Anand Moon ` (3 subsequent siblings) 4 siblings, 1 reply; 22+ messages in thread From: Anand Moon @ 2025-09-26 7:27 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Cc: Anand Moon Convert the legacy text-based binding documentation for nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following the Devicetree Schema format. This improves validation coverage and enables dtbs_check compliance for Tegra PCIe nodes. Cc: Jon Hunter <jonathanh@nvidia.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- v1: new patch in this series. --- .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ 2 files changed, 651 insertions(+), 670 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml new file mode 100644 index 000000000000..dd8cba125b53 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml @@ -0,0 +1,651 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NVIDIA Tegra PCIe Controller + +maintainers: + - Thierry Reding <thierry.reding@gmail.com> + - Jon Hunter <jonathanh@nvidia.com> + +description: | + PCIe controller found on NVIDIA Tegra SoCs including Tegra20, Tegra30, + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and + platform-specific clock, reset, and power supply configurations. + +properties: + compatible: + oneOf: + - items: + - enum: + - nvidia,tegra20-pcie + - nvidia,tegra30-pcie + - nvidia,tegra124-pcie + - nvidia,tegra210-pcie + - nvidia,tegra186-pcie + + reg: + items: + - description: PADS registers + - description: AFI registers + - description: Configuration space region + + reg-names: + items: + - const: pads + - const: afi + - const: cs + + device_type: + const: pci + + interrupts: + items: + - description: Controller interrupt + - description: MSI interrupt + + interrupt-names: + items: + - const: intr + - const: msi + + clocks: + oneOf: + - items: + - description: PCIe clock + - description: AFI clock + - description: PLL_E clock + - items: + - description: PCIe clock + - description: AFI clock + - description: PLL_E clock + - description: CML clock + + clock-names: + oneOf: + - items: + - const: pex + - const: afi + - const: pll_e + - items: + - const: pex + - const: afi + - const: pll_e + - const: cml + + resets: + items: + - description: PCIe reset + - description: AFI reset + - description: PCIe X reset + + reset-names: + items: + - const: pex + - const: afi + - const: pcie_x + + power-domains: + maxItems: 1 + description: | + A phandle to the node that controls power to the respective PCIe + controller and a specifier name for the PCIe controller. + + interconnects: + minItems: 1 + maxItems: 2 + + interconnect-names: + minItems: 1 + maxItems: 2 + description: + Should include name of the interconnect path for each interconnect + entry. Consult TRM documentation for information about available + memory clients, see DMA CONTROLLER and MEMORY WRITE sections. + + pinctrl-names: + items: + - const: default + - const: idle + + pinctrl-0: + $ref: /schemas/types.yaml#/definitions/phandle + + pinctrl-1: + $ref: /schemas/types.yaml#/definitions/phandle + + nvidia,num-lanes: + description: Number of PCIe lanes used + $ref: /schemas/types.yaml#/definitions/uint32 + +allOf: + - $ref: /schemas/pci/pci-host-bridge.yaml# + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra20-pcie + - nvidia,tegra186-pcie + then: + properties: + clocks: + minItems: 3 + maxItems: 3 + clock-names: + items: + - const: pex + - const: afi + - const: pll_e + resets: + minItems: 3 + maxItems: 3 + reset-names: + items: + - const: pex + - const: afi + - const: pcie_x + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra30-pcie + - nvidia,tegra124-pcie + - nvidia,tegra210-pcie + then: + properties: + clocks: + minItems: 4 + maxItems: 4 + clock-names: + items: + - const: pex + - const: afi + - const: pll_e + - const: cml + resets: + minItems: 3 + maxItems: 3 + reset-names: + items: + - const: pex + - const: afi + - const: pcie_x + + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra20-pcie + - nvidia,tegra30-pcie + - nvidia,tegra186-pcie + then: + required: + - power-domains + + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra186-pcie + then: + required: + - interconnects + - if: + properties: + compatible: + contains: + enum: + - nvidia,tegra210-pcie + then: + required: + - pinctrl-names + - pinctrl-0 + - pinctrl-1 + +unevaluatedProperties: false + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - resets + - reset-names + - interrupts + - interrupt-map + - interrupt-map-mask + - ranges + - bus-range + - device_type + - interconnects + - pinctrl-names + - nvidia,num-lanes + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + bus { + #address-cells = <1>; + #size-cells = <1>; + + pcie@80003000 { + compatible = "nvidia,tegra20-pcie"; + device_type = "pci"; + reg = <0x80003000 0x00000800>, + <0x80003800 0x00000200>, + <0x90000000 0x10000000>; + reg-names = "pads", "afi", "cs"; + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "intr", "msi"; + interrupt-parent = <&intc>; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &intc GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; + + bus-range = <0x00 0xff>; + #address-cells = <3>; + #size-cells = <2>; + + ranges = <0x02000000 0 0x80000000 0x80000000 0 0x00001000>, + <0x02000000 0 0x80001000 0x80001000 0 0x00001000>, + <0x01000000 0 0 0x82000000 0 0x00010000>, + <0x02000000 0 0xa0000000 0xa0000000 0 0x08000000>, + <0x42000000 0 0xa8000000 0xa8000000 0 0x18000000>; + + clocks = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 118>; + clock-names = "pex", "afi", "pll_e"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + power-domains = <&pd_core>; + operating-points-v2 = <&pcie_dvfs_opp_table>; + + status = "disabled"; + + pci@1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x80000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <2>; + }; + + pci@2,0 { + device_type = "pci"; + assigned-addresses = <0x82001000 0 0x80001000 0 0x1000>; + reg = <0x001000 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <2>; + }; + }; + }; + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + bus { + #address-cells = <1>; + #size-cells = <1>; + + pcie@3000 { + compatible = "nvidia,tegra30-pcie"; + device_type = "pci"; + reg = <0x00003000 0x00000800>, + <0x00003800 0x00000200>, + <0x10000000 0x10000000>; + reg-names = "pads", "afi", "cs"; + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "intr", "msi"; + interrupt-parent = <&intc>; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &intc GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; + + bus-range = <0x00 0xff>; + #address-cells = <3>; + #size-cells = <2>; + + ranges = <0x02000000 0 0x00000000 0x00000000 0 0x00001000>, + <0x02000000 0 0x00001000 0x00001000 0 0x00001000>, + <0x02000000 0 0x00004000 0x00004000 0 0x00001000>, + <0x01000000 0 0 0x02000000 0 0x00010000>, + <0x02000000 0 0x20000000 0x20000000 0 0x08000000>, + <0x42000000 0 0x28000000 0x28000000 0 0x18000000>; + + clocks = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 193>, + <&tegra_car 215>; + clock-names = "pex", "afi", "pll_e", "cml"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + power-domains = <&pd_core>; + operating-points-v2 = <&pcie_dvfs_opp_table>; + status = "disabled"; + + pci@1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x00000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <2>; + }; + + pci@2,0 { + device_type = "pci"; + assigned-addresses = <0x82001000 0 0x00001000 0 0x1000>; + reg = <0x001000 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <2>; + }; + + pci@3,0 { + device_type = "pci"; + assigned-addresses = <0x82001800 0 0x00004000 0 0x1000>; + reg = <0x001800 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <2>; + }; + }; + }; + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + + pcie@1003000 { + compatible = "nvidia,tegra124-pcie"; + device_type = "pci"; + reg = <0x0 0x01003000 0x0 0x00000800>, + <0x0 0x01003800 0x0 0x00000800>, + <0x0 0x02000000 0x0 0x10000000>; + reg-names = "pads", "afi", "cs"; + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "intr", "msi"; + interrupt-parent = <&gic>; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; + + bus-range = <0x00 0xff>; + #address-cells = <3>; + #size-cells = <2>; + + ranges = <0x02000000 0 0x01000000 0x0 0x01000000 0 0x00001000>, + <0x02000000 0 0x01001000 0x0 0x01001000 0 0x00001000>, + <0x01000000 0 0x0 0x0 0x12000000 0 0x00010000>, + <0x02000000 0 0x13000000 0x0 0x13000000 0 0x0d000000>, + <0x42000000 0 0x20000000 0x0 0x20000000 0 0x20000000>; + + clocks = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 231>, + <&tegra_car 268>; + clock-names = "pex", "afi", "pll_e", "cml"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + status = "disabled"; + + pci@1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <2>; + }; + + pci@2,0 { + device_type = "pci"; + assigned-addresses = <0x82001000 0 0x01001000 0 0x1000>; + reg = <0x001000 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <1>; + }; + }; + }; + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + + pcie@1003000 { + compatible = "nvidia,tegra210-pcie"; + device_type = "pci"; + reg = <0x0 0x01003000 0x0 0x00000800>, + <0x0 0x01003800 0x0 0x00000800>, + <0x0 0x02000000 0x0 0x10000000>; + reg-names = "pads", "afi", "cs"; + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "intr", "msi"; + interrupt-parent = <&gic>; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; + + + bus-range = <0x00 0xff>; + #address-cells = <3>; + #size-cells = <2>; + + ranges = <0x02000000 0 0x01000000 0x0 0x01000000 0 0x00001000>, + <0x02000000 0 0x01001000 0x0 0x01001000 0 0x00001000>, + <0x01000000 0 0x0 0x0 0x12000000 0 0x00010000>, + <0x02000000 0 0x13000000 0x0 0x13000000 0 0x0d000000>, + <0x42000000 0 0x20000000 0x0 0x20000000 0 0x20000000>; + + clocks = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 263>, + <&tegra_car 300>; + clock-names = "pex", "afi", "pll_e", "cml"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + + pinctrl-names = "default", "idle"; + pinctrl-0 = <&pex_dpd_disable>; + pinctrl-1 = <&pex_dpd_enable>; + + status = "disabled"; + + pci@1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <4>; + }; + + pci@2,0 { + device_type = "pci"; + assigned-addresses = <0x82001000 0 0x01001000 0 0x1000>; + reg = <0x001000 0 0 0 0>; + bus-range = <0x00 0xff>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <1>; + }; + }; + }; + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/power/tegra186-powergate.h> + #include <dt-bindings/memory/tegra186-mc.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + + pcie@10003000 { + compatible = "nvidia,tegra186-pcie"; + power-domains = <&bpmp TEGRA186_POWER_DOMAIN_PCX>; + device_type = "pci"; + reg = <0x0 0x10003000 0x0 0x00000800>, + <0x0 0x10003800 0x0 0x00000800>, + <0x0 0x40000000 0x0 0x10000000>; + reg-names = "pads", "afi", "cs"; + + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "intr", "msi"; + interrupt-parent = <&gic>; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; + + bus-range = <0x00 0xff>; + #address-cells = <3>; + #size-cells = <2>; + + ranges = <0x02000000 0 0x10000000 0x0 0x10000000 0 0x00001000>, + <0x02000000 0 0x10001000 0x0 0x10001000 0 0x00001000>, + <0x02000000 0 0x10004000 0x0 0x10004000 0 0x00001000>, + <0x01000000 0 0x0 0x0 0x50000000 0 0x00010000>, + <0x02000000 0 0x50100000 0x0 0x50100000 0 0x07f00000>, + <0x42000000 0 0x58000000 0x0 0x58000000 0 0x28000000>; + + clocks = <&bpmp 3>, + <&bpmp 4>, + <&bpmp 512>; + clock-names = "pex", "afi", "pll_e"; + + resets = <&bpmp 29>, + <&bpmp 1>, + <&bpmp 30>; + reset-names = "pex", "afi", "pcie_x"; + + interconnects = <&mc TEGRA186_MEMORY_CLIENT_AFIR &emc>, + <&mc TEGRA186_MEMORY_CLIENT_AFIW &emc>; + interconnect-names = "dma-mem", "write"; + + iommus = <&smmu TEGRA186_SID_AFI>; + iommu-map = <0x0 &smmu TEGRA186_SID_AFI 0x1000>; + iommu-map-mask = <0x0>; + + status = "disabled"; + + pci@1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x10000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <2>; + }; + + pci@2,0 { + device_type = "pci"; + assigned-addresses = <0x82001000 0 0x10001000 0 0x1000>; + reg = <0x001000 0 0 0 0>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <1>; + }; + + pci@3,0 { + device_type = "pci"; + assigned-addresses = <0x82001800 0 0x10004000 0 0x1000>; + reg = <0x001800 0 0 0 0>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + nvidia,num-lanes = <1>; + }; + }; + }; diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt deleted file mode 100644 index d099f3476ccc..000000000000 --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt +++ /dev/null @@ -1,670 +0,0 @@ -NVIDIA Tegra PCIe controller - -Required properties: -- compatible: Must be: - - "nvidia,tegra20-pcie": for Tegra20 - - "nvidia,tegra30-pcie": for Tegra30 - - "nvidia,tegra124-pcie": for Tegra124 and Tegra132 - - "nvidia,tegra210-pcie": for Tegra210 - - "nvidia,tegra186-pcie": for Tegra186 -- power-domains: To ungate power partition by BPMP powergate driver. Must - contain BPMP phandle and PCIe power partition ID. This is required only - for Tegra186. -- device_type: Must be "pci" -- reg: A list of physical base address and length for each set of controller - registers. Must contain an entry for each entry in the reg-names property. -- reg-names: Must include the following entries: - "pads": PADS registers - "afi": AFI registers - "cs": configuration space region -- interrupts: A list of interrupt outputs of the controller. Must contain an - entry for each entry in the interrupt-names property. -- interrupt-names: Must include the following entries: - "intr": The Tegra interrupt that is asserted for controller interrupts - "msi": The Tegra interrupt that is asserted when an MSI is received -- bus-range: Range of bus numbers associated with this controller -- #address-cells: Address representation for root ports (must be 3) - - cell 0 specifies the bus and device numbers of the root port: - [23:16]: bus number - [15:11]: device number - - cell 1 denotes the upper 32 address bits and should be 0 - - cell 2 contains the lower 32 address bits and is used to translate to the - CPU address space -- #size-cells: Size representation for root ports (must be 2) -- ranges: Describes the translation of addresses for root ports and standard - PCI regions. The entries must be 6 cells each, where the first three cells - correspond to the address as described for the #address-cells property - above, the fourth cell is the physical CPU address to translate to and the - fifth and six cells are as described for the #size-cells property above. - - The first two entries are expected to translate the addresses for the root - port registers, which are referenced by the assigned-addresses property of - the root port nodes (see below). - - The remaining entries setup the mapping for the standard I/O, memory and - prefetchable PCI regions. The first cell determines the type of region - that is setup: - - 0x81000000: I/O memory region - - 0x82000000: non-prefetchable memory region - - 0xc2000000: prefetchable memory region - Please refer to the standard PCI bus binding document for a more detailed - explanation. -- #interrupt-cells: Size representation for interrupts (must be 1) -- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties - Please refer to the standard PCI bus binding document for a more detailed - explanation. -- clocks: Must contain an entry for each entry in clock-names. - See ../clocks/clock-bindings.txt for details. -- clock-names: Must include the following entries: - - pex - - afi - - pll_e - - cml (not required for Tegra20) -- resets: Must contain an entry for each entry in reset-names. - See ../reset/reset.txt for details. -- reset-names: Must include the following entries: - - pex - - afi - - pcie_x - -Optional properties: -- pinctrl-names: A list of pinctrl state names. Must contain the following - entries: - - "default": active state, puts PCIe I/O out of deep power down state - - "idle": puts PCIe I/O into deep power down state -- pinctrl-0: phandle for the default/active state of pin configurations. -- pinctrl-1: phandle for the idle state of pin configurations. - -Required properties on Tegra124 and later (deprecated): -- phys: Must contain an entry for each entry in phy-names. -- phy-names: Must include the following entries: - - pcie - -These properties are deprecated in favour of per-lane PHYs define in each of -the root ports (see below). - -Power supplies for Tegra20: -- avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. -- vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. -- avdd-pex-pll-supply: Power supply for dedicated (internal) PCIe PLL. Must - supply 1.05 V. -- avdd-plle-supply: Power supply for PLLE, which is shared with SATA. Must - supply 1.05 V. -- vddio-pex-clk-supply: Power supply for PCIe clock. Must supply 3.3 V. - -Power supplies for Tegra30: -- Required: - - avdd-pex-pll-supply: Power supply for dedicated (internal) PCIe PLL. Must - supply 1.05 V. - - avdd-plle-supply: Power supply for PLLE, which is shared with SATA. Must - supply 1.05 V. - - vddio-pex-ctl-supply: Power supply for PCIe control I/O partition. Must - supply 1.8 V. - - hvdd-pex-supply: High-voltage supply for PCIe I/O and PCIe output clocks. - Must supply 3.3 V. -- Optional: - - If lanes 0 to 3 are used: - - avdd-pexa-supply: Power supply for analog PCIe logic. Must supply 1.05 V. - - vdd-pexa-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. - - If lanes 4 or 5 are used: - - avdd-pexb-supply: Power supply for analog PCIe logic. Must supply 1.05 V. - - vdd-pexb-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. - -Power supplies for Tegra124: -- Required: - - avddio-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. - - dvddio-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. - - hvdd-pex-supply: High-voltage supply for PCIe I/O and PCIe output clocks. - Must supply 3.3 V. - - vddio-pex-ctl-supply: Power supply for PCIe control I/O partition. Must - supply 2.8-3.3 V. - -Power supplies for Tegra210: -- Required: - - hvddio-pex-supply: High-voltage supply for PCIe I/O and PCIe output - clocks. Must supply 1.8 V. - - dvddio-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. - - vddio-pex-ctl-supply: Power supply for PCIe control I/O partition. Must - supply 1.8 V. - -Power supplies for Tegra186: -- Required: - - dvdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. - - hvdd-pex-pll-supply: High-voltage supply for PLLE (shared with USB3). Must - supply 1.8 V. - - hvdd-pex-supply: High-voltage supply for PCIe I/O and PCIe output clocks. - Must supply 1.8 V. - - vddio-pexctl-aud-supply: Power supply for PCIe side band signals. Must - supply 1.8 V. - -Root ports are defined as subnodes of the PCIe controller node. - -Required properties: -- device_type: Must be "pci" -- assigned-addresses: Address and size of the port configuration registers -- reg: PCI bus address of the root port -- #address-cells: Must be 3 -- #size-cells: Must be 2 -- ranges: Sub-ranges distributed from the PCIe controller node. An empty - property is sufficient. -- nvidia,num-lanes: Number of lanes to use for this port. Valid combinations - are: - - Root port 0 uses 4 lanes, root port 1 is unused. - - Both root ports use 2 lanes. - -Required properties for Tegra124 and later: -- phys: Must contain an phandle to a PHY for each entry in phy-names. -- phy-names: Must include an entry for each active lane. Note that the number - of entries does not have to (though usually will) be equal to the specified - number of lanes in the nvidia,num-lanes property. Entries are of the form - "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes. - -Examples: -========= - -Tegra20: --------- - -SoC DTSI: - - pcie-controller@80003000 { - compatible = "nvidia,tegra20-pcie"; - device_type = "pci"; - reg = <0x80003000 0x00000800 /* PADS registers */ - 0x80003800 0x00000200 /* AFI registers */ - 0x90000000 0x10000000>; /* configuration space */ - reg-names = "pads", "afi", "cs"; - interrupts = <0 98 0x04 /* controller interrupt */ - 0 99 0x04>; /* MSI interrupt */ - interrupt-names = "intr", "msi"; - - #interrupt-cells = <1>; - interrupt-map-mask = <0 0 0 0>; - interrupt-map = <0 0 0 0 &intc GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; - - bus-range = <0x00 0xff>; - #address-cells = <3>; - #size-cells = <2>; - - ranges = <0x82000000 0 0x80000000 0x80000000 0 0x00001000 /* port 0 registers */ - 0x82000000 0 0x80001000 0x80001000 0 0x00001000 /* port 1 registers */ - 0x81000000 0 0 0x82000000 0 0x00010000 /* downstream I/O */ - 0x82000000 0 0xa0000000 0xa0000000 0 0x10000000 /* non-prefetchable memory */ - 0xc2000000 0 0xb0000000 0xb0000000 0 0x10000000>; /* prefetchable memory */ - - clocks = <&tegra_car 70>, <&tegra_car 72>, <&tegra_car 118>; - clock-names = "pex", "afi", "pll_e"; - resets = <&tegra_car 70>, <&tegra_car 72>, <&tegra_car 74>; - reset-names = "pex", "afi", "pcie_x"; - status = "disabled"; - - pci@1,0 { - device_type = "pci"; - assigned-addresses = <0x82000800 0 0x80000000 0 0x1000>; - reg = <0x000800 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - - ranges; - - nvidia,num-lanes = <2>; - }; - - pci@2,0 { - device_type = "pci"; - assigned-addresses = <0x82001000 0 0x80001000 0 0x1000>; - reg = <0x001000 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - - ranges; - - nvidia,num-lanes = <2>; - }; - }; - -Board DTS: - - pcie-controller@80003000 { - status = "okay"; - - vdd-supply = <&pci_vdd_reg>; - pex-clk-supply = <&pci_clk_reg>; - - /* root port 00:01.0 */ - pci@1,0 { - status = "okay"; - - /* bridge 01:00.0 (optional) */ - pci@0,0 { - reg = <0x010000 0 0 0 0>; - - #address-cells = <3>; - #size-cells = <2>; - - device_type = "pci"; - - /* endpoint 02:00.0 */ - pci@0,0 { - reg = <0x020000 0 0 0 0>; - }; - }; - }; - }; - -Note that devices on the PCI bus are dynamically discovered using PCI's bus -enumeration and therefore don't need corresponding device nodes in DT. However -if a device on the PCI bus provides a non-probeable bus such as I2C or SPI, -device nodes need to be added in order to allow the bus' children to be -instantiated at the proper location in the operating system's device tree (as -illustrated by the optional nodes in the example above). - -Tegra30: --------- - -SoC DTSI: - - pcie-controller@3000 { - compatible = "nvidia,tegra30-pcie"; - device_type = "pci"; - reg = <0x00003000 0x00000800 /* PADS registers */ - 0x00003800 0x00000200 /* AFI registers */ - 0x10000000 0x10000000>; /* configuration space */ - reg-names = "pads", "afi", "cs"; - interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH /* controller interrupt */ - GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ - interrupt-names = "intr", "msi"; - - #interrupt-cells = <1>; - interrupt-map-mask = <0 0 0 0>; - interrupt-map = <0 0 0 0 &intc GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; - - bus-range = <0x00 0xff>; - #address-cells = <3>; - #size-cells = <2>; - - ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000 /* port 0 configuration space */ - 0x82000000 0 0x00001000 0x00001000 0 0x00001000 /* port 1 configuration space */ - 0x82000000 0 0x00004000 0x00004000 0 0x00001000 /* port 2 configuration space */ - 0x81000000 0 0 0x02000000 0 0x00010000 /* downstream I/O */ - 0x82000000 0 0x20000000 0x20000000 0 0x08000000 /* non-prefetchable memory */ - 0xc2000000 0 0x28000000 0x28000000 0 0x18000000>; /* prefetchable memory */ - - clocks = <&tegra_car TEGRA30_CLK_PCIE>, - <&tegra_car TEGRA30_CLK_AFI>, - <&tegra_car TEGRA30_CLK_PLL_E>, - <&tegra_car TEGRA30_CLK_CML0>; - clock-names = "pex", "afi", "pll_e", "cml"; - resets = <&tegra_car 70>, - <&tegra_car 72>, - <&tegra_car 74>; - reset-names = "pex", "afi", "pcie_x"; - status = "disabled"; - - pci@1,0 { - device_type = "pci"; - assigned-addresses = <0x82000800 0 0x00000000 0 0x1000>; - reg = <0x000800 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <2>; - }; - - pci@2,0 { - device_type = "pci"; - assigned-addresses = <0x82001000 0 0x00001000 0 0x1000>; - reg = <0x001000 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <2>; - }; - - pci@3,0 { - device_type = "pci"; - assigned-addresses = <0x82001800 0 0x00004000 0 0x1000>; - reg = <0x001800 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <2>; - }; - }; - -Board DTS: - - pcie-controller@3000 { - status = "okay"; - - avdd-pexa-supply = <&ldo1_reg>; - vdd-pexa-supply = <&ldo1_reg>; - avdd-pexb-supply = <&ldo1_reg>; - vdd-pexb-supply = <&ldo1_reg>; - avdd-pex-pll-supply = <&ldo1_reg>; - avdd-plle-supply = <&ldo1_reg>; - vddio-pex-ctl-supply = <&sys_3v3_reg>; - hvdd-pex-supply = <&sys_3v3_pexs_reg>; - - pci@1,0 { - status = "okay"; - }; - - pci@3,0 { - status = "okay"; - }; - }; - -Tegra124: ---------- - -SoC DTSI: - - pcie-controller@1003000 { - compatible = "nvidia,tegra124-pcie"; - device_type = "pci"; - reg = <0x0 0x01003000 0x0 0x00000800 /* PADS registers */ - 0x0 0x01003800 0x0 0x00000800 /* AFI registers */ - 0x0 0x02000000 0x0 0x10000000>; /* configuration space */ - reg-names = "pads", "afi", "cs"; - interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ - <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ - interrupt-names = "intr", "msi"; - - #interrupt-cells = <1>; - interrupt-map-mask = <0 0 0 0>; - interrupt-map = <0 0 0 0 &gic GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; - - bus-range = <0x00 0xff>; - #address-cells = <3>; - #size-cells = <2>; - - ranges = <0x82000000 0 0x01000000 0x0 0x01000000 0 0x00001000 /* port 0 configuration space */ - 0x82000000 0 0x01001000 0x0 0x01001000 0 0x00001000 /* port 1 configuration space */ - 0x81000000 0 0x0 0x0 0x12000000 0 0x00010000 /* downstream I/O (64 KiB) */ - 0x82000000 0 0x13000000 0x0 0x13000000 0 0x0d000000 /* non-prefetchable memory (208 MiB) */ - 0xc2000000 0 0x20000000 0x0 0x20000000 0 0x20000000>; /* prefetchable memory (512 MiB) */ - - clocks = <&tegra_car TEGRA124_CLK_PCIE>, - <&tegra_car TEGRA124_CLK_AFI>, - <&tegra_car TEGRA124_CLK_PLL_E>, - <&tegra_car TEGRA124_CLK_CML0>; - clock-names = "pex", "afi", "pll_e", "cml"; - resets = <&tegra_car 70>, - <&tegra_car 72>, - <&tegra_car 74>; - reset-names = "pex", "afi", "pcie_x"; - status = "disabled"; - - pci@1,0 { - device_type = "pci"; - assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; - reg = <0x000800 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <2>; - }; - - pci@2,0 { - device_type = "pci"; - assigned-addresses = <0x82001000 0 0x01001000 0 0x1000>; - reg = <0x001000 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <1>; - }; - }; - -Board DTS: - - pcie-controller@1003000 { - status = "okay"; - - avddio-pex-supply = <&vdd_1v05_run>; - dvddio-pex-supply = <&vdd_1v05_run>; - avdd-pex-pll-supply = <&vdd_1v05_run>; - hvdd-pex-supply = <&vdd_3v3_lp0>; - hvdd-pex-pll-e-supply = <&vdd_3v3_lp0>; - vddio-pex-ctl-supply = <&vdd_3v3_lp0>; - avdd-pll-erefe-supply = <&avdd_1v05_run>; - - /* Mini PCIe */ - pci@1,0 { - phys = <&{/padctl@7009f000/pads/pcie/lanes/pcie-4}>; - phy-names = "pcie-0"; - status = "okay"; - }; - - /* Gigabit Ethernet */ - pci@2,0 { - phys = <&{/padctl@7009f000/pads/pcie/lanes/pcie-2}>; - phy-names = "pcie-0"; - status = "okay"; - }; - }; - -Tegra210: ---------- - -SoC DTSI: - - pcie-controller@1003000 { - compatible = "nvidia,tegra210-pcie"; - device_type = "pci"; - reg = <0x0 0x01003000 0x0 0x00000800 /* PADS registers */ - 0x0 0x01003800 0x0 0x00000800 /* AFI registers */ - 0x0 0x02000000 0x0 0x10000000>; /* configuration space */ - reg-names = "pads", "afi", "cs"; - interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ - <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ - interrupt-names = "intr", "msi"; - - #interrupt-cells = <1>; - interrupt-map-mask = <0 0 0 0>; - interrupt-map = <0 0 0 0 &gic GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; - - bus-range = <0x00 0xff>; - #address-cells = <3>; - #size-cells = <2>; - - ranges = <0x82000000 0 0x01000000 0x0 0x01000000 0 0x00001000 /* port 0 configuration space */ - 0x82000000 0 0x01001000 0x0 0x01001000 0 0x00001000 /* port 1 configuration space */ - 0x81000000 0 0x0 0x0 0x12000000 0 0x00010000 /* downstream I/O (64 KiB) */ - 0x82000000 0 0x13000000 0x0 0x13000000 0 0x0d000000 /* non-prefetchable memory (208 MiB) */ - 0xc2000000 0 0x20000000 0x0 0x20000000 0 0x20000000>; /* prefetchable memory (512 MiB) */ - - clocks = <&tegra_car TEGRA210_CLK_PCIE>, - <&tegra_car TEGRA210_CLK_AFI>, - <&tegra_car TEGRA210_CLK_PLL_E>, - <&tegra_car TEGRA210_CLK_CML0>; - clock-names = "pex", "afi", "pll_e", "cml"; - resets = <&tegra_car 70>, - <&tegra_car 72>, - <&tegra_car 74>; - reset-names = "pex", "afi", "pcie_x"; - status = "disabled"; - - pci@1,0 { - device_type = "pci"; - assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; - reg = <0x000800 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <4>; - }; - - pci@2,0 { - device_type = "pci"; - assigned-addresses = <0x82001000 0 0x01001000 0 0x1000>; - reg = <0x001000 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <1>; - }; - }; - -Board DTS: - - pcie-controller@1003000 { - status = "okay"; - - avdd-pll-uerefe-supply = <&avdd_1v05_pll>; - hvddio-pex-supply = <&vdd_1v8>; - dvddio-pex-supply = <&vdd_pex_1v05>; - dvdd-pex-pll-supply = <&vdd_pex_1v05>; - hvdd-pex-pll-e-supply = <&vdd_1v8>; - vddio-pex-ctl-supply = <&vdd_1v8>; - - pci@1,0 { - phys = <&{/padctl@7009f000/pads/pcie/lanes/pcie-0}>, - <&{/padctl@7009f000/pads/pcie/lanes/pcie-1}>, - <&{/padctl@7009f000/pads/pcie/lanes/pcie-2}>, - <&{/padctl@7009f000/pads/pcie/lanes/pcie-3}>; - phy-names = "pcie-0", "pcie-1", "pcie-2", "pcie-3"; - status = "okay"; - }; - - pci@2,0 { - phys = <&{/padctl@7009f000/pads/pcie/lanes/pcie-4}>; - phy-names = "pcie-0"; - status = "okay"; - }; - }; - -Tegra186: ---------- - -SoC DTSI: - - pcie@10003000 { - compatible = "nvidia,tegra186-pcie"; - power-domains = <&bpmp TEGRA186_POWER_DOMAIN_PCX>; - device_type = "pci"; - reg = <0x0 0x10003000 0x0 0x00000800 /* PADS registers */ - 0x0 0x10003800 0x0 0x00000800 /* AFI registers */ - 0x0 0x40000000 0x0 0x10000000>; /* configuration space */ - reg-names = "pads", "afi", "cs"; - - interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ - <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ - interrupt-names = "intr", "msi"; - - #interrupt-cells = <1>; - interrupt-map-mask = <0 0 0 0>; - interrupt-map = <0 0 0 0 &gic GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; - - bus-range = <0x00 0xff>; - #address-cells = <3>; - #size-cells = <2>; - - ranges = <0x82000000 0 0x10000000 0x0 0x10000000 0 0x00001000 /* port 0 configuration space */ - 0x82000000 0 0x10001000 0x0 0x10001000 0 0x00001000 /* port 1 configuration space */ - 0x82000000 0 0x10004000 0x0 0x10004000 0 0x00001000 /* port 2 configuration space */ - 0x81000000 0 0x0 0x0 0x50000000 0 0x00010000 /* downstream I/O (64 KiB) */ - 0x82000000 0 0x50100000 0x0 0x50100000 0 0x07F00000 /* non-prefetchable memory (127 MiB) */ - 0xc2000000 0 0x58000000 0x0 0x58000000 0 0x28000000>; /* prefetchable memory (640 MiB) */ - - clocks = <&bpmp TEGRA186_CLK_AFI>, - <&bpmp TEGRA186_CLK_PCIE>, - <&bpmp TEGRA186_CLK_PLLE>; - clock-names = "afi", "pex", "pll_e"; - - resets = <&bpmp TEGRA186_RESET_AFI>, - <&bpmp TEGRA186_RESET_PCIE>, - <&bpmp TEGRA186_RESET_PCIEXCLK>; - reset-names = "afi", "pex", "pcie_x"; - - status = "disabled"; - - pci@1,0 { - device_type = "pci"; - assigned-addresses = <0x82000800 0 0x10000000 0 0x1000>; - reg = <0x000800 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <2>; - }; - - pci@2,0 { - device_type = "pci"; - assigned-addresses = <0x82001000 0 0x10001000 0 0x1000>; - reg = <0x001000 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <1>; - }; - - pci@3,0 { - device_type = "pci"; - assigned-addresses = <0x82001800 0 0x10004000 0 0x1000>; - reg = <0x001800 0 0 0 0>; - status = "disabled"; - - #address-cells = <3>; - #size-cells = <2>; - ranges; - - nvidia,num-lanes = <1>; - }; - }; - -Board DTS: - - pcie@10003000 { - status = "okay"; - - dvdd-pex-supply = <&vdd_pex>; - hvdd-pex-pll-supply = <&vdd_1v8>; - hvdd-pex-supply = <&vdd_1v8>; - vddio-pexctl-aud-supply = <&vdd_1v8>; - - pci@1,0 { - nvidia,num-lanes = <4>; - status = "okay"; - }; - - pci@2,0 { - nvidia,num-lanes = <0>; - status = "disabled"; - }; - - pci@3,0 { - nvidia,num-lanes = <1>; - status = "disabled"; - }; - }; -- 2.50.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-09-26 7:27 ` [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema Anand Moon @ 2025-09-26 13:56 ` Rob Herring 2025-09-29 7:39 ` Anand Moon 0 siblings, 1 reply; 22+ messages in thread From: Rob Herring @ 2025-09-26 13:56 UTC (permalink / raw) To: Anand Moon Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote: > > Convert the legacy text-based binding documentation for > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following s/YAML/DT/ > the Devicetree Schema format. This improves validation coverage and enables > dtbs_check compliance for Tegra PCIe nodes. Your subject needs some work too. 'existing' and 'bindings documentation' are redundant. > > Cc: Jon Hunter <jonathanh@nvidia.com> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > v1: new patch in this series. > --- > .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ > .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ > 2 files changed, 651 insertions(+), 670 deletions(-) > create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > new file mode 100644 > index 000000000000..dd8cba125b53 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > @@ -0,0 +1,651 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NVIDIA Tegra PCIe Controller > + > +maintainers: > + - Thierry Reding <thierry.reding@gmail.com> > + - Jon Hunter <jonathanh@nvidia.com> > + > +description: | Don't need '|'. > + PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30, > + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and > + platform-specific clock, reset, and power supply configurations. I would suggest not listing every SoC here unless the list is not going to grow. > + > +properties: > + compatible: > + oneOf: Only 1 entry here, don't need 'oneOf'. > + - items: > + - enum: > + - nvidia,tegra20-pcie > + - nvidia,tegra30-pcie > + - nvidia,tegra124-pcie > + - nvidia,tegra210-pcie > + - nvidia,tegra186-pcie > + > + reg: > + items: > + - description: PADS registers > + - description: AFI registers > + - description: Configuration space region > + > + reg-names: > + items: > + - const: pads > + - const: afi > + - const: cs > + > + device_type: > + const: pci Drop. This is covered by pci-host-bridge.yaml. > + > + interrupts: > + items: > + - description: Controller interrupt > + - description: MSI interrupt > + > + interrupt-names: > + items: > + - const: intr > + - const: msi > + > + clocks: > + oneOf: > + - items: > + - description: PCIe clock > + - description: AFI clock > + - description: PLL_E clock Drop this list and add 'minItems: 3' > + - items: > + - description: PCIe clock > + - description: AFI clock > + - description: PLL_E clock > + - description: CML clock > + > + clock-names: > + oneOf: > + - items: > + - const: pex > + - const: afi > + - const: pll_e Same here. > + - items: > + - const: pex > + - const: afi > + - const: pll_e > + - const: cml > + > + resets: > + items: > + - description: PCIe reset > + - description: AFI reset > + - description: PCIe X reset > + > + reset-names: > + items: > + - const: pex > + - const: afi > + - const: pcie_x > + > + power-domains: > + maxItems: 1 > + description: | > + A phandle to the node that controls power to the respective PCIe > + controller and a specifier name for the PCIe controller. Don't need generic descriptions of common properties. Drop. > + > + interconnects: > + minItems: 1 > + maxItems: 2 > + > + interconnect-names: > + minItems: 1 > + maxItems: 2 > + description: > + Should include name of the interconnect path for each interconnect > + entry. Consult TRM documentation for information about available > + memory clients, see DMA CONTROLLER and MEMORY WRITE sections. You have to document what the names are. > + > + pinctrl-names: > + items: > + - const: default > + - const: idle > + > + pinctrl-0: > + $ref: /schemas/types.yaml#/definitions/phandle This already has a type. Just 'pinctrl-0: true' is enough. > + > + pinctrl-1: > + $ref: /schemas/types.yaml#/definitions/phandle > + > + nvidia,num-lanes: > + description: Number of PCIe lanes used > + $ref: /schemas/types.yaml#/definitions/uint32 The examples show this in child nodes. > + > +allOf: > + - $ref: /schemas/pci/pci-host-bridge.yaml# > + - if: > + properties: > + compatible: > + contains: > + enum: > + - nvidia,tegra20-pcie > + - nvidia,tegra186-pcie > + then: > + properties: > + clocks: > + minItems: 3 3 is already the min, so drop. > + maxItems: 3 > + clock-names: > + items: > + - const: pex > + - const: afi > + - const: pll_e Names are already defined, so just 'maxItems: 3' Same comments apply to the rest... > + resets: > + minItems: 3 > + maxItems: 3 > + reset-names: > + items: > + - const: pex > + - const: afi > + - const: pcie_x > + - if: > + properties: > + compatible: > + contains: > + enum: > + - nvidia,tegra30-pcie > + - nvidia,tegra124-pcie > + - nvidia,tegra210-pcie > + then: > + properties: > + clocks: > + minItems: 4 > + maxItems: 4 Just 'minItems' here. > + clock-names: > + items: > + - const: pex > + - const: afi > + - const: pll_e > + - const: cml And here... > + resets: > + minItems: 3 > + maxItems: 3 > + reset-names: > + items: > + - const: pex > + - const: afi > + - const: pcie_x > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - nvidia,tegra20-pcie > + - nvidia,tegra30-pcie > + - nvidia,tegra186-pcie > + then: > + required: > + - power-domains > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - nvidia,tegra186-pcie > + then: > + required: > + - interconnects > + - if: > + properties: > + compatible: > + contains: > + enum: > + - nvidia,tegra210-pcie > + then: > + required: > + - pinctrl-names > + - pinctrl-0 > + - pinctrl-1 > + > +unevaluatedProperties: false > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - clock-names > + - resets > + - reset-names > + - interrupts > + - interrupt-map > + - interrupt-map-mask > + - ranges Already required by pci-host-bridge.yaml. > + - bus-range Generally, bus-range is only required when there's some h/w issue. > + - device_type Already required by pci-host-bridge.yaml. > + - interconnects > + - pinctrl-names Above you said this was conditional. > + - nvidia,num-lanes > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + bus { > + #address-cells = <1>; > + #size-cells = <1>; > + > + pcie@80003000 { > + compatible = "nvidia,tegra20-pcie"; > + device_type = "pci"; > + reg = <0x80003000 0x00000800>, > + <0x80003800 0x00000200>, > + <0x90000000 0x10000000>; > + reg-names = "pads", "afi", "cs"; > + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "intr", "msi"; > + interrupt-parent = <&intc>; > + > + #interrupt-cells = <1>; > + interrupt-map-mask = <0 0 0 0>; > + interrupt-map = <0 0 0 0 &intc GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; > + > + bus-range = <0x00 0xff>; > + #address-cells = <3>; > + #size-cells = <2>; > + > + ranges = <0x02000000 0 0x80000000 0x80000000 0 0x00001000>, > + <0x02000000 0 0x80001000 0x80001000 0 0x00001000>, > + <0x01000000 0 0 0x82000000 0 0x00010000>, > + <0x02000000 0 0xa0000000 0xa0000000 0 0x08000000>, > + <0x42000000 0 0xa8000000 0xa8000000 0 0x18000000>; > + > + clocks = <&tegra_car 70>, > + <&tegra_car 72>, > + <&tegra_car 118>; > + clock-names = "pex", "afi", "pll_e"; > + resets = <&tegra_car 70>, > + <&tegra_car 72>, > + <&tegra_car 74>; > + reset-names = "pex", "afi", "pcie_x"; > + power-domains = <&pd_core>; > + operating-points-v2 = <&pcie_dvfs_opp_table>; > + > + status = "disabled"; Examples must be enabled. > + > + pci@1,0 { > + device_type = "pci"; > + assigned-addresses = <0x82000800 0 0x80000000 0 0x1000>; > + reg = <0x000800 0 0 0 0>; > + bus-range = <0x00 0xff>; > + status = "disabled"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + > + nvidia,num-lanes = <2>; This doesn't match the schema. > + }; > + > + pci@2,0 { > + device_type = "pci"; > + assigned-addresses = <0x82001000 0 0x80001000 0 0x1000>; > + reg = <0x001000 0 0 0 0>; > + bus-range = <0x00 0xff>; > + status = "disabled"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + > + nvidia,num-lanes = <2>; > + }; > + }; > + }; > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + bus { I don't think we need 4 examples. Rob ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-09-26 13:56 ` Rob Herring @ 2025-09-29 7:39 ` Anand Moon 2025-09-29 13:48 ` Rob Herring 0 siblings, 1 reply; 22+ messages in thread From: Anand Moon @ 2025-09-29 7:39 UTC (permalink / raw) To: Rob Herring Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Hi Rob, Thanks for your review comments On Fri, 26 Sept 2025 at 19:26, Rob Herring <robh@kernel.org> wrote: > > On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > Convert the legacy text-based binding documentation for > > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following > > s/YAML/DT/ > Ok, > > the Devicetree Schema format. This improves validation coverage and enables > > dtbs_check compliance for Tegra PCIe nodes. > > Your subject needs some work too. 'existing' and 'bindings > documentation' are redundant. > Here is the simplified version dt-bindings: PCI: Convert the nvidia,tegra-pcie bindings documentation into a YAML schema Convert the existing text-based DT bindings documentation for the NVIDIA Tegra PCIe host controller to a YAML schema format. > > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > v1: new patch in this series. > > --- > > .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ > > .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ > > 2 files changed, 651 insertions(+), 670 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > new file mode 100644 > > index 000000000000..dd8cba125b53 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > @@ -0,0 +1,651 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: NVIDIA Tegra PCIe Controller > > + > > +maintainers: > > + - Thierry Reding <thierry.reding@gmail.com> > > + - Jon Hunter <jonathanh@nvidia.com> > > + > > +description: | > > Don't need '|'. > Ok > > + PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30, > > + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and > > + platform-specific clock, reset, and power supply configurations. > > I would suggest not listing every SoC here unless the list is not going to grow. > Here is the short format. PCIe controller found on NVIDIA Tegra SoCs which supports multiple root ports and platform-specific clock, reset, and power supply configurations. Ok > > + > > +properties: > > + compatible: > > + oneOf: > > Only 1 entry here, don't need 'oneOf'. I am observing the following warning if I remove this. make ARCH=arm64 -j$(nproc) dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml CHKDT ./Documentation/devicetree/bindings /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml: properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie', 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie', 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean' from schema $id: http://json-schema.org/draft-07/schema# /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml: properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie', 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie', 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean' from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > > + - items: > > + - enum: > > + - nvidia,tegra20-pcie > > + - nvidia,tegra30-pcie > > + - nvidia,tegra124-pcie > > + - nvidia,tegra210-pcie > > + - nvidia,tegra186-pcie > > + > > + reg: > > + items: > > + - description: PADS registers > > + - description: AFI registers > > + - description: Configuration space region > > + > > + reg-names: > > + items: > > + - const: pads > > + - const: afi > > + - const: cs > > + > > + device_type: > > + const: pci > > Drop. This is covered by pci-host-bridge.yaml. Ok > > > + > > + interrupts: > > + items: > > + - description: Controller interrupt > > + - description: MSI interrupt > > + > > + interrupt-names: > > + items: > > + - const: intr > > + - const: msi > > + > > + clocks: > > + oneOf: > > + - items: > > + - description: PCIe clock > > + - description: AFI clock > > + - description: PLL_E clock > > Drop this list and add 'minItems: 3' Ok > > > + - items: > > + - description: PCIe clock > > + - description: AFI clock > > + - description: PLL_E clock > > + - description: CML clock > > + > > + clock-names: > > + oneOf: > > + - items: > > + - const: pex > > + - const: afi > > + - const: pll_e > > Same here. Ok these are dumpicate will remove this. > > > + - items: > > + - const: pex > > + - const: afi > > + - const: pll_e > > + - const: cml > > + > > + resets: > > + items: > > + - description: PCIe reset > > + - description: AFI reset > > + - description: PCIe X reset > > + > > + reset-names: > > + items: > > + - const: pex > > + - const: afi > > + - const: pcie_x > > + > > + power-domains: > > + maxItems: 1 > > + description: | > > + A phandle to the node that controls power to the respective PCIe > > + controller and a specifier name for the PCIe controller. > > Don't need generic descriptions of common properties. Drop. > Ok > > + > > + interconnects: > > + minItems: 1 > > + maxItems: 2 > > + > > + interconnect-names: > > + minItems: 1 > > + maxItems: 2 > > + description: > > + Should include name of the interconnect path for each interconnect > > + entry. Consult TRM documentation for information about available > > + memory clients, see DMA CONTROLLER and MEMORY WRITE sections. > > You have to document what the names are. items: - const: dma-mem - const: write Ok. > > > + > > + pinctrl-names: > > + items: > > + - const: default > > + - const: idle > > + > > + pinctrl-0: > > + $ref: /schemas/types.yaml#/definitions/phandle > > This already has a type. Just 'pinctrl-0: true' is enough. > Ok I will drop pinctrl > > + > > + pinctrl-1: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + > > + nvidia,num-lanes: > > + description: Number of PCIe lanes used > > + $ref: /schemas/types.yaml#/definitions/uint32 > > The examples show this in child nodes. yes it patternProperties example I missed this. patternProperties: "^pci@[0-9a-f]+$": type: object properties: reg: maxItems: 1 nvidia,num-lanes: description: Number of PCIe lanes used $ref: /schemas/types.yaml#/definitions/uint32 minimum: 1 unevaluatedProperties: false > > > + > > +allOf: > > + - $ref: /schemas/pci/pci-host-bridge.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - nvidia,tegra20-pcie > > + - nvidia,tegra186-pcie > > + then: > > + properties: > > + clocks: > > + minItems: 3 > > 3 is already the min, so drop. > > > + maxItems: 3 > > + clock-names: > > + items: > > + - const: pex > > + - const: afi > > + - const: pll_e > > Names are already defined, so just 'maxItems: 3' > > Same comments apply to the rest... > Ok correct. > > + resets: > > + minItems: 3 > > + maxItems: 3 > > + reset-names: > > + items: > > + - const: pex > > + - const: afi > > + - const: pcie_x > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - nvidia,tegra30-pcie > > + - nvidia,tegra124-pcie > > + - nvidia,tegra210-pcie > > + then: > > + properties: > > + clocks: > > + minItems: 4 > > + maxItems: 4 > > Just 'minItems' here. > Ok, > > + clock-names: > > + items: > > + - const: pex > > + - const: afi > > + - const: pll_e > > + - const: cml > > And here... > > > + resets: > > + minItems: 3 > > + maxItems: 3 > > + reset-names: > > + items: > > + - const: pex > > + - const: afi > > + - const: pcie_x > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - nvidia,tegra20-pcie > > + - nvidia,tegra30-pcie > > + - nvidia,tegra186-pcie > > + then: > > + required: > > + - power-domains > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - nvidia,tegra186-pcie > > + then: > > + required: > > + - interconnects > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - nvidia,tegra210-pcie > > + then: > > + required: > > + - pinctrl-names > > + - pinctrl-0 > > + - pinctrl-1 > > + > > +unevaluatedProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - reg-names > > + - clocks > > + - clock-names > > + - resets > > + - reset-names > > + - interrupts > > + - interrupt-map > > + - interrupt-map-mask > > + - ranges > > Already required by pci-host-bridge.yaml. > > > + - bus-range > Ok > Generally, bus-range is only required when there's some h/w issue. > > > + - device_type > > Already required by pci-host-bridge.yaml. Ok > > > + - interconnects > > + - pinctrl-names > > Above you said this was conditional. > Ok, I will drop this. > > + - nvidia,num-lanes > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/interrupt-controller/irq.h> > > + > > + bus { > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + pcie@80003000 { > > + compatible = "nvidia,tegra20-pcie"; > > + device_type = "pci"; > > + reg = <0x80003000 0x00000800>, > > + <0x80003800 0x00000200>, > > + <0x90000000 0x10000000>; > > + reg-names = "pads", "afi", "cs"; > > + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-names = "intr", "msi"; > > + interrupt-parent = <&intc>; > > + > > + #interrupt-cells = <1>; > > + interrupt-map-mask = <0 0 0 0>; > > + interrupt-map = <0 0 0 0 &intc GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; > > + > > + bus-range = <0x00 0xff>; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + > > + ranges = <0x02000000 0 0x80000000 0x80000000 0 0x00001000>, > > + <0x02000000 0 0x80001000 0x80001000 0 0x00001000>, > > + <0x01000000 0 0 0x82000000 0 0x00010000>, > > + <0x02000000 0 0xa0000000 0xa0000000 0 0x08000000>, > > + <0x42000000 0 0xa8000000 0xa8000000 0 0x18000000>; > > + > > + clocks = <&tegra_car 70>, > > + <&tegra_car 72>, > > + <&tegra_car 118>; > > + clock-names = "pex", "afi", "pll_e"; > > + resets = <&tegra_car 70>, > > + <&tegra_car 72>, > > + <&tegra_car 74>; > > + reset-names = "pex", "afi", "pcie_x"; > > + power-domains = <&pd_core>; > > + operating-points-v2 = <&pcie_dvfs_opp_table>; > > + > > + status = "disabled"; > > Examples must be enabled. Ok > > > + > > + pci@1,0 { > > + device_type = "pci"; > > + assigned-addresses = <0x82000800 0 0x80000000 0 0x1000>; > > + reg = <0x000800 0 0 0 0>; > > + bus-range = <0x00 0xff>; > > + status = "disabled"; > > + > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + > > + nvidia,num-lanes = <2>; > > This doesn't match the schema. I will try to validate it as a child node with patternProperties. > > > + }; > > + > > + pci@2,0 { > > + device_type = "pci"; > > + assigned-addresses = <0x82001000 0 0x80001000 0 0x1000>; > > + reg = <0x001000 0 0 0 0>; > > + bus-range = <0x00 0xff>; > > + status = "disabled"; > > + > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + > > + nvidia,num-lanes = <2>; > > + }; > > + }; > > + }; > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/interrupt-controller/irq.h> > > + > > + bus { > > I don't think we need 4 examples. Ok nvidia,tegra20-pcie and nvidia,tegra210-pcie should be valid > > Rob Thanks -Anand ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-09-29 7:39 ` Anand Moon @ 2025-09-29 13:48 ` Rob Herring 2025-09-29 15:25 ` Anand Moon 0 siblings, 1 reply; 22+ messages in thread From: Rob Herring @ 2025-09-29 13:48 UTC (permalink / raw) To: Anand Moon Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list On Mon, Sep 29, 2025 at 2:40 AM Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Rob, > > Thanks for your review comments > > On Fri, 26 Sept 2025 at 19:26, Rob Herring <robh@kernel.org> wrote: > > > > On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > Convert the legacy text-based binding documentation for > > > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following > > > > s/YAML/DT/ > > > Ok, > > > the Devicetree Schema format. This improves validation coverage and enables > > > dtbs_check compliance for Tegra PCIe nodes. > > > > Your subject needs some work too. 'existing' and 'bindings > > documentation' are redundant. > > > Here is the simplified version > > dt-bindings: PCI: Convert the nvidia,tegra-pcie bindings documentation > into a YAML schema Still doesn't fit on one line and you say bindings twice: dt-bindings: PCI: Convert nvidia,tegra-pcie to DT schema > > Convert the existing text-based DT bindings documentation for the > NVIDIA Tegra PCIe host controller to a YAML schema format. s/YAML/DT/ Lots of things are YAML. Only one thing is DT schema. > > > > > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > --- > > > v1: new patch in this series. > > > --- > > > .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ > > > .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ > > > 2 files changed, 651 insertions(+), 670 deletions(-) > > > create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > new file mode 100644 > > > index 000000000000..dd8cba125b53 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > @@ -0,0 +1,651 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: NVIDIA Tegra PCIe Controller > > > + > > > +maintainers: > > > + - Thierry Reding <thierry.reding@gmail.com> > > > + - Jon Hunter <jonathanh@nvidia.com> > > > + > > > +description: | > > > > Don't need '|'. > > > Ok > > > + PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30, > > > + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and > > > + platform-specific clock, reset, and power supply configurations. > > > > I would suggest not listing every SoC here unless the list is not going to grow. > > > Here is the short format. > PCIe controller found on NVIDIA Tegra SoCs which supports multiple > root ports and platform-specific clock, reset, and power supply > configurations. > Ok > > > + > > > +properties: > > > + compatible: > > > + oneOf: > > > > Only 1 entry here, don't need 'oneOf'. > > I am observing the following warning if I remove this. > > make ARCH=arm64 -j$(nproc) dt_binding_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > CHKDT ./Documentation/devicetree/bindings > /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml: > properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie', > 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie', > 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean' Because you made 'compatible' a list rather than a schema/map/dict. IOW, You need to remove the '-' as well. > > > + nvidia,num-lanes: > > > + description: Number of PCIe lanes used > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > The examples show this in child nodes. > yes it patternProperties example I missed this. > > patternProperties: > "^pci@[0-9a-f]+$": > type: object > > properties: > reg: > maxItems: 1 > > nvidia,num-lanes: > description: Number of PCIe lanes used > $ref: /schemas/types.yaml#/definitions/uint32 > minimum: 1 > > unevaluatedProperties: false What about all the other properties in the child nodes? You need a $ref to pci-pci-bridge.yaml as well. Rob ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-09-29 13:48 ` Rob Herring @ 2025-09-29 15:25 ` Anand Moon 2025-09-30 14:37 ` Rob Herring 0 siblings, 1 reply; 22+ messages in thread From: Anand Moon @ 2025-09-29 15:25 UTC (permalink / raw) To: Rob Herring Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Hi Rob On Mon, 29 Sept 2025 at 19:19, Rob Herring <robh@kernel.org> wrote: > > On Mon, Sep 29, 2025 at 2:40 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > Hi Rob, > > > > Thanks for your review comments > > > > On Fri, 26 Sept 2025 at 19:26, Rob Herring <robh@kernel.org> wrote: > > > > > > On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > Convert the legacy text-based binding documentation for > > > > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following > > > > > > s/YAML/DT/ > > > > > Ok, > > > > the Devicetree Schema format. This improves validation coverage and enables > > > > dtbs_check compliance for Tegra PCIe nodes. > > > > > > Your subject needs some work too. 'existing' and 'bindings > > > documentation' are redundant. > > > > > Here is the simplified version > > > > dt-bindings: PCI: Convert the nvidia,tegra-pcie bindings documentation > > into a YAML schema > > Still doesn't fit on one line and you say bindings twice: > > dt-bindings: PCI: Convert nvidia,tegra-pcie to DT schema > Ok > > > > Convert the existing text-based DT bindings documentation for the > > NVIDIA Tegra PCIe host controller to a YAML schema format. > > s/YAML/DT/ > > Lots of things are YAML. Only one thing is DT schema. Ok, understood. > > > > > > > > > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > v1: new patch in this series. > > > > --- > > > > .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ > > > > .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ > > > > 2 files changed, 651 insertions(+), 670 deletions(-) > > > > create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > new file mode 100644 > > > > index 000000000000..dd8cba125b53 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > @@ -0,0 +1,651 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: NVIDIA Tegra PCIe Controller > > > > + > > > > +maintainers: > > > > + - Thierry Reding <thierry.reding@gmail.com> > > > > + - Jon Hunter <jonathanh@nvidia.com> > > > > + > > > > +description: | > > > > > > Don't need '|'. > > > > > Ok > > > > + PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30, > > > > + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and > > > > + platform-specific clock, reset, and power supply configurations. > > > > > > I would suggest not listing every SoC here unless the list is not going to grow. > > > > > Here is the short format. > > PCIe controller found on NVIDIA Tegra SoCs which supports multiple > > root ports and platform-specific clock, reset, and power supply > > configurations. > > Ok > > > > + > > > > +properties: > > > > + compatible: > > > > + oneOf: > > > > > > Only 1 entry here, don't need 'oneOf'. > > > > I am observing the following warning if I remove this. > > > > make ARCH=arm64 -j$(nproc) dt_binding_check > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > CHKDT ./Documentation/devicetree/bindings > > /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml: > > properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie', > > 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie', > > 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean' > > Because you made 'compatible' a list rather than a schema/map/dict. > IOW, You need to remove the '-' as well. > Ok fixed. > > > > > + nvidia,num-lanes: > > > > + description: Number of PCIe lanes used > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > The examples show this in child nodes. > > yes it patternProperties example I missed this. > > > > patternProperties: > > "^pci@[0-9a-f]+$": > > type: object > > > > properties: > > reg: > > maxItems: 1 > > > > nvidia,num-lanes: > > description: Number of PCIe lanes used > > $ref: /schemas/types.yaml#/definitions/uint32 > > minimum: 1 > > > > unevaluatedProperties: false > > What about all the other properties in the child nodes? You need a > $ref to pci-pci-bridge.yaml as well. Thanks for the input. patternProperties: "^pci@[0-9a-f]+$": type: object allOf: - $ref: /schemas/pci/pci-host-bridge.yaml# - properties: reg: maxItems: 1 "#address-cells": const: 3 "#size-cells": const: 2 nvidia,num-lanes: description: Number of PCIe lanes used $ref: /schemas/types.yaml#/definitions/uint32 minimum: 1 required: - "#address-cells" - "#size-cells" - nvidia,num-lanes unevaluatedProperties: false > Rob Thanks -Anand ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-09-29 15:25 ` Anand Moon @ 2025-09-30 14:37 ` Rob Herring 2025-09-30 16:32 ` Anand Moon 0 siblings, 1 reply; 22+ messages in thread From: Rob Herring @ 2025-09-30 14:37 UTC (permalink / raw) To: Anand Moon Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list On Mon, Sep 29, 2025 at 10:25 AM Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Rob > > On Mon, 29 Sept 2025 at 19:19, Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Sep 29, 2025 at 2:40 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > Hi Rob, > > > > > > Thanks for your review comments > > > > > > On Fri, 26 Sept 2025 at 19:26, Rob Herring <robh@kernel.org> wrote: > > > > > > > > On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > > > Convert the legacy text-based binding documentation for > > > > > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following > > > > > > > > s/YAML/DT/ > > > > > > > Ok, > > > > > the Devicetree Schema format. This improves validation coverage and enables > > > > > dtbs_check compliance for Tegra PCIe nodes. > > > > > > > > Your subject needs some work too. 'existing' and 'bindings > > > > documentation' are redundant. > > > > > > > Here is the simplified version > > > > > > dt-bindings: PCI: Convert the nvidia,tegra-pcie bindings documentation > > > into a YAML schema > > > > Still doesn't fit on one line and you say bindings twice: > > > > dt-bindings: PCI: Convert nvidia,tegra-pcie to DT schema > > > Ok > > > > > > Convert the existing text-based DT bindings documentation for the > > > NVIDIA Tegra PCIe host controller to a YAML schema format. > > > > s/YAML/DT/ > > > > Lots of things are YAML. Only one thing is DT schema. > Ok, understood. > > > > > > > > > > > > > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > > --- > > > > > v1: new patch in this series. > > > > > --- > > > > > .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ > > > > > .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ > > > > > 2 files changed, 651 insertions(+), 670 deletions(-) > > > > > create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > new file mode 100644 > > > > > index 000000000000..dd8cba125b53 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > @@ -0,0 +1,651 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: NVIDIA Tegra PCIe Controller > > > > > + > > > > > +maintainers: > > > > > + - Thierry Reding <thierry.reding@gmail.com> > > > > > + - Jon Hunter <jonathanh@nvidia.com> > > > > > + > > > > > +description: | > > > > > > > > Don't need '|'. > > > > > > > Ok > > > > > + PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30, > > > > > + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and > > > > > + platform-specific clock, reset, and power supply configurations. > > > > > > > > I would suggest not listing every SoC here unless the list is not going to grow. > > > > > > > Here is the short format. > > > PCIe controller found on NVIDIA Tegra SoCs which supports multiple > > > root ports and platform-specific clock, reset, and power supply > > > configurations. > > > Ok > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + oneOf: > > > > > > > > Only 1 entry here, don't need 'oneOf'. > > > > > > I am observing the following warning if I remove this. > > > > > > make ARCH=arm64 -j$(nproc) dt_binding_check > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > CHKDT ./Documentation/devicetree/bindings > > > /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml: > > > properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie', > > > 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie', > > > 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean' > > > > Because you made 'compatible' a list rather than a schema/map/dict. > > IOW, You need to remove the '-' as well. > > > Ok fixed. > > > > > > > + nvidia,num-lanes: > > > > > + description: Number of PCIe lanes used > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > > > The examples show this in child nodes. > > > yes it patternProperties example I missed this. > > > > > > patternProperties: > > > "^pci@[0-9a-f]+$": > > > type: object > > > > > > properties: > > > reg: > > > maxItems: 1 > > > > > > nvidia,num-lanes: > > > description: Number of PCIe lanes used > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > minimum: 1 > > > > > > unevaluatedProperties: false > > > > What about all the other properties in the child nodes? You need a > > $ref to pci-pci-bridge.yaml as well. > Thanks for the input. > > patternProperties: > "^pci@[0-9a-f]+$": > type: object > allOf: > - $ref: /schemas/pci/pci-host-bridge.yaml# That's not the one you need. Read my reply again. > - properties: properties doesn't need to go under allOf. Actually, don't need allOf here at all. > reg: > maxItems: 1 > "#address-cells": > const: 3 > "#size-cells": > const: 2 These 2 are already defined in the referenced schema. > nvidia,num-lanes: > description: Number of PCIe lanes used > $ref: /schemas/types.yaml#/definitions/uint32 > minimum: 1 I assume there's a maximum <=16? blank line here and between all DT properties. > required: > - "#address-cells" > - "#size-cells" These 2 are already required in the referenced schema. > - nvidia,num-lanes > unevaluatedProperties: false > > > Rob > > Thanks > -Anand > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-09-30 14:37 ` Rob Herring @ 2025-09-30 16:32 ` Anand Moon 2025-10-01 15:33 ` Rob Herring 0 siblings, 1 reply; 22+ messages in thread From: Anand Moon @ 2025-09-30 16:32 UTC (permalink / raw) To: Rob Herring Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Hi Rob On Tue, 30 Sept 2025 at 20:07, Rob Herring <robh@kernel.org> wrote: > > On Mon, Sep 29, 2025 at 10:25 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > Hi Rob > > > > On Mon, 29 Sept 2025 at 19:19, Rob Herring <robh@kernel.org> wrote: > > > > > > On Mon, Sep 29, 2025 at 2:40 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > Hi Rob, > > > > > > > > Thanks for your review comments > > > > > > > > On Fri, 26 Sept 2025 at 19:26, Rob Herring <robh@kernel.org> wrote: > > > > > > > > > > On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > > > > > Convert the legacy text-based binding documentation for > > > > > > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following > > > > > > > > > > s/YAML/DT/ > > > > > > > > > Ok, > > > > > > the Devicetree Schema format. This improves validation coverage and enables > > > > > > dtbs_check compliance for Tegra PCIe nodes. > > > > > > > > > > Your subject needs some work too. 'existing' and 'bindings > > > > > documentation' are redundant. > > > > > > > > > Here is the simplified version > > > > > > > > dt-bindings: PCI: Convert the nvidia,tegra-pcie bindings documentation > > > > into a YAML schema > > > > > > Still doesn't fit on one line and you say bindings twice: > > > > > > dt-bindings: PCI: Convert nvidia,tegra-pcie to DT schema > > > > > Ok > > > > > > > > Convert the existing text-based DT bindings documentation for the > > > > NVIDIA Tegra PCIe host controller to a YAML schema format. > > > > > > s/YAML/DT/ > > > > > > Lots of things are YAML. Only one thing is DT schema. > > Ok, understood. > > > > > > > > > > > > > > > > > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > > > --- > > > > > > v1: new patch in this series. > > > > > > --- > > > > > > .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ > > > > > > .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ > > > > > > 2 files changed, 651 insertions(+), 670 deletions(-) > > > > > > create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > new file mode 100644 > > > > > > index 000000000000..dd8cba125b53 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > @@ -0,0 +1,651 @@ > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: NVIDIA Tegra PCIe Controller > > > > > > + > > > > > > +maintainers: > > > > > > + - Thierry Reding <thierry.reding@gmail.com> > > > > > > + - Jon Hunter <jonathanh@nvidia.com> > > > > > > + > > > > > > +description: | > > > > > > > > > > Don't need '|'. > > > > > > > > > Ok > > > > > > + PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30, > > > > > > + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and > > > > > > + platform-specific clock, reset, and power supply configurations. > > > > > > > > > > I would suggest not listing every SoC here unless the list is not going to grow. > > > > > > > > > Here is the short format. > > > > PCIe controller found on NVIDIA Tegra SoCs which supports multiple > > > > root ports and platform-specific clock, reset, and power supply > > > > configurations. > > > > Ok > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > + oneOf: > > > > > > > > > > Only 1 entry here, don't need 'oneOf'. > > > > > > > > I am observing the following warning if I remove this. > > > > > > > > make ARCH=arm64 -j$(nproc) dt_binding_check > > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > CHKDT ./Documentation/devicetree/bindings > > > > /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml: > > > > properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie', > > > > 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie', > > > > 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean' > > > > > > Because you made 'compatible' a list rather than a schema/map/dict. > > > IOW, You need to remove the '-' as well. > > > > > Ok fixed. > > > > > > > > > + nvidia,num-lanes: > > > > > > + description: Number of PCIe lanes used > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > > > > > The examples show this in child nodes. > > > > yes it patternProperties example I missed this. > > > > > > > > patternProperties: > > > > "^pci@[0-9a-f]+$": > > > > type: object > > > > > > > > properties: > > > > reg: > > > > maxItems: 1 > > > > > > > > nvidia,num-lanes: > > > > description: Number of PCIe lanes used > > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > > minimum: 1 > > > > > > > > unevaluatedProperties: false > > > > > > What about all the other properties in the child nodes? You need a > > > $ref to pci-pci-bridge.yaml as well. > > Thanks for the input. > > > > patternProperties: > > "^pci@[0-9a-f]+$": > > type: object > > allOf: > > - $ref: /schemas/pci/pci-host-bridge.yaml# > > That's not the one you need. Read my reply again. > I'm sorry, I missed pci-pci-bridge.yaml > > - properties: > > properties doesn't need to go under allOf. Actually, don't need allOf > here at all. > > > reg: > > maxItems: 1 > > > "#address-cells": > > const: 3 > > "#size-cells": > > const: 2 > > These 2 are already defined in the referenced schema. Earlier, I had tried to search for these reference schemas, but I could not find them. > > > nvidia,num-lanes: > > description: Number of PCIe lanes used > > $ref: /schemas/types.yaml#/definitions/uint32 > > minimum: 1 > > I assume there's a maximum <=16? I will check and update > > blank line here and between all DT properties. > > > required: > > - "#address-cells" > > - "#size-cells" > > These 2 are already required in the referenced schema. Ok dropped > > > - nvidia,num-lanes > > unevaluatedProperties: false > > > > > Rob Thanks -Anand ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-09-30 16:32 ` Anand Moon @ 2025-10-01 15:33 ` Rob Herring 2025-10-01 18:57 ` Anand Moon 0 siblings, 1 reply; 22+ messages in thread From: Rob Herring @ 2025-10-01 15:33 UTC (permalink / raw) To: Anand Moon Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list On Tue, Sep 30, 2025 at 11:32 AM Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Rob > > On Tue, 30 Sept 2025 at 20:07, Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Sep 29, 2025 at 10:25 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > Hi Rob > > > > > > On Mon, 29 Sept 2025 at 19:19, Rob Herring <robh@kernel.org> wrote: > > > > > > > > On Mon, Sep 29, 2025 at 2:40 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > > > Hi Rob, > > > > > > > > > > Thanks for your review comments > > > > > > > > > > On Fri, 26 Sept 2025 at 19:26, Rob Herring <robh@kernel.org> wrote: > > > > > > > > > > > > On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > > > > > > > Convert the legacy text-based binding documentation for > > > > > > > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following > > > > > > > > > > > > s/YAML/DT/ > > > > > > > > > > > Ok, > > > > > > > the Devicetree Schema format. This improves validation coverage and enables > > > > > > > dtbs_check compliance for Tegra PCIe nodes. > > > > > > > > > > > > Your subject needs some work too. 'existing' and 'bindings > > > > > > documentation' are redundant. > > > > > > > > > > > Here is the simplified version > > > > > > > > > > dt-bindings: PCI: Convert the nvidia,tegra-pcie bindings documentation > > > > > into a YAML schema > > > > > > > > Still doesn't fit on one line and you say bindings twice: > > > > > > > > dt-bindings: PCI: Convert nvidia,tegra-pcie to DT schema > > > > > > > Ok > > > > > > > > > > Convert the existing text-based DT bindings documentation for the > > > > > NVIDIA Tegra PCIe host controller to a YAML schema format. > > > > > > > > s/YAML/DT/ > > > > > > > > Lots of things are YAML. Only one thing is DT schema. > > > Ok, understood. > > > > > > > > > > > > > > > > > > > > > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > > > > --- > > > > > > > v1: new patch in this series. > > > > > > > --- > > > > > > > .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ > > > > > > > .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ > > > > > > > 2 files changed, 651 insertions(+), 670 deletions(-) > > > > > > > create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > > delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > > new file mode 100644 > > > > > > > index 000000000000..dd8cba125b53 > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > > @@ -0,0 +1,651 @@ > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > > +%YAML 1.2 > > > > > > > +--- > > > > > > > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > + > > > > > > > +title: NVIDIA Tegra PCIe Controller > > > > > > > + > > > > > > > +maintainers: > > > > > > > + - Thierry Reding <thierry.reding@gmail.com> > > > > > > > + - Jon Hunter <jonathanh@nvidia.com> > > > > > > > + > > > > > > > +description: | > > > > > > > > > > > > Don't need '|'. > > > > > > > > > > > Ok > > > > > > > + PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30, > > > > > > > + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and > > > > > > > + platform-specific clock, reset, and power supply configurations. > > > > > > > > > > > > I would suggest not listing every SoC here unless the list is not going to grow. > > > > > > > > > > > Here is the short format. > > > > > PCIe controller found on NVIDIA Tegra SoCs which supports multiple > > > > > root ports and platform-specific clock, reset, and power supply > > > > > configurations. > > > > > Ok > > > > > > > + > > > > > > > +properties: > > > > > > > + compatible: > > > > > > > + oneOf: > > > > > > > > > > > > Only 1 entry here, don't need 'oneOf'. > > > > > > > > > > I am observing the following warning if I remove this. > > > > > > > > > > make ARCH=arm64 -j$(nproc) dt_binding_check > > > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > CHKDT ./Documentation/devicetree/bindings > > > > > /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml: > > > > > properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie', > > > > > 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie', > > > > > 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean' > > > > > > > > Because you made 'compatible' a list rather than a schema/map/dict. > > > > IOW, You need to remove the '-' as well. > > > > > > > Ok fixed. > > > > > > > > > > > + nvidia,num-lanes: > > > > > > > + description: Number of PCIe lanes used > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > > > > > > > The examples show this in child nodes. > > > > > yes it patternProperties example I missed this. > > > > > > > > > > patternProperties: > > > > > "^pci@[0-9a-f]+$": > > > > > type: object > > > > > > > > > > properties: > > > > > reg: > > > > > maxItems: 1 > > > > > > > > > > nvidia,num-lanes: > > > > > description: Number of PCIe lanes used > > > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > > > minimum: 1 > > > > > > > > > > unevaluatedProperties: false > > > > > > > > What about all the other properties in the child nodes? You need a > > > > $ref to pci-pci-bridge.yaml as well. > > > Thanks for the input. > > > > > > patternProperties: > > > "^pci@[0-9a-f]+$": > > > type: object > > > allOf: > > > - $ref: /schemas/pci/pci-host-bridge.yaml# > > > > That's not the one you need. Read my reply again. > > > I'm sorry, I missed pci-pci-bridge.yaml > > > - properties: > > > > properties doesn't need to go under allOf. Actually, don't need allOf > > here at all. > > > > > reg: > > > maxItems: 1 > > > > > "#address-cells": > > > const: 3 > > > "#size-cells": > > > const: 2 > > > > These 2 are already defined in the referenced schema. > Earlier, I had tried to search for these reference schemas, > but I could not find them. They are part of dtschema, so they are on your system wherever it is installed. Most common schemas are in dtschema. Rob ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema 2025-10-01 15:33 ` Rob Herring @ 2025-10-01 18:57 ` Anand Moon 0 siblings, 0 replies; 22+ messages in thread From: Anand Moon @ 2025-10-01 18:57 UTC (permalink / raw) To: Rob Herring Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Hi Rob, On Wed, 1 Oct 2025 at 21:04, Rob Herring <robh@kernel.org> wrote: > > On Tue, Sep 30, 2025 at 11:32 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > Hi Rob > > > > On Tue, 30 Sept 2025 at 20:07, Rob Herring <robh@kernel.org> wrote: > > > > > > On Mon, Sep 29, 2025 at 10:25 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > Hi Rob > > > > > > > > On Mon, 29 Sept 2025 at 19:19, Rob Herring <robh@kernel.org> wrote: > > > > > > > > > > On Mon, Sep 29, 2025 at 2:40 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > > > > > Hi Rob, > > > > > > > > > > > > Thanks for your review comments > > > > > > > > > > > > On Fri, 26 Sept 2025 at 19:26, Rob Herring <robh@kernel.org> wrote: > > > > > > > > > > > > > > On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > > > > > > > > > Convert the legacy text-based binding documentation for > > > > > > > > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following > > > > > > > > > > > > > > s/YAML/DT/ > > > > > > > > > > > > > Ok, > > > > > > > > the Devicetree Schema format. This improves validation coverage and enables > > > > > > > > dtbs_check compliance for Tegra PCIe nodes. > > > > > > > > > > > > > > Your subject needs some work too. 'existing' and 'bindings > > > > > > > documentation' are redundant. > > > > > > > > > > > > > Here is the simplified version > > > > > > > > > > > > dt-bindings: PCI: Convert the nvidia,tegra-pcie bindings documentation > > > > > > into a YAML schema > > > > > > > > > > Still doesn't fit on one line and you say bindings twice: > > > > > > > > > > dt-bindings: PCI: Convert nvidia,tegra-pcie to DT schema > > > > > > > > > Ok > > > > > > > > > > > > Convert the existing text-based DT bindings documentation for the > > > > > > NVIDIA Tegra PCIe host controller to a YAML schema format. > > > > > > > > > > s/YAML/DT/ > > > > > > > > > > Lots of things are YAML. Only one thing is DT schema. > > > > Ok, understood. > > > > > > > > > > > > > > > > > > > > > > > > > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > > > > > --- > > > > > > > > v1: new patch in this series. > > > > > > > > --- > > > > > > > > .../bindings/pci/nvidia,tegra-pcie.yaml | 651 +++++++++++++++++ > > > > > > > > .../bindings/pci/nvidia,tegra20-pcie.txt | 670 ------------------ > > > > > > > > 2 files changed, 651 insertions(+), 670 deletions(-) > > > > > > > > create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > > > delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > > > new file mode 100644 > > > > > > > > index 000000000000..dd8cba125b53 > > > > > > > > --- /dev/null > > > > > > > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > > > @@ -0,0 +1,651 @@ > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > > > +%YAML 1.2 > > > > > > > > +--- > > > > > > > > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml# > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > > + > > > > > > > > +title: NVIDIA Tegra PCIe Controller > > > > > > > > + > > > > > > > > +maintainers: > > > > > > > > + - Thierry Reding <thierry.reding@gmail.com> > > > > > > > > + - Jon Hunter <jonathanh@nvidia.com> > > > > > > > > + > > > > > > > > +description: | > > > > > > > > > > > > > > Don't need '|'. > > > > > > > > > > > > > Ok > > > > > > > > + PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30, > > > > > > > > + Tegra124, Tegra210, and Tegra186. Supports multiple root ports and > > > > > > > > + platform-specific clock, reset, and power supply configurations. > > > > > > > > > > > > > > I would suggest not listing every SoC here unless the list is not going to grow. > > > > > > > > > > > > > Here is the short format. > > > > > > PCIe controller found on NVIDIA Tegra SoCs which supports multiple > > > > > > root ports and platform-specific clock, reset, and power supply > > > > > > configurations. > > > > > > Ok > > > > > > > > + > > > > > > > > +properties: > > > > > > > > + compatible: > > > > > > > > + oneOf: > > > > > > > > > > > > > > Only 1 entry here, don't need 'oneOf'. > > > > > > > > > > > > I am observing the following warning if I remove this. > > > > > > > > > > > > make ARCH=arm64 -j$(nproc) dt_binding_check > > > > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml > > > > > > CHKDT ./Documentation/devicetree/bindings > > > > > > /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml: > > > > > > properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie', > > > > > > 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie', > > > > > > 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean' > > > > > > > > > > Because you made 'compatible' a list rather than a schema/map/dict. > > > > > IOW, You need to remove the '-' as well. > > > > > > > > > Ok fixed. > > > > > > > > > > > > > + nvidia,num-lanes: > > > > > > > > + description: Number of PCIe lanes used > > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > > > > > > > > > The examples show this in child nodes. > > > > > > yes it patternProperties example I missed this. > > > > > > > > > > > > patternProperties: > > > > > > "^pci@[0-9a-f]+$": > > > > > > type: object > > > > > > > > > > > > properties: > > > > > > reg: > > > > > > maxItems: 1 > > > > > > > > > > > > nvidia,num-lanes: > > > > > > description: Number of PCIe lanes used > > > > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > minimum: 1 > > > > > > > > > > > > unevaluatedProperties: false > > > > > > > > > > What about all the other properties in the child nodes? You need a > > > > > $ref to pci-pci-bridge.yaml as well. > > > > Thanks for the input. > > > > > > > > patternProperties: > > > > "^pci@[0-9a-f]+$": > > > > type: object > > > > allOf: > > > > - $ref: /schemas/pci/pci-host-bridge.yaml# > > > > > > That's not the one you need. Read my reply again. > > > > > I'm sorry, I missed pci-pci-bridge.yaml > > > > - properties: > > > > > > properties doesn't need to go under allOf. Actually, don't need allOf > > > here at all. > > > > > > > reg: > > > > maxItems: 1 > > > > > > > "#address-cells": > > > > const: 3 > > > > "#size-cells": > > > > const: 2 > > > > > > These 2 are already defined in the referenced schema. > > Earlier, I had tried to search for these reference schemas, > > but I could not find them. > > They are part of dtschema, so they are on your system wherever it is > installed. Most common schemas are in dtschema. > Thanks, it's preset in the .local dir. $ .local/lib/python3.13/site-packages/dtschema/schemas/ > Rob Thanks -Anand ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions 2025-09-26 7:27 [PATCH v1 0/5] PCI: tegra: A couple of cleanups Anand Moon 2025-09-26 7:27 ` [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema Anand Moon @ 2025-09-26 7:27 ` Anand Moon 2025-09-26 18:12 ` Frank Li 2025-09-26 7:27 ` [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling Anand Moon ` (2 subsequent siblings) 4 siblings, 1 reply; 22+ messages in thread From: Anand Moon @ 2025-09-26 7:27 UTC (permalink / raw) To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Cc: Anand Moon Currently, the driver acquires clocks and prepare/enable/disable/unprepare the clocks individually thereby making the driver complex to read. The driver can be simplified by using the clk_bulk*() APIs. Use: - devm_clk_bulk_get() API to acquire all the clocks - clk_bulk_prepare_enable() to prepare/enable clocks - clk_bulk_disable_unprepare() APIs to disable/unprepare them in bulk Following change also removes the legacy has_cml_clk flag and its associated conditional logic. Instead, the driver now relies on the clock definitions from the device tree to determine the correct clock sequencing. This reduces hardcoded dependencies and improves the driver's maintainability. Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Jon Hunter <jonathanh@nvidia.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- v1: Switch from devm_clk_bulk_get_all() -> devm_clk_bulk_get() with fix clks array. nvidia,tegra20-pcie and nvidia,tegra186-pcie uses three clocks pex, afi, pll_e where as nvidia,tegra30-pcie, nvidia,tegra124-pcie, nvidia,tegra210-pcie uses four clks pex, afi, pll_e, cml --- --- drivers/pci/controller/pci-tegra.c | 100 +++++++++++++---------------- 1 file changed, 45 insertions(+), 55 deletions(-) diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index 467ddc701adc..07a61d902eae 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -287,6 +287,8 @@ struct tegra_pcie_port_soc { struct tegra_pcie_soc { unsigned int num_ports; const struct tegra_pcie_port_soc *ports; + const char * const *clk_names; + unsigned int num_clks; unsigned int msi_base_shift; unsigned long afi_pex2_ctrl; u32 pads_pll_ctl; @@ -297,7 +299,6 @@ struct tegra_pcie_soc { bool has_pex_clkreq_en; bool has_pex_bias_ctrl; bool has_intr_prsnt_sense; - bool has_cml_clk; bool has_gen2; bool force_pca_enable; bool program_uphy; @@ -330,10 +331,7 @@ struct tegra_pcie { struct resource cs; - struct clk *pex_clk; - struct clk *afi_clk; - struct clk *pll_e; - struct clk *cml_clk; + struct clk_bulk_data *clks; struct reset_control *pex_rst; struct reset_control *afi_rst; @@ -1158,10 +1156,7 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie) reset_control_assert(pcie->afi_rst); - clk_disable_unprepare(pcie->pll_e); - if (soc->has_cml_clk) - clk_disable_unprepare(pcie->cml_clk); - clk_disable_unprepare(pcie->afi_clk); + clk_bulk_disable_unprepare(soc->num_clks, pcie->clks); if (!dev->pm_domain) tegra_powergate_power_off(TEGRA_POWERGATE_PCIE); @@ -1202,35 +1197,16 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie) } } - err = clk_prepare_enable(pcie->afi_clk); + err = clk_bulk_prepare_enable(soc->num_clks, pcie->clks); if (err < 0) { - dev_err(dev, "failed to enable AFI clock: %d\n", err); + dev_err(dev, "filed to enable clocks: %d\n", err); goto powergate; } - if (soc->has_cml_clk) { - err = clk_prepare_enable(pcie->cml_clk); - if (err < 0) { - dev_err(dev, "failed to enable CML clock: %d\n", err); - goto disable_afi_clk; - } - } - - err = clk_prepare_enable(pcie->pll_e); - if (err < 0) { - dev_err(dev, "failed to enable PLLE clock: %d\n", err); - goto disable_cml_clk; - } - reset_control_deassert(pcie->afi_rst); return 0; -disable_cml_clk: - if (soc->has_cml_clk) - clk_disable_unprepare(pcie->cml_clk); -disable_afi_clk: - clk_disable_unprepare(pcie->afi_clk); powergate: if (!dev->pm_domain) tegra_powergate_power_off(TEGRA_POWERGATE_PCIE); @@ -1255,26 +1231,21 @@ static int tegra_pcie_clocks_get(struct tegra_pcie *pcie) { struct device *dev = pcie->dev; const struct tegra_pcie_soc *soc = pcie->soc; + int ret, i; - pcie->pex_clk = devm_clk_get(dev, "pex"); - if (IS_ERR(pcie->pex_clk)) - return PTR_ERR(pcie->pex_clk); - - pcie->afi_clk = devm_clk_get(dev, "afi"); - if (IS_ERR(pcie->afi_clk)) - return PTR_ERR(pcie->afi_clk); + pcie->clks = devm_kcalloc(dev, soc->num_clks, sizeof(*pcie->clks), + GFP_KERNEL); + if (!pcie->clks) + return -ENOMEM; - pcie->pll_e = devm_clk_get(dev, "pll_e"); - if (IS_ERR(pcie->pll_e)) - return PTR_ERR(pcie->pll_e); + for (i = 0; i < soc->num_clks; i++) + pcie->clks[i].id = soc->clk_names[i]; - if (soc->has_cml_clk) { - pcie->cml_clk = devm_clk_get(dev, "cml"); - if (IS_ERR(pcie->cml_clk)) - return PTR_ERR(pcie->cml_clk); - } + ret = devm_clk_bulk_get(dev, soc->num_clks, pcie->clks); + if (ret) + dev_err(dev, "failed to get PCIe clocks: %d\n", ret); - return 0; + return ret; } static int tegra_pcie_resets_get(struct tegra_pcie *pcie) @@ -2335,9 +2306,17 @@ static const struct tegra_pcie_port_soc tegra20_pcie_ports[] = { { .pme.turnoff_bit = 8, .pme.ack_bit = 10 }, }; +static const char * const tegra20_pcie_clks[] = { + "pex", + "afi", + "pll_e", +}; + static const struct tegra_pcie_soc tegra20_pcie = { .num_ports = 2, .ports = tegra20_pcie_ports, + .clk_names = tegra20_pcie_clks, + .num_clks = ARRAY_SIZE(tegra20_pcie_clks), .msi_base_shift = 0, .pads_pll_ctl = PADS_PLL_CTL_TEGRA20, .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10, @@ -2345,7 +2324,6 @@ static const struct tegra_pcie_soc tegra20_pcie = { .has_pex_clkreq_en = false, .has_pex_bias_ctrl = false, .has_intr_prsnt_sense = false, - .has_cml_clk = false, .has_gen2 = false, .force_pca_enable = false, .program_uphy = true, @@ -2356,6 +2334,13 @@ static const struct tegra_pcie_soc tegra20_pcie = { .ectl.enable = false, }; +static const char * const tegra30_pcie_clks[] = { + "pex", + "afi", + "pll_e", + "cml", +}; + static const struct tegra_pcie_port_soc tegra30_pcie_ports[] = { { .pme.turnoff_bit = 0, .pme.ack_bit = 5 }, { .pme.turnoff_bit = 8, .pme.ack_bit = 10 }, @@ -2365,6 +2350,8 @@ static const struct tegra_pcie_port_soc tegra30_pcie_ports[] = { static const struct tegra_pcie_soc tegra30_pcie = { .num_ports = 3, .ports = tegra30_pcie_ports, + .clk_names = tegra30_pcie_clks, + .num_clks = ARRAY_SIZE(tegra30_pcie_clks), .msi_base_shift = 8, .afi_pex2_ctrl = 0x128, .pads_pll_ctl = PADS_PLL_CTL_TEGRA30, @@ -2374,7 +2361,6 @@ static const struct tegra_pcie_soc tegra30_pcie = { .has_pex_clkreq_en = true, .has_pex_bias_ctrl = true, .has_intr_prsnt_sense = true, - .has_cml_clk = true, .has_gen2 = false, .force_pca_enable = false, .program_uphy = true, @@ -2388,6 +2374,8 @@ static const struct tegra_pcie_soc tegra30_pcie = { static const struct tegra_pcie_soc tegra124_pcie = { .num_ports = 2, .ports = tegra20_pcie_ports, + .clk_names = tegra30_pcie_clks, + .num_clks = ARRAY_SIZE(tegra30_pcie_clks), .msi_base_shift = 8, .pads_pll_ctl = PADS_PLL_CTL_TEGRA30, .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN, @@ -2395,7 +2383,6 @@ static const struct tegra_pcie_soc tegra124_pcie = { .has_pex_clkreq_en = true, .has_pex_bias_ctrl = true, .has_intr_prsnt_sense = true, - .has_cml_clk = true, .has_gen2 = true, .force_pca_enable = false, .program_uphy = true, @@ -2409,6 +2396,8 @@ static const struct tegra_pcie_soc tegra124_pcie = { static const struct tegra_pcie_soc tegra210_pcie = { .num_ports = 2, .ports = tegra20_pcie_ports, + .clk_names = tegra30_pcie_clks, + .num_clks = ARRAY_SIZE(tegra30_pcie_clks), .msi_base_shift = 8, .pads_pll_ctl = PADS_PLL_CTL_TEGRA30, .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN, @@ -2418,7 +2407,6 @@ static const struct tegra_pcie_soc tegra210_pcie = { .has_pex_clkreq_en = true, .has_pex_bias_ctrl = true, .has_intr_prsnt_sense = true, - .has_cml_clk = true, .has_gen2 = true, .force_pca_enable = true, .program_uphy = true, @@ -2450,6 +2438,8 @@ static const struct tegra_pcie_port_soc tegra186_pcie_ports[] = { static const struct tegra_pcie_soc tegra186_pcie = { .num_ports = 3, .ports = tegra186_pcie_ports, + .clk_names = tegra20_pcie_clks, + .num_clks = ARRAY_SIZE(tegra20_pcie_clks), .msi_base_shift = 8, .afi_pex2_ctrl = 0x19c, .pads_pll_ctl = PADS_PLL_CTL_TEGRA30, @@ -2459,7 +2449,6 @@ static const struct tegra_pcie_soc tegra186_pcie = { .has_pex_clkreq_en = true, .has_pex_bias_ctrl = true, .has_intr_prsnt_sense = true, - .has_cml_clk = false, .has_gen2 = true, .force_pca_enable = false, .program_uphy = false, @@ -2651,6 +2640,7 @@ static void tegra_pcie_remove(struct platform_device *pdev) static int tegra_pcie_pm_suspend(struct device *dev) { struct tegra_pcie *pcie = dev_get_drvdata(dev); + const struct tegra_pcie_soc *soc = pcie->soc; struct tegra_pcie_port *port; int err; @@ -2672,7 +2662,7 @@ static int tegra_pcie_pm_suspend(struct device *dev) } reset_control_assert(pcie->pex_rst); - clk_disable_unprepare(pcie->pex_clk); + clk_bulk_disable_unprepare(soc->num_clks, pcie->clks); if (IS_ENABLED(CONFIG_PCI_MSI)) tegra_pcie_disable_msi(pcie); @@ -2686,6 +2676,7 @@ static int tegra_pcie_pm_suspend(struct device *dev) static int tegra_pcie_pm_resume(struct device *dev) { struct tegra_pcie *pcie = dev_get_drvdata(dev); + const struct tegra_pcie_soc *soc = pcie->soc; int err; err = tegra_pcie_power_on(pcie); @@ -2706,9 +2697,9 @@ static int tegra_pcie_pm_resume(struct device *dev) if (IS_ENABLED(CONFIG_PCI_MSI)) tegra_pcie_enable_msi(pcie); - err = clk_prepare_enable(pcie->pex_clk); + err = clk_bulk_prepare_enable(soc->num_clks, pcie->clks); if (err) { - dev_err(dev, "failed to enable PEX clock: %d\n", err); + dev_err(dev, "failed to enable clock: %d\n", err); goto pex_dpd_enable; } @@ -2729,7 +2720,6 @@ static int tegra_pcie_pm_resume(struct device *dev) disable_pex_clk: reset_control_assert(pcie->pex_rst); - clk_disable_unprepare(pcie->pex_clk); pex_dpd_enable: pinctrl_pm_select_idle_state(dev); poweroff: -- 2.50.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions 2025-09-26 7:27 ` [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions Anand Moon @ 2025-09-26 18:12 ` Frank Li 2025-09-27 5:50 ` Anand Moon 0 siblings, 1 reply; 22+ messages in thread From: Frank Li @ 2025-09-26 18:12 UTC (permalink / raw) To: Anand Moon Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list On Fri, Sep 26, 2025 at 12:57:43PM +0530, Anand Moon wrote: > Currently, the driver acquires clocks and prepare/enable/disable/unprepare > the clocks individually thereby making the driver complex to read. > > The driver can be simplified by using the clk_bulk*() APIs. > > Use: > - devm_clk_bulk_get() API to acquire all the clocks > - clk_bulk_prepare_enable() to prepare/enable clocks > - clk_bulk_disable_unprepare() APIs to disable/unprepare them in bulk > > Following change also removes the legacy has_cml_clk flag and its associated > conditional logic. Instead, the driver now relies on the clock definitions from > the device tree to determine the correct clock sequencing. > This reduces hardcoded dependencies and improves the driver's maintainability. > > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Jon Hunter <jonathanh@nvidia.com> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > v1: Switch from devm_clk_bulk_get_all() -> devm_clk_bulk_get() with > fix clks array. why not use devm_clk_bulk_get_all()? Frank > > nvidia,tegra20-pcie and nvidia,tegra186-pcie uses three clocks > pex, afi, pll_e > where as nvidia,tegra30-pcie, nvidia,tegra124-pcie, nvidia,tegra210-pcie > uses four clks > pex, afi, pll_e, cml > --- > --- > drivers/pci/controller/pci-tegra.c | 100 +++++++++++++---------------- > 1 file changed, 45 insertions(+), 55 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index 467ddc701adc..07a61d902eae 100644 > --- a/drivers/pci/controller/pci-tegra.c > +++ b/drivers/pci/controller/pci-tegra.c > @@ -287,6 +287,8 @@ struct tegra_pcie_port_soc { > struct tegra_pcie_soc { > unsigned int num_ports; > const struct tegra_pcie_port_soc *ports; > + const char * const *clk_names; > + unsigned int num_clks; > unsigned int msi_base_shift; > unsigned long afi_pex2_ctrl; > u32 pads_pll_ctl; > @@ -297,7 +299,6 @@ struct tegra_pcie_soc { > bool has_pex_clkreq_en; > bool has_pex_bias_ctrl; > bool has_intr_prsnt_sense; > - bool has_cml_clk; > bool has_gen2; > bool force_pca_enable; > bool program_uphy; > @@ -330,10 +331,7 @@ struct tegra_pcie { > > struct resource cs; > > - struct clk *pex_clk; > - struct clk *afi_clk; > - struct clk *pll_e; > - struct clk *cml_clk; > + struct clk_bulk_data *clks; > > struct reset_control *pex_rst; > struct reset_control *afi_rst; > @@ -1158,10 +1156,7 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie) > > reset_control_assert(pcie->afi_rst); > > - clk_disable_unprepare(pcie->pll_e); > - if (soc->has_cml_clk) > - clk_disable_unprepare(pcie->cml_clk); > - clk_disable_unprepare(pcie->afi_clk); > + clk_bulk_disable_unprepare(soc->num_clks, pcie->clks); > > if (!dev->pm_domain) > tegra_powergate_power_off(TEGRA_POWERGATE_PCIE); > @@ -1202,35 +1197,16 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie) > } > } > > - err = clk_prepare_enable(pcie->afi_clk); > + err = clk_bulk_prepare_enable(soc->num_clks, pcie->clks); > if (err < 0) { > - dev_err(dev, "failed to enable AFI clock: %d\n", err); > + dev_err(dev, "filed to enable clocks: %d\n", err); > goto powergate; > } > > - if (soc->has_cml_clk) { > - err = clk_prepare_enable(pcie->cml_clk); > - if (err < 0) { > - dev_err(dev, "failed to enable CML clock: %d\n", err); > - goto disable_afi_clk; > - } > - } > - > - err = clk_prepare_enable(pcie->pll_e); > - if (err < 0) { > - dev_err(dev, "failed to enable PLLE clock: %d\n", err); > - goto disable_cml_clk; > - } > - > reset_control_deassert(pcie->afi_rst); > > return 0; > > -disable_cml_clk: > - if (soc->has_cml_clk) > - clk_disable_unprepare(pcie->cml_clk); > -disable_afi_clk: > - clk_disable_unprepare(pcie->afi_clk); > powergate: > if (!dev->pm_domain) > tegra_powergate_power_off(TEGRA_POWERGATE_PCIE); > @@ -1255,26 +1231,21 @@ static int tegra_pcie_clocks_get(struct tegra_pcie *pcie) > { > struct device *dev = pcie->dev; > const struct tegra_pcie_soc *soc = pcie->soc; > + int ret, i; > > - pcie->pex_clk = devm_clk_get(dev, "pex"); > - if (IS_ERR(pcie->pex_clk)) > - return PTR_ERR(pcie->pex_clk); > - > - pcie->afi_clk = devm_clk_get(dev, "afi"); > - if (IS_ERR(pcie->afi_clk)) > - return PTR_ERR(pcie->afi_clk); > + pcie->clks = devm_kcalloc(dev, soc->num_clks, sizeof(*pcie->clks), > + GFP_KERNEL); > + if (!pcie->clks) > + return -ENOMEM; > > - pcie->pll_e = devm_clk_get(dev, "pll_e"); > - if (IS_ERR(pcie->pll_e)) > - return PTR_ERR(pcie->pll_e); > + for (i = 0; i < soc->num_clks; i++) > + pcie->clks[i].id = soc->clk_names[i]; > > - if (soc->has_cml_clk) { > - pcie->cml_clk = devm_clk_get(dev, "cml"); > - if (IS_ERR(pcie->cml_clk)) > - return PTR_ERR(pcie->cml_clk); > - } > + ret = devm_clk_bulk_get(dev, soc->num_clks, pcie->clks); > + if (ret) > + dev_err(dev, "failed to get PCIe clocks: %d\n", ret); > > - return 0; > + return ret; > } > > static int tegra_pcie_resets_get(struct tegra_pcie *pcie) > @@ -2335,9 +2306,17 @@ static const struct tegra_pcie_port_soc tegra20_pcie_ports[] = { > { .pme.turnoff_bit = 8, .pme.ack_bit = 10 }, > }; > > +static const char * const tegra20_pcie_clks[] = { > + "pex", > + "afi", > + "pll_e", > +}; > + > static const struct tegra_pcie_soc tegra20_pcie = { > .num_ports = 2, > .ports = tegra20_pcie_ports, > + .clk_names = tegra20_pcie_clks, > + .num_clks = ARRAY_SIZE(tegra20_pcie_clks), > .msi_base_shift = 0, > .pads_pll_ctl = PADS_PLL_CTL_TEGRA20, > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10, > @@ -2345,7 +2324,6 @@ static const struct tegra_pcie_soc tegra20_pcie = { > .has_pex_clkreq_en = false, > .has_pex_bias_ctrl = false, > .has_intr_prsnt_sense = false, > - .has_cml_clk = false, > .has_gen2 = false, > .force_pca_enable = false, > .program_uphy = true, > @@ -2356,6 +2334,13 @@ static const struct tegra_pcie_soc tegra20_pcie = { > .ectl.enable = false, > }; > > +static const char * const tegra30_pcie_clks[] = { > + "pex", > + "afi", > + "pll_e", > + "cml", > +}; > + > static const struct tegra_pcie_port_soc tegra30_pcie_ports[] = { > { .pme.turnoff_bit = 0, .pme.ack_bit = 5 }, > { .pme.turnoff_bit = 8, .pme.ack_bit = 10 }, > @@ -2365,6 +2350,8 @@ static const struct tegra_pcie_port_soc tegra30_pcie_ports[] = { > static const struct tegra_pcie_soc tegra30_pcie = { > .num_ports = 3, > .ports = tegra30_pcie_ports, > + .clk_names = tegra30_pcie_clks, > + .num_clks = ARRAY_SIZE(tegra30_pcie_clks), > .msi_base_shift = 8, > .afi_pex2_ctrl = 0x128, > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30, > @@ -2374,7 +2361,6 @@ static const struct tegra_pcie_soc tegra30_pcie = { > .has_pex_clkreq_en = true, > .has_pex_bias_ctrl = true, > .has_intr_prsnt_sense = true, > - .has_cml_clk = true, > .has_gen2 = false, > .force_pca_enable = false, > .program_uphy = true, > @@ -2388,6 +2374,8 @@ static const struct tegra_pcie_soc tegra30_pcie = { > static const struct tegra_pcie_soc tegra124_pcie = { > .num_ports = 2, > .ports = tegra20_pcie_ports, > + .clk_names = tegra30_pcie_clks, > + .num_clks = ARRAY_SIZE(tegra30_pcie_clks), > .msi_base_shift = 8, > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30, > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN, > @@ -2395,7 +2383,6 @@ static const struct tegra_pcie_soc tegra124_pcie = { > .has_pex_clkreq_en = true, > .has_pex_bias_ctrl = true, > .has_intr_prsnt_sense = true, > - .has_cml_clk = true, > .has_gen2 = true, > .force_pca_enable = false, > .program_uphy = true, > @@ -2409,6 +2396,8 @@ static const struct tegra_pcie_soc tegra124_pcie = { > static const struct tegra_pcie_soc tegra210_pcie = { > .num_ports = 2, > .ports = tegra20_pcie_ports, > + .clk_names = tegra30_pcie_clks, > + .num_clks = ARRAY_SIZE(tegra30_pcie_clks), > .msi_base_shift = 8, > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30, > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN, > @@ -2418,7 +2407,6 @@ static const struct tegra_pcie_soc tegra210_pcie = { > .has_pex_clkreq_en = true, > .has_pex_bias_ctrl = true, > .has_intr_prsnt_sense = true, > - .has_cml_clk = true, > .has_gen2 = true, > .force_pca_enable = true, > .program_uphy = true, > @@ -2450,6 +2438,8 @@ static const struct tegra_pcie_port_soc tegra186_pcie_ports[] = { > static const struct tegra_pcie_soc tegra186_pcie = { > .num_ports = 3, > .ports = tegra186_pcie_ports, > + .clk_names = tegra20_pcie_clks, > + .num_clks = ARRAY_SIZE(tegra20_pcie_clks), > .msi_base_shift = 8, > .afi_pex2_ctrl = 0x19c, > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30, > @@ -2459,7 +2449,6 @@ static const struct tegra_pcie_soc tegra186_pcie = { > .has_pex_clkreq_en = true, > .has_pex_bias_ctrl = true, > .has_intr_prsnt_sense = true, > - .has_cml_clk = false, > .has_gen2 = true, > .force_pca_enable = false, > .program_uphy = false, > @@ -2651,6 +2640,7 @@ static void tegra_pcie_remove(struct platform_device *pdev) > static int tegra_pcie_pm_suspend(struct device *dev) > { > struct tegra_pcie *pcie = dev_get_drvdata(dev); > + const struct tegra_pcie_soc *soc = pcie->soc; > struct tegra_pcie_port *port; > int err; > > @@ -2672,7 +2662,7 @@ static int tegra_pcie_pm_suspend(struct device *dev) > } > > reset_control_assert(pcie->pex_rst); > - clk_disable_unprepare(pcie->pex_clk); > + clk_bulk_disable_unprepare(soc->num_clks, pcie->clks); > > if (IS_ENABLED(CONFIG_PCI_MSI)) > tegra_pcie_disable_msi(pcie); > @@ -2686,6 +2676,7 @@ static int tegra_pcie_pm_suspend(struct device *dev) > static int tegra_pcie_pm_resume(struct device *dev) > { > struct tegra_pcie *pcie = dev_get_drvdata(dev); > + const struct tegra_pcie_soc *soc = pcie->soc; > int err; > > err = tegra_pcie_power_on(pcie); > @@ -2706,9 +2697,9 @@ static int tegra_pcie_pm_resume(struct device *dev) > if (IS_ENABLED(CONFIG_PCI_MSI)) > tegra_pcie_enable_msi(pcie); > > - err = clk_prepare_enable(pcie->pex_clk); > + err = clk_bulk_prepare_enable(soc->num_clks, pcie->clks); > if (err) { > - dev_err(dev, "failed to enable PEX clock: %d\n", err); > + dev_err(dev, "failed to enable clock: %d\n", err); > goto pex_dpd_enable; > } > > @@ -2729,7 +2720,6 @@ static int tegra_pcie_pm_resume(struct device *dev) > > disable_pex_clk: > reset_control_assert(pcie->pex_rst); > - clk_disable_unprepare(pcie->pex_clk); > pex_dpd_enable: > pinctrl_pm_select_idle_state(dev); > poweroff: > -- > 2.50.1 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions 2025-09-26 18:12 ` Frank Li @ 2025-09-27 5:50 ` Anand Moon 2025-09-29 14:01 ` Manivannan Sadhasivam 0 siblings, 1 reply; 22+ messages in thread From: Anand Moon @ 2025-09-27 5:50 UTC (permalink / raw) To: Frank Li Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Hi Frank, On Fri, 26 Sept 2025 at 23:42, Frank Li <Frank.li@nxp.com> wrote: > > On Fri, Sep 26, 2025 at 12:57:43PM +0530, Anand Moon wrote: > > Currently, the driver acquires clocks and prepare/enable/disable/unprepare > > the clocks individually thereby making the driver complex to read. > > > > The driver can be simplified by using the clk_bulk*() APIs. > > > > Use: > > - devm_clk_bulk_get() API to acquire all the clocks > > - clk_bulk_prepare_enable() to prepare/enable clocks > > - clk_bulk_disable_unprepare() APIs to disable/unprepare them in bulk > > > > Following change also removes the legacy has_cml_clk flag and its associated > > conditional logic. Instead, the driver now relies on the clock definitions from > > the device tree to determine the correct clock sequencing. > > This reduces hardcoded dependencies and improves the driver's maintainability. > > > > Cc: Thierry Reding <thierry.reding@gmail.com> > > Cc: Jon Hunter <jonathanh@nvidia.com> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > v1: Switch from devm_clk_bulk_get_all() -> devm_clk_bulk_get() with > > fix clks array. > > why not use devm_clk_bulk_get_all()? > My RFC used this devm_clk_bulk_get_all() which could work for all the SoC, However, Jon recommended switching to named clocks, following the approach used in . but Jon suggested to use clock names as per dwmac-tegra.c driver. [0] https://lore.kernel.org/linux-tegra/8fac00fe-2ad4-4202-a6f2-c5043f7343f9@nvidia.com/ > Frank Thanks -Anand ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions 2025-09-27 5:50 ` Anand Moon @ 2025-09-29 14:01 ` Manivannan Sadhasivam 2025-09-29 16:12 ` Anand Moon 0 siblings, 1 reply; 22+ messages in thread From: Manivannan Sadhasivam @ 2025-09-29 14:01 UTC (permalink / raw) To: Anand Moon Cc: Frank Li, Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list On Sat, Sep 27, 2025 at 11:20:10AM +0530, Anand Moon wrote: > Hi Frank, > > On Fri, 26 Sept 2025 at 23:42, Frank Li <Frank.li@nxp.com> wrote: > > > > On Fri, Sep 26, 2025 at 12:57:43PM +0530, Anand Moon wrote: > > > Currently, the driver acquires clocks and prepare/enable/disable/unprepare > > > the clocks individually thereby making the driver complex to read. > > > > > > The driver can be simplified by using the clk_bulk*() APIs. > > > > > > Use: > > > - devm_clk_bulk_get() API to acquire all the clocks > > > - clk_bulk_prepare_enable() to prepare/enable clocks > > > - clk_bulk_disable_unprepare() APIs to disable/unprepare them in bulk > > > > > > Following change also removes the legacy has_cml_clk flag and its associated > > > conditional logic. Instead, the driver now relies on the clock definitions from > > > the device tree to determine the correct clock sequencing. > > > This reduces hardcoded dependencies and improves the driver's maintainability. > > > > > > Cc: Thierry Reding <thierry.reding@gmail.com> > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > --- > > > v1: Switch from devm_clk_bulk_get_all() -> devm_clk_bulk_get() with > > > fix clks array. > > > > why not use devm_clk_bulk_get_all()? > > > My RFC used this devm_clk_bulk_get_all() which could work for all the SoC, > However, Jon recommended switching to named clocks, following the > approach used in . > but Jon suggested to use clock names as per dwmac-tegra.c driver. > The concern was with validating the DTS files with binding. Since it was in .txt format, validation was not possible. But you are converting it to .yaml, so you can safely use devm_clk_bulk_get_all(). - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions 2025-09-29 14:01 ` Manivannan Sadhasivam @ 2025-09-29 16:12 ` Anand Moon 0 siblings, 0 replies; 22+ messages in thread From: Anand Moon @ 2025-09-29 16:12 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Frank Li, Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Hi Manivannan, Jon, On Mon, 29 Sept 2025 at 19:31, Manivannan Sadhasivam <mani@kernel.org> wrote: > > On Sat, Sep 27, 2025 at 11:20:10AM +0530, Anand Moon wrote: > > Hi Frank, > > > > On Fri, 26 Sept 2025 at 23:42, Frank Li <Frank.li@nxp.com> wrote: > > > > > > On Fri, Sep 26, 2025 at 12:57:43PM +0530, Anand Moon wrote: > > > > Currently, the driver acquires clocks and prepare/enable/disable/unprepare > > > > the clocks individually thereby making the driver complex to read. > > > > > > > > The driver can be simplified by using the clk_bulk*() APIs. > > > > > > > > Use: > > > > - devm_clk_bulk_get() API to acquire all the clocks > > > > - clk_bulk_prepare_enable() to prepare/enable clocks > > > > - clk_bulk_disable_unprepare() APIs to disable/unprepare them in bulk > > > > > > > > Following change also removes the legacy has_cml_clk flag and its associated > > > > conditional logic. Instead, the driver now relies on the clock definitions from > > > > the device tree to determine the correct clock sequencing. > > > > This reduces hardcoded dependencies and improves the driver's maintainability. > > > > > > > > Cc: Thierry Reding <thierry.reding@gmail.com> > > > > Cc: Jon Hunter <jonathanh@nvidia.com> > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > v1: Switch from devm_clk_bulk_get_all() -> devm_clk_bulk_get() with > > > > fix clks array. > > > > > > why not use devm_clk_bulk_get_all()? > > > > > My RFC used this devm_clk_bulk_get_all() which could work for all the SoC, > > However, Jon recommended switching to named clocks, following the > > approach used in . > > but Jon suggested to use clock names as per dwmac-tegra.c driver. > > > > The concern was with validating the DTS files with binding. Since it was in .txt > format, validation was not possible. But you are converting it to .yaml, so you > can safely use devm_clk_bulk_get_all(). > Yes I would also like to use the previous approach. > - Mani Thanks -Anand > > -- > மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling 2025-09-26 7:27 [PATCH v1 0/5] PCI: tegra: A couple of cleanups Anand Moon 2025-09-26 7:27 ` [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema Anand Moon 2025-09-26 7:27 ` [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions Anand Moon @ 2025-09-26 7:27 ` Anand Moon 2025-10-19 7:50 ` Manivannan Sadhasivam 2025-09-26 7:27 ` [PATCH v1 4/5] PCI: tegra: Use BIT() and GENMASK() macros for register definitions Anand Moon 2025-09-26 7:27 ` [PATCH v1 5/5] PCI: tegra: Document map_lock and mask_lock usage Anand Moon 4 siblings, 1 reply; 22+ messages in thread From: Anand Moon @ 2025-09-26 7:27 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Cc: Anand Moon, Mikko Perttunen Replace the manual `do-while` polling loops with the readl_poll_timeout() helper when checking the link DL_UP and DL_LINK_ACTIVE status bits during link bring-up. This simplifies the code by removing the open-coded timeout logic in favor of the standard, more robust iopoll framework. The change improves readability and reduces code duplication. Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Mikko Perttunen <mperttunen@nvidia.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- v1: dropped the include <linux/iopoll.h> header file. --- drivers/pci/controller/pci-tegra.c | 37 +++++++++++------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index 07a61d902eae..b0056818a203 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port) value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT; writel(value, port->base + RP_PRIV_MISC); - do { - unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT; + while (retries--) { + int err; - do { - value = readl(port->base + RP_VEND_XP); - - if (value & RP_VEND_XP_DL_UP) - break; - - usleep_range(1000, 2000); - } while (--timeout); - - if (!timeout) { + err = readl_poll_timeout(port->base + RP_VEND_XP, value, + value & RP_VEND_XP_DL_UP, + 1000, + TEGRA_PCIE_LINKUP_TIMEOUT * 1000); + if (err) { dev_dbg(dev, "link %u down, retrying\n", port->index); goto retry; } - timeout = TEGRA_PCIE_LINKUP_TIMEOUT; - - do { - value = readl(port->base + RP_LINK_CONTROL_STATUS); - - if (value & RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE) - return true; - - usleep_range(1000, 2000); - } while (--timeout); + err = readl_poll_timeout(port->base + RP_LINK_CONTROL_STATUS, + value, + value & RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE, + 1000, TEGRA_PCIE_LINKUP_TIMEOUT * 1000); + if (!err) + return true; retry: tegra_pcie_port_reset(port); - } while (--retries); + } return false; } -- 2.50.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling 2025-09-26 7:27 ` [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling Anand Moon @ 2025-10-19 7:50 ` Manivannan Sadhasivam 2025-10-20 12:17 ` Anand Moon 0 siblings, 1 reply; 22+ messages in thread From: Manivannan Sadhasivam @ 2025-10-19 7:50 UTC (permalink / raw) To: Anand Moon Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list, Mikko Perttunen On Fri, Sep 26, 2025 at 12:57:44PM +0530, Anand Moon wrote: > Replace the manual `do-while` polling loops with the readl_poll_timeout() > helper when checking the link DL_UP and DL_LINK_ACTIVE status bits > during link bring-up. This simplifies the code by removing the open-coded > timeout logic in favor of the standard, more robust iopoll framework. > The change improves readability and reduces code duplication. > > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Mikko Perttunen <mperttunen@nvidia.com> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > v1: dropped the include <linux/iopoll.h> header file. > --- > drivers/pci/controller/pci-tegra.c | 37 +++++++++++------------------- > 1 file changed, 14 insertions(+), 23 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index 07a61d902eae..b0056818a203 100644 > --- a/drivers/pci/controller/pci-tegra.c > +++ b/drivers/pci/controller/pci-tegra.c > @@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port) > value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT; > writel(value, port->base + RP_PRIV_MISC); > > - do { > - unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT; > + while (retries--) { > + int err; > > - do { > - value = readl(port->base + RP_VEND_XP); > - > - if (value & RP_VEND_XP_DL_UP) > - break; > - > - usleep_range(1000, 2000); > - } while (--timeout); > - > - if (!timeout) { > + err = readl_poll_timeout(port->base + RP_VEND_XP, value, > + value & RP_VEND_XP_DL_UP, > + 1000, The delay between the iterations had range of (1000, 2000), now it will become (250, 1000). How can you ensure that this delay is sufficient? - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling 2025-10-19 7:50 ` Manivannan Sadhasivam @ 2025-10-20 12:17 ` Anand Moon 2025-10-21 1:43 ` Manivannan Sadhasivam 0 siblings, 1 reply; 22+ messages in thread From: Anand Moon @ 2025-10-20 12:17 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list, Mikko Perttunen Hi Manivannan, Thanks for your review comment. On Sun, 19 Oct 2025 at 13:20, Manivannan Sadhasivam <mani@kernel.org> wrote: > > On Fri, Sep 26, 2025 at 12:57:44PM +0530, Anand Moon wrote: > > Replace the manual `do-while` polling loops with the readl_poll_timeout() > > helper when checking the link DL_UP and DL_LINK_ACTIVE status bits > > during link bring-up. This simplifies the code by removing the open-coded > > timeout logic in favor of the standard, more robust iopoll framework. > > The change improves readability and reduces code duplication. > > > > Cc: Thierry Reding <thierry.reding@gmail.com> > > Cc: Mikko Perttunen <mperttunen@nvidia.com> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > v1: dropped the include <linux/iopoll.h> header file. > > --- > > drivers/pci/controller/pci-tegra.c | 37 +++++++++++------------------- > > 1 file changed, 14 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > > index 07a61d902eae..b0056818a203 100644 > > --- a/drivers/pci/controller/pci-tegra.c > > +++ b/drivers/pci/controller/pci-tegra.c > > @@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port) > > value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT; > > writel(value, port->base + RP_PRIV_MISC); > > > > - do { > > - unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT; > > + while (retries--) { > > + int err; > > > > - do { > > - value = readl(port->base + RP_VEND_XP); > > - > > - if (value & RP_VEND_XP_DL_UP) > > - break; > > - > > - usleep_range(1000, 2000); > > - } while (--timeout); > > - > > - if (!timeout) { > > + err = readl_poll_timeout(port->base + RP_VEND_XP, value, > > + value & RP_VEND_XP_DL_UP, > > + 1000, > > The delay between the iterations had range of (1000, 2000), now it will become > (250, 1000). How can you ensure that this delay is sufficient? > I asked if the timeout should be increased for the loops, but Mikko Perttunen said that 200ms delay is fine. [1] https://lore.kernel.org/linux-tegra/CANAwSgT615R32WTBzi2-8FYntmaxbmVRLmA3yi+=4ryH43aaWQ@mail.gmail.com/#t > - Mani > Thanks -Anand > -- > மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling 2025-10-20 12:17 ` Anand Moon @ 2025-10-21 1:43 ` Manivannan Sadhasivam 2025-10-24 6:17 ` Anand Moon 0 siblings, 1 reply; 22+ messages in thread From: Manivannan Sadhasivam @ 2025-10-21 1:43 UTC (permalink / raw) To: Anand Moon Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list, Mikko Perttunen On Mon, Oct 20, 2025 at 05:47:15PM +0530, Anand Moon wrote: > Hi Manivannan, > > Thanks for your review comment. > > On Sun, 19 Oct 2025 at 13:20, Manivannan Sadhasivam <mani@kernel.org> wrote: > > > > On Fri, Sep 26, 2025 at 12:57:44PM +0530, Anand Moon wrote: > > > Replace the manual `do-while` polling loops with the readl_poll_timeout() > > > helper when checking the link DL_UP and DL_LINK_ACTIVE status bits > > > during link bring-up. This simplifies the code by removing the open-coded > > > timeout logic in favor of the standard, more robust iopoll framework. > > > The change improves readability and reduces code duplication. > > > > > > Cc: Thierry Reding <thierry.reding@gmail.com> > > > Cc: Mikko Perttunen <mperttunen@nvidia.com> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > --- > > > v1: dropped the include <linux/iopoll.h> header file. > > > --- > > > drivers/pci/controller/pci-tegra.c | 37 +++++++++++------------------- > > > 1 file changed, 14 insertions(+), 23 deletions(-) > > > > > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > > > index 07a61d902eae..b0056818a203 100644 > > > --- a/drivers/pci/controller/pci-tegra.c > > > +++ b/drivers/pci/controller/pci-tegra.c > > > @@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port) > > > value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT; > > > writel(value, port->base + RP_PRIV_MISC); > > > > > > - do { > > > - unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT; > > > + while (retries--) { > > > + int err; > > > > > > - do { > > > - value = readl(port->base + RP_VEND_XP); > > > - > > > - if (value & RP_VEND_XP_DL_UP) > > > - break; > > > - > > > - usleep_range(1000, 2000); > > > - } while (--timeout); > > > - > > > - if (!timeout) { > > > + err = readl_poll_timeout(port->base + RP_VEND_XP, value, > > > + value & RP_VEND_XP_DL_UP, > > > + 1000, > > > > The delay between the iterations had range of (1000, 2000), now it will become > > (250, 1000). How can you ensure that this delay is sufficient? > > > I asked if the timeout should be increased for the loops, but Mikko > Perttunen said that 200ms delay is fine. > readl_poll_timeout() internally uses usleep_range(), which transforms the 1000us delay into, usleep_range(251, 1000). So the delay *could* theoretically be 251us * 200 = ~50ms. So I doubt it will be sifficient, as from the old code, it looks like the hardware could take around 200ms to complete link up. - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling 2025-10-21 1:43 ` Manivannan Sadhasivam @ 2025-10-24 6:17 ` Anand Moon 0 siblings, 0 replies; 22+ messages in thread From: Anand Moon @ 2025-10-24 6:17 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list, Mikko Perttunen Hi Manivannan, On Tue, 21 Oct 2025 at 07:13, Manivannan Sadhasivam <mani@kernel.org> wrote: > > On Mon, Oct 20, 2025 at 05:47:15PM +0530, Anand Moon wrote: > > Hi Manivannan, > > > > Thanks for your review comment. > > > > On Sun, 19 Oct 2025 at 13:20, Manivannan Sadhasivam <mani@kernel.org> wrote: > > > > > > On Fri, Sep 26, 2025 at 12:57:44PM +0530, Anand Moon wrote: > > > > Replace the manual `do-while` polling loops with the readl_poll_timeout() > > > > helper when checking the link DL_UP and DL_LINK_ACTIVE status bits > > > > during link bring-up. This simplifies the code by removing the open-coded > > > > timeout logic in favor of the standard, more robust iopoll framework. > > > > The change improves readability and reduces code duplication. > > > > > > > > Cc: Thierry Reding <thierry.reding@gmail.com> > > > > Cc: Mikko Perttunen <mperttunen@nvidia.com> > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > v1: dropped the include <linux/iopoll.h> header file. > > > > --- > > > > drivers/pci/controller/pci-tegra.c | 37 +++++++++++------------------- > > > > 1 file changed, 14 insertions(+), 23 deletions(-) > > > > > > > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > > > > index 07a61d902eae..b0056818a203 100644 > > > > --- a/drivers/pci/controller/pci-tegra.c > > > > +++ b/drivers/pci/controller/pci-tegra.c > > > > @@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port) > > > > value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT; > > > > writel(value, port->base + RP_PRIV_MISC); > > > > > > > > - do { > > > > - unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT; > > > > + while (retries--) { > > > > + int err; > > > > > > > > - do { > > > > - value = readl(port->base + RP_VEND_XP); > > > > - > > > > - if (value & RP_VEND_XP_DL_UP) > > > > - break; > > > > - > > > > - usleep_range(1000, 2000); > > > > - } while (--timeout); > > > > - > > > > - if (!timeout) { > > > > + err = readl_poll_timeout(port->base + RP_VEND_XP, value, > > > > + value & RP_VEND_XP_DL_UP, > > > > + 1000, > > > > > > The delay between the iterations had range of (1000, 2000), now it will become > > > (250, 1000). How can you ensure that this delay is sufficient? > > > > > I asked if the timeout should be increased for the loops, but Mikko > > Perttunen said that 200ms delay is fine. > > > > readl_poll_timeout() internally uses usleep_range(), which transforms the 1000us > delay into, usleep_range(251, 1000). So the delay *could* theoretically be 251us > * 200 = ~50ms. > > So I doubt it will be sifficient, as from the old code, it looks like the > hardware could take around 200ms to complete link up. > Instead of implementing a busy-waiting while loop with udelay, a more efficient and responsive approach is to use the readl_poll_timeout() function. This function periodically polls a memory-mapped address, waiting for a condition to be met or for a specified timeout to occur. If there are any issues with HW, we could extend the timeout to compensate. > - Mani Thanks -Anand > > -- > மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v1 4/5] PCI: tegra: Use BIT() and GENMASK() macros for register definitions 2025-09-26 7:27 [PATCH v1 0/5] PCI: tegra: A couple of cleanups Anand Moon ` (2 preceding siblings ...) 2025-09-26 7:27 ` [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling Anand Moon @ 2025-09-26 7:27 ` Anand Moon 2025-09-26 7:27 ` [PATCH v1 5/5] PCI: tegra: Document map_lock and mask_lock usage Anand Moon 4 siblings, 0 replies; 22+ messages in thread From: Anand Moon @ 2025-09-26 7:27 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Cc: Anand Moon Replace opencode masking and shifting bit operations with standard BIT() and GENMASK() macros. This improves code readability and maintainability by removing magic numbers and resolving checkpatch.pl warnings. Cc: Jon Hunter <jonathanh@nvidia.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- v1: New patch in this series --- drivers/pci/controller/pci-tegra.c | 129 +++++++++++++++-------------- 1 file changed, 65 insertions(+), 64 deletions(-) diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index b0056818a203..02cee0763396 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -13,6 +13,7 @@ * Author: Thierry Reding <treding@nvidia.com> */ +#include <linux/bitops.h> #include <linux/clk.h> #include <linux/debugfs.h> #include <linux/delay.h> @@ -84,17 +85,17 @@ #define AFI_MSI_EN_VEC(x) (0x8c + ((x) * 4)) #define AFI_CONFIGURATION 0xac -#define AFI_CONFIGURATION_EN_FPCI (1 << 0) -#define AFI_CONFIGURATION_CLKEN_OVERRIDE (1 << 31) +#define AFI_CONFIGURATION_EN_FPCI BIT(0) +#define AFI_CONFIGURATION_CLKEN_OVERRIDE BIT(31) #define AFI_FPCI_ERROR_MASKS 0xb0 #define AFI_INTR_MASK 0xb4 -#define AFI_INTR_MASK_INT_MASK (1 << 0) -#define AFI_INTR_MASK_MSI_MASK (1 << 8) +#define AFI_INTR_MASK_INT_MASK BIT(0) +#define AFI_INTR_MASK_MSI_MASK BIT(8) #define AFI_INTR_CODE 0xb8 -#define AFI_INTR_CODE_MASK 0xf +#define AFI_INTR_CODE_MASK GENMASK(3, 0) #define AFI_INTR_INI_SLAVE_ERROR 1 #define AFI_INTR_INI_DECODE_ERROR 2 #define AFI_INTR_TARGET_ABORT 3 @@ -113,32 +114,32 @@ #define AFI_INTR_SIGNATURE 0xbc #define AFI_UPPER_FPCI_ADDRESS 0xc0 #define AFI_SM_INTR_ENABLE 0xc4 -#define AFI_SM_INTR_INTA_ASSERT (1 << 0) -#define AFI_SM_INTR_INTB_ASSERT (1 << 1) -#define AFI_SM_INTR_INTC_ASSERT (1 << 2) -#define AFI_SM_INTR_INTD_ASSERT (1 << 3) -#define AFI_SM_INTR_INTA_DEASSERT (1 << 4) -#define AFI_SM_INTR_INTB_DEASSERT (1 << 5) -#define AFI_SM_INTR_INTC_DEASSERT (1 << 6) -#define AFI_SM_INTR_INTD_DEASSERT (1 << 7) +#define AFI_SM_INTR_INTA_ASSERT BIT(0) +#define AFI_SM_INTR_INTB_ASSERT BIT(1) +#define AFI_SM_INTR_INTC_ASSERT BIT(2) +#define AFI_SM_INTR_INTD_ASSERT BIT(3) +#define AFI_SM_INTR_INTA_DEASSERT BIT(4) +#define AFI_SM_INTR_INTB_DEASSERT BIT(5) +#define AFI_SM_INTR_INTC_DEASSERT BIT(6) +#define AFI_SM_INTR_INTD_DEASSERT BIT(7) #define AFI_AFI_INTR_ENABLE 0xc8 -#define AFI_INTR_EN_INI_SLVERR (1 << 0) -#define AFI_INTR_EN_INI_DECERR (1 << 1) -#define AFI_INTR_EN_TGT_SLVERR (1 << 2) -#define AFI_INTR_EN_TGT_DECERR (1 << 3) -#define AFI_INTR_EN_TGT_WRERR (1 << 4) -#define AFI_INTR_EN_DFPCI_DECERR (1 << 5) -#define AFI_INTR_EN_AXI_DECERR (1 << 6) -#define AFI_INTR_EN_FPCI_TIMEOUT (1 << 7) -#define AFI_INTR_EN_PRSNT_SENSE (1 << 8) +#define AFI_INTR_EN_INI_SLVERR BIT(0) +#define AFI_INTR_EN_INI_DECERR BIT(1) +#define AFI_INTR_EN_TGT_SLVERR BIT(2) +#define AFI_INTR_EN_TGT_DECERR BIT(3) +#define AFI_INTR_EN_TGT_WRERR BIT(4) +#define AFI_INTR_EN_DFPCI_DECERR BIT(5) +#define AFI_INTR_EN_AXI_DECERR BIT(6) +#define AFI_INTR_EN_FPCI_TIMEOUT BIT(7) +#define AFI_INTR_EN_PRSNT_SENSE BIT(8) #define AFI_PCIE_PME 0xf0 #define AFI_PCIE_CONFIG 0x0f8 -#define AFI_PCIE_CONFIG_PCIE_DISABLE(x) (1 << ((x) + 1)) -#define AFI_PCIE_CONFIG_PCIE_DISABLE_ALL 0xe -#define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_MASK (0xf << 20) +#define AFI_PCIE_CONFIG_PCIE_DISABLE(x) BIT((x) + 1) +#define AFI_PCIE_CONFIG_PCIE_DISABLE_ALL GENMASK(3, 1) +#define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_MASK GENMASK(23, 20) #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_SINGLE (0x0 << 20) #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_420 (0x0 << 20) #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_X2_X1 (0x0 << 20) @@ -149,79 +150,79 @@ #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_211 (0x1 << 20) #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_411 (0x2 << 20) #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_111 (0x2 << 20) -#define AFI_PCIE_CONFIG_PCIE_CLKREQ_GPIO(x) (1 << ((x) + 29)) -#define AFI_PCIE_CONFIG_PCIE_CLKREQ_GPIO_ALL (0x7 << 29) +#define AFI_PCIE_CONFIG_PCIE_CLKREQ_GPIO(x) BIT((x) + 29) +#define AFI_PCIE_CONFIG_PCIE_CLKREQ_GPIO_ALL GENMASK(31, 29) #define AFI_FUSE 0x104 -#define AFI_FUSE_PCIE_T0_GEN2_DIS (1 << 2) +#define AFI_FUSE_PCIE_T0_GEN2_DIS BIT(2) #define AFI_PEX0_CTRL 0x110 #define AFI_PEX1_CTRL 0x118 -#define AFI_PEX_CTRL_RST (1 << 0) -#define AFI_PEX_CTRL_CLKREQ_EN (1 << 1) -#define AFI_PEX_CTRL_REFCLK_EN (1 << 3) -#define AFI_PEX_CTRL_OVERRIDE_EN (1 << 4) +#define AFI_PEX_CTRL_RST BIT(0) +#define AFI_PEX_CTRL_CLKREQ_EN BIT(1) +#define AFI_PEX_CTRL_REFCLK_EN BIT(3) +#define AFI_PEX_CTRL_OVERRIDE_EN BIT(4) #define AFI_PLLE_CONTROL 0x160 -#define AFI_PLLE_CONTROL_BYPASS_PADS2PLLE_CONTROL (1 << 9) -#define AFI_PLLE_CONTROL_PADS2PLLE_CONTROL_EN (1 << 1) +#define AFI_PLLE_CONTROL_BYPASS_PADS2PLLE_CONTROL BIT(9) +#define AFI_PLLE_CONTROL_PADS2PLLE_CONTROL_EN BIT(1) #define AFI_PEXBIAS_CTRL_0 0x168 #define RP_ECTL_2_R1 0x00000e84 -#define RP_ECTL_2_R1_RX_CTLE_1C_MASK 0xffff +#define RP_ECTL_2_R1_RX_CTLE_1C_MASK GENMASK(15, 0) #define RP_ECTL_4_R1 0x00000e8c -#define RP_ECTL_4_R1_RX_CDR_CTRL_1C_MASK (0xffff << 16) +#define RP_ECTL_4_R1_RX_CDR_CTRL_1C_MASK GENMASK(31, 16) #define RP_ECTL_4_R1_RX_CDR_CTRL_1C_SHIFT 16 #define RP_ECTL_5_R1 0x00000e90 -#define RP_ECTL_5_R1_RX_EQ_CTRL_L_1C_MASK 0xffffffff +#define RP_ECTL_5_R1_RX_EQ_CTRL_L_1C_MASK GENMASK(31, 0) #define RP_ECTL_6_R1 0x00000e94 -#define RP_ECTL_6_R1_RX_EQ_CTRL_H_1C_MASK 0xffffffff +#define RP_ECTL_6_R1_RX_EQ_CTRL_H_1C_MASK GENMASK(31, 0) #define RP_ECTL_2_R2 0x00000ea4 #define RP_ECTL_2_R2_RX_CTLE_1C_MASK 0xffff #define RP_ECTL_4_R2 0x00000eac -#define RP_ECTL_4_R2_RX_CDR_CTRL_1C_MASK (0xffff << 16) +#define RP_ECTL_4_R2_RX_CDR_CTRL_1C_MASK GENMASK(31, 16) #define RP_ECTL_4_R2_RX_CDR_CTRL_1C_SHIFT 16 #define RP_ECTL_5_R2 0x00000eb0 -#define RP_ECTL_5_R2_RX_EQ_CTRL_L_1C_MASK 0xffffffff +#define RP_ECTL_5_R2_RX_EQ_CTRL_L_1C_MASK GENMASK(31, 0) #define RP_ECTL_6_R2 0x00000eb4 -#define RP_ECTL_6_R2_RX_EQ_CTRL_H_1C_MASK 0xffffffff +#define RP_ECTL_6_R2_RX_EQ_CTRL_H_1C_MASK GENMASK(31, 0) #define RP_VEND_XP 0x00000f00 -#define RP_VEND_XP_DL_UP (1 << 30) -#define RP_VEND_XP_OPPORTUNISTIC_ACK (1 << 27) -#define RP_VEND_XP_OPPORTUNISTIC_UPDATEFC (1 << 28) -#define RP_VEND_XP_UPDATE_FC_THRESHOLD_MASK (0xff << 18) +#define RP_VEND_XP_DL_UP BIT(30) +#define RP_VEND_XP_OPPORTUNISTIC_ACK BIT(27) +#define RP_VEND_XP_OPPORTUNISTIC_UPDATEFC BIT(28) +#define RP_VEND_XP_UPDATE_FC_THRESHOLD_MASK GENMASK(25, 18) #define RP_VEND_CTL0 0x00000f44 -#define RP_VEND_CTL0_DSK_RST_PULSE_WIDTH_MASK (0xf << 12) +#define RP_VEND_CTL0_DSK_RST_PULSE_WIDTH_MASK GENMASK(15, 12) #define RP_VEND_CTL0_DSK_RST_PULSE_WIDTH (0x9 << 12) #define RP_VEND_CTL1 0x00000f48 -#define RP_VEND_CTL1_ERPT (1 << 13) +#define RP_VEND_CTL1_ERPT BIT(13) #define RP_VEND_XP_BIST 0x00000f4c -#define RP_VEND_XP_BIST_GOTO_L1_L2_AFTER_DLLP_DONE (1 << 28) +#define RP_VEND_XP_BIST_GOTO_L1_L2_AFTER_DLLP_DONE BIT(28) #define RP_VEND_CTL2 0x00000fa8 -#define RP_VEND_CTL2_PCA_ENABLE (1 << 7) +#define RP_VEND_CTL2_PCA_ENABLE BIT(7) #define RP_PRIV_MISC 0x00000fe0 -#define RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT (0xe << 0) -#define RP_PRIV_MISC_PRSNT_MAP_EP_ABSNT (0xf << 0) -#define RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD_MASK (0x7f << 16) +#define RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT GENMASK(3, 1) +#define RP_PRIV_MISC_PRSNT_MAP_EP_ABSNT GENMASK(3, 0) +#define RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD_MASK GENMASK(22, 16) #define RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD (0xf << 16) -#define RP_PRIV_MISC_CTLR_CLK_CLAMP_ENABLE (1 << 23) -#define RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD_MASK (0x7f << 24) +#define RP_PRIV_MISC_CTLR_CLK_CLAMP_ENABLE BIT(23) +#define RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD_MASK GENMASK(30, 24) #define RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD (0xf << 24) -#define RP_PRIV_MISC_TMS_CLK_CLAMP_ENABLE (1 << 31) +#define RP_PRIV_MISC_TMS_CLK_CLAMP_ENABLE BIT(31) #define RP_LINK_CONTROL_STATUS 0x00000090 #define RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE 0x20000000 @@ -232,22 +233,22 @@ #define PADS_CTL_SEL 0x0000009c #define PADS_CTL 0x000000a0 -#define PADS_CTL_IDDQ_1L (1 << 0) -#define PADS_CTL_TX_DATA_EN_1L (1 << 6) -#define PADS_CTL_RX_DATA_EN_1L (1 << 10) +#define PADS_CTL_IDDQ_1L BIT(0) +#define PADS_CTL_TX_DATA_EN_1L BIT(6) +#define PADS_CTL_RX_DATA_EN_1L BIT(10) #define PADS_PLL_CTL_TEGRA20 0x000000b8 #define PADS_PLL_CTL_TEGRA30 0x000000b4 -#define PADS_PLL_CTL_RST_B4SM (1 << 1) -#define PADS_PLL_CTL_LOCKDET (1 << 8) -#define PADS_PLL_CTL_REFCLK_MASK (0x3 << 16) +#define PADS_PLL_CTL_RST_B4SM BIT(1) +#define PADS_PLL_CTL_LOCKDET BIT(8) +#define PADS_PLL_CTL_REFCLK_MASK GENMASK(17, 16) #define PADS_PLL_CTL_REFCLK_INTERNAL_CML (0 << 16) -#define PADS_PLL_CTL_REFCLK_INTERNAL_CMOS (1 << 16) +#define PADS_PLL_CTL_REFCLK_INTERNAL_CMOS BIT(16) #define PADS_PLL_CTL_REFCLK_EXTERNAL (2 << 16) #define PADS_PLL_CTL_TXCLKREF_MASK (0x1 << 20) #define PADS_PLL_CTL_TXCLKREF_DIV10 (0 << 20) -#define PADS_PLL_CTL_TXCLKREF_DIV5 (1 << 20) -#define PADS_PLL_CTL_TXCLKREF_BUF_EN (1 << 22) +#define PADS_PLL_CTL_TXCLKREF_DIV5 BIT(20) +#define PADS_PLL_CTL_TXCLKREF_BUF_EN BIT(22) #define PADS_REFCLK_CFG0 0x000000c8 #define PADS_REFCLK_CFG1 0x000000cc -- 2.50.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v1 5/5] PCI: tegra: Document map_lock and mask_lock usage 2025-09-26 7:27 [PATCH v1 0/5] PCI: tegra: A couple of cleanups Anand Moon ` (3 preceding siblings ...) 2025-09-26 7:27 ` [PATCH v1 4/5] PCI: tegra: Use BIT() and GENMASK() macros for register definitions Anand Moon @ 2025-09-26 7:27 ` Anand Moon 4 siblings, 0 replies; 22+ messages in thread From: Anand Moon @ 2025-09-26 7:27 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Thierry Reding, Jonathan Hunter, open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:TEGRA ARCHITECTURE SUPPORT, open list Cc: Anand Moon Add comments to clarify the purpose of map_lock and mask_lock in the tegra_msi structure. This addresses checkpatch.pl warnings about missing comments for struct mutex and spinlock_t definitions, improving code readability without functional changes. CHECK: struct mutex definition without comment + struct mutex map_lock; CHECK: spinlock_t definition without comment + spinlock_t mask_lock; Cc: Jon Hunter <jonathanh@nvidia.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- v1: new patch in this series. --- drivers/pci/controller/pci-tegra.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index 02cee0763396..24b6ae0f0035 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -270,7 +270,9 @@ struct tegra_msi { DECLARE_BITMAP(used, INT_PCI_MSI_NR); struct irq_domain *domain; + /* Protects mapping operations */ struct mutex map_lock; + /* Guards interrupt mask updates */ spinlock_t mask_lock; void *virt; dma_addr_t phys; -- 2.50.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-10-24 6:17 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-26 7:27 [PATCH v1 0/5] PCI: tegra: A couple of cleanups Anand Moon 2025-09-26 7:27 ` [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema Anand Moon 2025-09-26 13:56 ` Rob Herring 2025-09-29 7:39 ` Anand Moon 2025-09-29 13:48 ` Rob Herring 2025-09-29 15:25 ` Anand Moon 2025-09-30 14:37 ` Rob Herring 2025-09-30 16:32 ` Anand Moon 2025-10-01 15:33 ` Rob Herring 2025-10-01 18:57 ` Anand Moon 2025-09-26 7:27 ` [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using clk_bulk*() functions Anand Moon 2025-09-26 18:12 ` Frank Li 2025-09-27 5:50 ` Anand Moon 2025-09-29 14:01 ` Manivannan Sadhasivam 2025-09-29 16:12 ` Anand Moon 2025-09-26 7:27 ` [PATCH v1 3/5] PCI: tegra: Use readl_poll_timeout() for link status polling Anand Moon 2025-10-19 7:50 ` Manivannan Sadhasivam 2025-10-20 12:17 ` Anand Moon 2025-10-21 1:43 ` Manivannan Sadhasivam 2025-10-24 6:17 ` Anand Moon 2025-09-26 7:27 ` [PATCH v1 4/5] PCI: tegra: Use BIT() and GENMASK() macros for register definitions Anand Moon 2025-09-26 7:27 ` [PATCH v1 5/5] PCI: tegra: Document map_lock and mask_lock usage Anand Moon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.