| Message ID | 1554407683-31580-1-git-send-email-vidyas@nvidia.com |
|---|---|
| Headers | show |
| Series | Add Tegra194 PCIe support | expand |
On Fri, Apr 05, 2019 at 01:24:28AM +0530, Vidya Sagar wrote: > Add #defines for the Data Link Feature and Physical Layer 16.0 GT/s > features. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > Changes from [v1]: > * None > > include/uapi/linux/pci_regs.h | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > index 5c98133f2c94..3e01b55d548d 100644 > --- a/include/uapi/linux/pci_regs.h > +++ b/include/uapi/linux/pci_regs.h > @@ -705,7 +705,9 @@ > #define PCI_EXT_CAP_ID_DPC 0x1D /* Downstream Port Containment */ > #define PCI_EXT_CAP_ID_L1SS 0x1E /* L1 PM Substates */ > #define PCI_EXT_CAP_ID_PTM 0x1F /* Precision Time Measurement */ > -#define PCI_EXT_CAP_ID_MAX PCI_EXT_CAP_ID_PTM > +#define PCI_EXT_CAP_ID_DLF 0x25 /* Data Link Feature */ > +#define PCI_EXT_CAP_ID_PL 0x26 /* Physical Layer 16.0 GT/s */ > +#define PCI_EXT_CAP_ID_MAX PCI_EXT_CAP_ID_PL > > #define PCI_EXT_CAP_DSN_SIZEOF 12 > #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40 > @@ -1045,4 +1047,22 @@ > #define PCI_L1SS_CTL1_LTR_L12_TH_SCALE 0xe0000000 /* LTR_L1.2_THRESHOLD_Scale */ > #define PCI_L1SS_CTL2 0x0c /* Control 2 Register */ > > +/* Data Link Feature */ > +#define PCI_DLF_CAP 0x04 /* Capabilities Register */ > +#define PCI_DLF_LOCAL_DLF_SUP_MASK 0x007fffff /* Local Data Link Feature Supported */ > +#define PCI_DLF_EXCHANGE_ENABLE 0x80000000 /* Data Link Feature Exchange Enable */ > +#define PCI_DLF_STS 0x08 /* Status Register */ > +#define PCI_DLF_REMOTE_DLF_SUP_MASK 0x007fffff /* Remote Data Link Feature Supported */ > +#define PCI_DLF_REMOTE_DLF_SUP_VALID 0x80000000 /* Remote Data Link Feature Support Valid */ > + > +/* Physical Layer 16.0 GT/s */ > +#define PCI_PL_16GT_CAP 0x04 /* Capabilities Register */ > +#define PCI_PL_16GT_CTRL 0x08 /* Control Register */ > +#define PCI_PL_16GT_STS 0x0c /* Status Register */ > +#define PCI_PL_16GT_LDPM_STS 0x10 /* Local Data Parity Mismatch Status Register */ > +#define PCI_PL_16GT_FRDPM_STS 0x14 /* First Retimer Data Parity Mismatch Status Register */ > +#define PCI_PL_16GT_SRDPM_STS 0x18 /* Second Retimer Data Parity Mismatch Status Register */ > +#define PCI_PL_16GT_RSVD 0x1C /* Reserved */ > +#define PCI_PL_16GT_LE_CTRL 0x20 /* Lane Equalization Control Register */ This looks correct comparing to the specification. However, this leaves out some definitions, so I'm wondering if perhaps this should include all field definitions. There are also extended capabilities between the current maximum 0x1F and 0x25. Perhaps those should be added as well. I guess this could always be done as a follow-up. Perhaps it'd be better to change the subject to more accurately reflect that you're only adding a couple of PCIe 4.0 features. Other than that: Reviewed-by: Thierry Reding <treding@nvidia.com>
On Fri, Apr 05, 2019 at 01:24:29AM +0530, Vidya Sagar wrote: > Export pcie_pme_disable_msi() API to enable drivers using this API be able to > build as loadable modules > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > Changes from [v1]: > * This is a new patch in v2 series > > drivers/pci/pcie/pme.c | 6 ++++++ > drivers/pci/pcie/portdrv.h | 5 +---- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c > index 54d593d10396..18e815c1e3c9 100644 > --- a/drivers/pci/pcie/pme.c > +++ b/drivers/pci/pcie/pme.c > @@ -27,6 +27,12 @@ > */ > bool pcie_pme_msi_disabled; > > +void pcie_pme_disable_msi(void) > +{ > + pcie_pme_msi_disabled = true; > +} > +EXPORT_SYMBOL_GPL(pcie_pme_disable_msi); > + > static int __init pcie_pme_setup(char *str) > { > if (!strncmp(str, "nomsi", 5)) > diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h > index 1d50dc58ac40..2be7b7e9a784 100644 > --- a/drivers/pci/pcie/portdrv.h > +++ b/drivers/pci/pcie/portdrv.h > @@ -127,10 +127,7 @@ struct pci_dev; > #ifdef CONFIG_PCIE_PME > extern bool pcie_pme_msi_disabled; > > -static inline void pcie_pme_disable_msi(void) > -{ > - pcie_pme_msi_disabled = true; > -} > +void pcie_pme_disable_msi(void); > > static inline bool pcie_pme_no_msi(void) > { Perhaps also export pcie_pme_no_msi()? That way you could get rid of the extern declaration of the pcie_pme_msi_disabled variable and make it static to drivers/pci/pcie/pme.c? Thierry
On Fri, Apr 05, 2019 at 01:24:40AM +0530, Vidya Sagar wrote: > Enable PCIe controller nodes to enable respective PCIe slots on > P2972-0000 board. Following is the ownership of slots by different > PCIe controllers. > Controller-0 : M.2 Key-M slot > Controller-1 : On-board Marvell eSATA controller > Controller-3 : M.2 Key-E slot > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > Changes since [v1]: > * Dropped 'pcie-' from phy-names property strings > > arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 2 +- > arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts | 50 ++++++++++++++++++++++ > 2 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi > index 246c1ebbd055..13263529125b 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi > +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi > @@ -191,7 +191,7 @@ > regulator-boot-on; > }; > > - sd3 { > + vdd_1v8ao: sd3 { > regulator-name = "VDD_1V8AO"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > index b62e96945846..82eb30bceaa6 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > @@ -169,4 +169,54 @@ > }; > }; > }; > + > + pcie@14180000 { > + status = "okay"; > + > + vddio-pex-ctl-supply = <&vdd_1v8ao>; > + > + phys = <&p2u_2>, > + <&p2u_3>, > + <&p2u_4>, > + <&p2u_5>; You can use multiple entries on a single line, especially if they are this short. > + phy-names = "p2u-0", "p2u-1", "p2u-2", > + "p2u-3"; Same here. > + }; > + > + pcie@14100000 { > + status = "okay"; > + > + vddio-pex-ctl-supply = <&vdd_1v8ao>; > + > + phys = <&p2u_0>; > + phy-names = "p2u-0"; > + }; > + > + pcie@14140000 { > + status = "okay"; > + > + vddio-pex-ctl-supply = <&vdd_1v8ao>; > + > + phys = <&p2u_7>; > + phy-names = "p2u-0"; > + }; > + > + pcie@141a0000 { > + status = "disabled"; > + > + vddio-pex-ctl-supply = <&vdd_1v8ao>; > + > + phys = <&p2u_12>, > + <&p2u_13>, > + <&p2u_14>, > + <&p2u_15>, > + <&p2u_16>, > + <&p2u_17>, > + <&p2u_18>, > + <&p2u_19>; > + > + phy-names = "p2u-0", "p2u-1", "p2u-2", > + "p2u-3", "p2u-4", "p2u-5", > + "p2u-6", "p2u-7"; And here. Thierry > + }; > }; > -- > 2.7.4 >
On Fri, Apr 05, 2019 at 01:24:39AM +0530, Vidya Sagar wrote: > Add P2U (PIPE to UPHY) and PCIe controller nodes to device tree. > The Tegra194 SoC contains six PCIe controllers and twenty P2U instances > grouped into two different PHY bricks namely High-Speed IO (HSIO-12 P2Us) > and NVIDIA High Speed (NVHS-8 P2Us) respectively. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > Changes since [v1]: > * Flattened all P2U nodes by removing 'hsio-p2u' and 'nvhs-p2u' super nodes > * Changed P2U nodes compatible string from 'nvidia,tegra194-phy-p2u' to 'nvidia,tegra194-p2u' > * Changed reg-name from 'base' to 'ctl' > * Updated all PCIe nodes according to the changes made to DT documentation file > > arch/arm64/boot/dts/nvidia/tegra194.dtsi | 449 +++++++++++++++++++++++++++++++ > 1 file changed, 449 insertions(+) > > diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi > index c77ca211fa8f..5b62136d97a5 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi > +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi > @@ -884,6 +884,166 @@ > nvidia,interface = <3>; > }; > }; > + > + p2u_0: p2u@03e10000 { /* HSIO-Lane-0 */ > + compatible = "nvidia,tegra194-p2u"; > + reg = <0x03e10000 0x10000>; > + reg-names = "ctl"; > + > + #phy-cells = <0>; > + }; > + [...] > + p2u_12: p2u@03eb0000 { /* NVHS-Lane-0 */ > + compatible = "nvidia,tegra194-p2u"; > + reg = <0x03eb0000 0x10000>; > + reg-names = "ctl"; > + > + #phy-cells = <0>; > + }; [...] Do we perhaps want to include the type of P2U in the label? That would make it more obvious which ones to list in the PCIe controller nodes' phys properties. Something like: p2u_hsio_0: p2u@3e10000 { ... }; ... p2u_nvhs_0: p2u@3eb0000 { ... }; ? Also, make sure to drop the leading 0 from unit-addresses. Recent versions of DTC have checks for that in place and will warn about it in recent Linux builds. [...] > @@ -1054,4 +1214,293 @@ > (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > interrupt-parent = <&gic>; > }; > + > + pcie@14180000 { > + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; > + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX8B>; > + reg = <0x00 0x14180000 0x0 0x00020000 /* appl registers (128K) */ > + 0x00 0x38000000 0x0 0x00040000 /* configuration space (256K) */ > + 0x00 0x38040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ > + 0x00 0x38080000 0x0 0x00040000>; /* DBI reg space (256K) */ > + reg-names = "appl", "config", "atu_dma", "dbi"; > + > + status = "disabled"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + num-lanes = <8>; > + num-viewport = <8>; > + linux,pci-domain = <0>; > + > + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_0>; > + clock-names = "core"; > + > + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_0_APB>, > + <&bpmp TEGRA194_RESET_PEX0_CORE_0>; > + reset-names = "core_apb", "core"; > + > + 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 0 72 0x04>; > + > + nvidia,bpmp = <&bpmp>; > + > + supports-clkreq; > + nvidia,disable-aspm-states = <0xf>; > + nvidia,controller-id = <0>; > + nvidia,aspm-cmrt-us = <60>; > + nvidia,aspm-pwr-on-t-us = <20>; > + nvidia,aspm-l0s-entrance-latency-us = <3>; Didn't you remove some of these from the bindings? Thierry > + > + bus-range = <0x0 0xff>; > + ranges = <0x81000000 0x0 0x38100000 0x0 0x38100000 0x0 0x00100000 /* downstream I/O (1MB) */ > + 0xc2000000 0x18 0x00000000 0x18 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */ > + 0x82000000 0x0 0x40000000 0x1B 0x40000000 0x0 0xC0000000>; /* non-prefetchable memory (3GB) */ > + }; > + > + pcie@14100000 { > + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; > + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX1A>; > + reg = <0x00 0x14100000 0x0 0x00020000 /* appl registers (128K) */ > + 0x00 0x30000000 0x0 0x00040000 /* configuration space (256K) */ > + 0x00 0x30040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ > + 0x00 0x30080000 0x0 0x00040000>; /* DBI reg space (256K) */ > + reg-names = "appl", "config", "atu_dma", "dbi"; > + > + status = "disabled"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + num-lanes = <1>; > + num-viewport = <8>; > + linux,pci-domain = <1>; > + > + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_1>; > + clock-names = "core"; > + > + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_1_APB>, > + <&bpmp TEGRA194_RESET_PEX0_CORE_1>; > + reset-names = "core_apb", "core"; > + > + interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ > + <GIC_SPI 46 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 0 45 0x04>; > + > + nvidia,bpmp = <&bpmp>; > + > + supports-clkreq; > + nvidia,disable-aspm-states = <0xf>; > + nvidia,controller-id = <1>; > + nvidia,aspm-cmrt-us = <60>; > + nvidia,aspm-pwr-on-t-us = <20>; > + nvidia,aspm-l0s-entrance-latency-us = <3>; > + > + bus-range = <0x0 0xff>; > + ranges = <0x81000000 0x0 0x30100000 0x0 0x30100000 0x0 0x00100000 /* downstream I/O (1MB) */ > + 0xc2000000 0x12 0x00000000 0x12 0x00000000 0x0 0x30000000 /* prefetchable memory (768MB) */ > + 0x82000000 0x0 0x40000000 0x12 0x30000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */ > + }; > + > + pcie@14120000 { > + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; > + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX1A>; > + reg = <0x00 0x14120000 0x0 0x00020000 /* appl registers (128K) */ > + 0x00 0x32000000 0x0 0x00040000 /* configuration space (256K) */ > + 0x00 0x32040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ > + 0x00 0x32080000 0x0 0x00040000>; /* DBI reg space (256K) */ > + reg-names = "appl", "config", "atu_dma", "dbi"; > + > + status = "disabled"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + num-lanes = <1>; > + num-viewport = <8>; > + linux,pci-domain = <2>; > + > + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_2>; > + clock-names = "core"; > + > + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_2_APB>, > + <&bpmp TEGRA194_RESET_PEX0_CORE_2>; > + reset-names = "core_apb", "core"; > + > + interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ > + <GIC_SPI 48 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 0 47 0x04>; > + > + nvidia,bpmp = <&bpmp>; > + > + supports-clkreq; > + nvidia,disable-aspm-states = <0xf>; > + nvidia,controller-id = <2>; > + nvidia,aspm-cmrt-us = <60>; > + nvidia,aspm-pwr-on-t-us = <20>; > + nvidia,aspm-l0s-entrance-latency-us = <3>; > + > + bus-range = <0x0 0xff>; > + ranges = <0x81000000 0x0 0x32100000 0x0 0x32100000 0x0 0x00100000 /* downstream I/O (1MB) */ > + 0xc2000000 0x12 0x40000000 0x12 0x40000000 0x0 0x30000000 /* prefetchable memory (768MB) */ > + 0x82000000 0x0 0x40000000 0x12 0x70000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */ > + }; > + > + pcie@14140000 { > + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; > + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX1A>; > + reg = <0x00 0x14140000 0x0 0x00020000 /* appl registers (128K) */ > + 0x00 0x34000000 0x0 0x00040000 /* configuration space (256K) */ > + 0x00 0x34040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ > + 0x00 0x34080000 0x0 0x00040000>; /* DBI reg space (256K) */ > + reg-names = "appl", "config", "atu_dma", "dbi"; > + > + status = "disabled"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + num-lanes = <1>; > + num-viewport = <8>; > + linux,pci-domain = <3>; > + > + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_3>; > + clock-names = "core"; > + > + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_3_APB>, > + <&bpmp TEGRA194_RESET_PEX0_CORE_3>; > + reset-names = "core_apb", "core"; > + > + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ > + <GIC_SPI 50 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 0 49 0x04>; > + > + nvidia,bpmp = <&bpmp>; > + > + supports-clkreq; > + nvidia,disable-aspm-states = <0xf>; > + nvidia,controller-id = <3>; > + nvidia,aspm-cmrt-us = <60>; > + nvidia,aspm-pwr-on-t-us = <20>; > + nvidia,aspm-l0s-entrance-latency-us = <3>; > + > + bus-range = <0x0 0xff>; > + ranges = <0x81000000 0x0 0x34100000 0x0 0x34100000 0x0 0x00100000 /* downstream I/O (1MB) */ > + 0xc2000000 0x12 0x80000000 0x12 0x80000000 0x0 0x30000000 /* prefetchable memory (768MB) */ > + 0x82000000 0x0 0x40000000 0x12 0xB0000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */ > + }; > + > + pcie@14160000 { > + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; > + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX4A>; > + reg = <0x00 0x14160000 0x0 0x00020000 /* appl registers (128K) */ > + 0x00 0x36000000 0x0 0x00040000 /* configuration space (256K) */ > + 0x00 0x36040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ > + 0x00 0x36080000 0x0 0x00040000>; /* DBI reg space (256K) */ > + reg-names = "appl", "config", "atu_dma", "dbi"; > + > + status = "disabled"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + num-lanes = <4>; > + num-viewport = <8>; > + linux,pci-domain = <4>; > + > + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_4>; > + clock-names = "core"; > + > + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_4_APB>, > + <&bpmp TEGRA194_RESET_PEX0_CORE_4>; > + reset-names = "core_apb", "core"; > + > + interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ > + <GIC_SPI 52 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 0 51 0x04>; > + > + nvidia,bpmp = <&bpmp>; > + > + supports-clkreq; > + nvidia,disable-aspm-states = <0xf>; > + nvidia,controller-id = <4>; > + nvidia,aspm-cmrt-us = <60>; > + nvidia,aspm-pwr-on-t-us = <20>; > + nvidia,aspm-l0s-entrance-latency-us = <3>; > + > + bus-range = <0x0 0xff>; > + ranges = <0x81000000 0x0 0x36100000 0x0 0x36100000 0x0 0x00100000 /* downstream I/O (1MB) */ > + 0xc2000000 0x14 0x00000000 0x14 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */ > + 0x82000000 0x0 0x40000000 0x17 0x40000000 0x0 0xC0000000>; /* non-prefetchable memory (3GB) */ > + }; > + > + pcie@141a0000 { > + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; > + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX8A>; > + reg = <0x00 0x141a0000 0x0 0x00020000 /* appl registers (128K) */ > + 0x00 0x3a000000 0x0 0x00040000 /* configuration space (256K) */ > + 0x00 0x3a040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ > + 0x00 0x3a080000 0x0 0x00040000>; /* DBI reg space (256K) */ > + reg-names = "appl", "config", "atu_dma", "dbi"; > + > + status = "disabled"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + num-lanes = <8>; > + num-viewport = <8>; > + linux,pci-domain = <5>; > + > + clocks = <&bpmp TEGRA194_CLK_PEX1_CORE_5>, > + <&bpmp TEGRA194_CLK_PEX1_CORE_5M>; > + clock-names = "core", "core_m"; > + > + resets = <&bpmp TEGRA194_RESET_PEX1_CORE_5_APB>, > + <&bpmp TEGRA194_RESET_PEX1_CORE_5>; > + reset-names = "core_apb", "core"; > + > + interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ > + <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ > + interrupt-names = "intr", "msi"; > + > + nvidia,bpmp = <&bpmp>; > + > + #interrupt-cells = <1>; > + interrupt-map-mask = <0 0 0 0>; > + interrupt-map = <0 0 0 0 &gic 0 53 0x04>; > + > + supports-clkreq; > + nvidia,disable-aspm-states = <0xf>; > + nvidia,controller-id = <5>; > + nvidia,aspm-cmrt-us = <60>; > + nvidia,aspm-pwr-on-t-us = <20>; > + nvidia,aspm-l0s-entrance-latency-us = <3>; > + > + bus-range = <0x0 0xff>; > + ranges = <0x81000000 0x0 0x3a100000 0x0 0x3a100000 0x0 0x00100000 /* downstream I/O (1MB) */ > + 0xc2000000 0x1c 0x00000000 0x1c 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */ > + 0x82000000 0x0 0x40000000 0x1f 0x40000000 0x0 0xC0000000>; /* non-prefetchable memory (3GB) */ > + }; > }; > -- > 2.7.4 >
On Fri, Apr 05, 2019 at 01:24:41AM +0530, Vidya Sagar wrote: > Synopsys DesignWare core based PCIe controllers in Tegra 194 SoC interface > with Universal PHY (UPHY) module through a PIPE2UPHY (P2U) module. > For each PCIe lane of a controller, there is a P2U unit instantiated at > hardware level. This driver provides support for the programming required > for each P2U that is going to be used for a PCIe controller. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > Changes since [v1]: > * Added COMPILE_TEST in Kconfig > * Removed empty phy_ops implementations > * Modified code according to DT documentation file modifications > > drivers/phy/tegra/Kconfig | 7 ++ > drivers/phy/tegra/Makefile | 1 + > drivers/phy/tegra/pcie-p2u-tegra194.c | 120 ++++++++++++++++++++++++++++++++++ > 3 files changed, 128 insertions(+) > create mode 100644 drivers/phy/tegra/pcie-p2u-tegra194.c > > diff --git a/drivers/phy/tegra/Kconfig b/drivers/phy/tegra/Kconfig > index a3b1de953fb7..eb93ee72ecf0 100644 > --- a/drivers/phy/tegra/Kconfig > +++ b/drivers/phy/tegra/Kconfig > @@ -6,3 +6,10 @@ config PHY_TEGRA_XUSB > > To compile this driver as a module, choose M here: the module will > be called phy-tegra-xusb. > + > +config PHY_TEGRA194_PCIE_P2U > + tristate "NVIDIA Tegra P2U PHY Driver" > + depends on ARCH_TEGRA || COMPILE_TEST > + select GENERIC_PHY > + help > + Enable this to support the P2U (PIPE to UPHY) that is part of Tegra 19x SOCs. This should be using tabs for indentation. > diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile > index 898589238fd9..f85b2c86643d 100644 > --- a/drivers/phy/tegra/Makefile > +++ b/drivers/phy/tegra/Makefile > @@ -4,3 +4,4 @@ phy-tegra-xusb-y += xusb.o > phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_124_SOC) += xusb-tegra124.o > phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o > phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o > +obj-$(CONFIG_PHY_TEGRA194_PCIE_P2U) += pcie-p2u-tegra194.o > diff --git a/drivers/phy/tegra/pcie-p2u-tegra194.c b/drivers/phy/tegra/pcie-p2u-tegra194.c > new file mode 100644 > index 000000000000..d4df8bfa9979 > --- /dev/null > +++ b/drivers/phy/tegra/pcie-p2u-tegra194.c > @@ -0,0 +1,120 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * P2U (PIPE to UPHY) driver for Tegra T194 SoC > + * > + * Copyright (C) 2019 NVIDIA Corporation. > + * > + * Author: Vidya Sagar <vidyas@nvidia.com> > + */ > + > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/phy/phy.h> > +#include <linux/of.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/interrupt.h> > +#include <linux/delay.h> > +#include <linux/of_platform.h> > +#include <soc/tegra/bpmp-abi.h> It's a good idea to keep these sorted alphabetically. That makes it a lot easier to insert new ones in the right place subsequently. > + > +#define P2U_PERIODIC_EQ_CTRL_GEN3 0xc0 > +#define P2U_PERIODIC_EQ_CTRL_GEN3_PERIODIC_EQ_EN BIT(0) > +#define P2U_PERIODIC_EQ_CTRL_GEN3_INIT_PRESET_EQ_TRAIN_EN BIT(1) > +#define P2U_PERIODIC_EQ_CTRL_GEN4 0xc4 > +#define P2U_PERIODIC_EQ_CTRL_GEN4_INIT_PRESET_EQ_TRAIN_EN BIT(1) > + > +#define P2U_RX_DEBOUNCE_TIME 0xa4 > +#define P2U_RX_DEBOUNCE_TIME_DEBOUNCE_TIMER_MASK 0xFFFF Use consistent case for hexadecimal. All other register definitions use lower-case, so this one should, too. > +#define P2U_RX_DEBOUNCE_TIME_DEBOUNCE_TIMER_VAL 160 > + > +struct tegra_p2u { > + void __iomem *base; I personally wouldn't bother with the extra padding. A single space is enough and avoid extra churn if you ever add something here that doesn't fit into the existing padding space. > +}; > + > +static int tegra_p2u_power_on(struct phy *x) > +{ > + u32 val; > + struct tegra_p2u *phy = phy_get_drvdata(x); It's often common to structure these as "reverse christmas tree" so that the longest lines go first, followed by shorter lines. Not sure if Kishon cares, though. > + > + val = readl(phy->base + P2U_PERIODIC_EQ_CTRL_GEN3); > + val &= ~P2U_PERIODIC_EQ_CTRL_GEN3_PERIODIC_EQ_EN; > + val |= P2U_PERIODIC_EQ_CTRL_GEN3_INIT_PRESET_EQ_TRAIN_EN; > + writel(val, phy->base + P2U_PERIODIC_EQ_CTRL_GEN3); > + > + val = readl(phy->base + P2U_PERIODIC_EQ_CTRL_GEN4); > + val |= P2U_PERIODIC_EQ_CTRL_GEN4_INIT_PRESET_EQ_TRAIN_EN; > + writel(val, phy->base + P2U_PERIODIC_EQ_CTRL_GEN4); > + > + val = readl(phy->base + P2U_RX_DEBOUNCE_TIME); > + val &= ~P2U_RX_DEBOUNCE_TIME_DEBOUNCE_TIMER_MASK; > + val |= P2U_RX_DEBOUNCE_TIME_DEBOUNCE_TIMER_VAL; > + writel(val, phy->base + P2U_RX_DEBOUNCE_TIME); > + > + return 0; > +} > + > +static const struct phy_ops ops = { > + .power_on = tegra_p2u_power_on, > + .owner = THIS_MODULE, > +}; > + > +static int tegra_p2u_probe(struct platform_device *pdev) > +{ > + struct tegra_p2u *phy; > + struct phy *generic_phy; > + struct phy_provider *phy_provider; > + struct device *dev = &pdev->dev; > + struct resource *res; > + > + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL); > + if (!phy) > + return -ENOMEM; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctl"); > + phy->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(phy->base)) > + return PTR_ERR_OR_ZERO(phy->base); > + > + platform_set_drvdata(pdev, phy); > + > + generic_phy = devm_phy_create(dev, NULL, &ops); > + if (IS_ERR(generic_phy)) > + return PTR_ERR_OR_ZERO(generic_phy); > + > + phy_set_drvdata(generic_phy, phy); > + > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > + if (IS_ERR(phy_provider)) > + return PTR_ERR_OR_ZERO(phy_provider); > + > + return 0; > +} > + > +static int tegra_p2u_remove(struct platform_device *pdev) > +{ > + return 0; > +} I think you can drop this. > + > +static const struct of_device_id tegra_p2u_id_table[] = { > + { > + .compatible = "nvidia,tegra194-p2u", > + }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, tegra_p2u_id_table); > + > +static struct platform_driver tegra_p2u_driver = { > + .probe = tegra_p2u_probe, > + .remove = tegra_p2u_remove, > + .driver = { > + .name = "tegra194-p2u", > + .of_match_table = tegra_p2u_id_table, > + }, > +}; > + > +module_platform_driver(tegra_p2u_driver); > + > +MODULE_AUTHOR("Vidya Sagar <vidyas@nvidia.com>"); > +MODULE_DESCRIPTION("NVIDIA Tegra PIPE_To_UPHY phy driver"); The driver description is somewhat odd here. Perhaps something like: "NVIDIA Tegra PIPE to UPHY PHY driver" ? Thierry > +MODULE_LICENSE("GPL v2"); > -- > 2.7.4 >
On Mon, Apr 15, 2019 at 05:31:48PM +0200, Thierry Reding wrote: > On Fri, Apr 05, 2019 at 01:24:41AM +0530, Vidya Sagar wrote: [...] > > diff --git a/drivers/phy/tegra/pcie-p2u-tegra194.c b/drivers/phy/tegra/pcie-p2u-tegra194.c > > new file mode 100644 > > index 000000000000..d4df8bfa9979 > > --- /dev/null > > +++ b/drivers/phy/tegra/pcie-p2u-tegra194.c [...] > > +MODULE_AUTHOR("Vidya Sagar <vidyas@nvidia.com>"); > > +MODULE_DESCRIPTION("NVIDIA Tegra PIPE_To_UPHY phy driver"); > > The driver description is somewhat odd here. Perhaps something like: > > "NVIDIA Tegra PIPE to UPHY PHY driver" > > ? Or perhaps just "NVIDIA Tegra PIPE2UPHY PHY driver" which is what you already use in the commit subject. Thierry
On 4/11/2019 3:43 PM, Thierry Reding wrote: > On Fri, Apr 05, 2019 at 01:24:28AM +0530, Vidya Sagar wrote: >> Add #defines for the Data Link Feature and Physical Layer 16.0 GT/s >> features. >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> Changes from [v1]: >> * None >> >> include/uapi/linux/pci_regs.h | 22 +++++++++++++++++++++- >> 1 file changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h >> index 5c98133f2c94..3e01b55d548d 100644 >> --- a/include/uapi/linux/pci_regs.h >> +++ b/include/uapi/linux/pci_regs.h >> @@ -705,7 +705,9 @@ >> #define PCI_EXT_CAP_ID_DPC 0x1D /* Downstream Port Containment */ >> #define PCI_EXT_CAP_ID_L1SS 0x1E /* L1 PM Substates */ >> #define PCI_EXT_CAP_ID_PTM 0x1F /* Precision Time Measurement */ >> -#define PCI_EXT_CAP_ID_MAX PCI_EXT_CAP_ID_PTM >> +#define PCI_EXT_CAP_ID_DLF 0x25 /* Data Link Feature */ >> +#define PCI_EXT_CAP_ID_PL 0x26 /* Physical Layer 16.0 GT/s */ >> +#define PCI_EXT_CAP_ID_MAX PCI_EXT_CAP_ID_PL >> >> #define PCI_EXT_CAP_DSN_SIZEOF 12 >> #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40 >> @@ -1045,4 +1047,22 @@ >> #define PCI_L1SS_CTL1_LTR_L12_TH_SCALE 0xe0000000 /* LTR_L1.2_THRESHOLD_Scale */ >> #define PCI_L1SS_CTL2 0x0c /* Control 2 Register */ >> >> +/* Data Link Feature */ >> +#define PCI_DLF_CAP 0x04 /* Capabilities Register */ >> +#define PCI_DLF_LOCAL_DLF_SUP_MASK 0x007fffff /* Local Data Link Feature Supported */ >> +#define PCI_DLF_EXCHANGE_ENABLE 0x80000000 /* Data Link Feature Exchange Enable */ >> +#define PCI_DLF_STS 0x08 /* Status Register */ >> +#define PCI_DLF_REMOTE_DLF_SUP_MASK 0x007fffff /* Remote Data Link Feature Supported */ >> +#define PCI_DLF_REMOTE_DLF_SUP_VALID 0x80000000 /* Remote Data Link Feature Support Valid */ >> + >> +/* Physical Layer 16.0 GT/s */ >> +#define PCI_PL_16GT_CAP 0x04 /* Capabilities Register */ >> +#define PCI_PL_16GT_CTRL 0x08 /* Control Register */ >> +#define PCI_PL_16GT_STS 0x0c /* Status Register */ >> +#define PCI_PL_16GT_LDPM_STS 0x10 /* Local Data Parity Mismatch Status Register */ >> +#define PCI_PL_16GT_FRDPM_STS 0x14 /* First Retimer Data Parity Mismatch Status Register */ >> +#define PCI_PL_16GT_SRDPM_STS 0x18 /* Second Retimer Data Parity Mismatch Status Register */ >> +#define PCI_PL_16GT_RSVD 0x1C /* Reserved */ >> +#define PCI_PL_16GT_LE_CTRL 0x20 /* Lane Equalization Control Register */ > > This looks correct comparing to the specification. However, this leaves > out some definitions, so I'm wondering if perhaps this should include > all field definitions. There are also extended capabilities between the > current maximum 0x1F and 0x25. Perhaps those should be added as well. I > guess this could always be done as a follow-up. > > Perhaps it'd be better to change the subject to more accurately reflect > that you're only adding a couple of PCIe 4.0 features. I'll change subject accordingly. > > Other than that: > > Reviewed-by: Thierry Reding <treding@nvidia.com> >
On 4/11/2019 3:46 PM, Thierry Reding wrote: > On Fri, Apr 05, 2019 at 01:24:29AM +0530, Vidya Sagar wrote: >> Export pcie_pme_disable_msi() API to enable drivers using this API be able to >> build as loadable modules >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> Changes from [v1]: >> * This is a new patch in v2 series >> >> drivers/pci/pcie/pme.c | 6 ++++++ >> drivers/pci/pcie/portdrv.h | 5 +---- >> 2 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c >> index 54d593d10396..18e815c1e3c9 100644 >> --- a/drivers/pci/pcie/pme.c >> +++ b/drivers/pci/pcie/pme.c >> @@ -27,6 +27,12 @@ >> */ >> bool pcie_pme_msi_disabled; >> >> +void pcie_pme_disable_msi(void) >> +{ >> + pcie_pme_msi_disabled = true; >> +} >> +EXPORT_SYMBOL_GPL(pcie_pme_disable_msi); >> + >> static int __init pcie_pme_setup(char *str) >> { >> if (!strncmp(str, "nomsi", 5)) >> diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h >> index 1d50dc58ac40..2be7b7e9a784 100644 >> --- a/drivers/pci/pcie/portdrv.h >> +++ b/drivers/pci/pcie/portdrv.h >> @@ -127,10 +127,7 @@ struct pci_dev; >> #ifdef CONFIG_PCIE_PME >> extern bool pcie_pme_msi_disabled; >> >> -static inline void pcie_pme_disable_msi(void) >> -{ >> - pcie_pme_msi_disabled = true; >> -} >> +void pcie_pme_disable_msi(void); >> >> static inline bool pcie_pme_no_msi(void) >> { > > Perhaps also export pcie_pme_no_msi()? That way you could get rid of the > extern declaration of the pcie_pme_msi_disabled variable and make it > static to drivers/pci/pcie/pme.c? Agree. I'll take care of this in V3 patch series. > > Thierry >
On 4/15/2019 8:45 PM, Thierry Reding wrote: > On Fri, Apr 05, 2019 at 01:24:39AM +0530, Vidya Sagar wrote: >> Add P2U (PIPE to UPHY) and PCIe controller nodes to device tree. >> The Tegra194 SoC contains six PCIe controllers and twenty P2U instances >> grouped into two different PHY bricks namely High-Speed IO (HSIO-12 P2Us) >> and NVIDIA High Speed (NVHS-8 P2Us) respectively. >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> Changes since [v1]: >> * Flattened all P2U nodes by removing 'hsio-p2u' and 'nvhs-p2u' super nodes >> * Changed P2U nodes compatible string from 'nvidia,tegra194-phy-p2u' to 'nvidia,tegra194-p2u' >> * Changed reg-name from 'base' to 'ctl' >> * Updated all PCIe nodes according to the changes made to DT documentation file >> >> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 449 +++++++++++++++++++++++++++++++ >> 1 file changed, 449 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi >> index c77ca211fa8f..5b62136d97a5 100644 >> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi >> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi >> @@ -884,6 +884,166 @@ >> nvidia,interface = <3>; >> }; >> }; >> + >> + p2u_0: p2u@03e10000 { /* HSIO-Lane-0 */ >> + compatible = "nvidia,tegra194-p2u"; >> + reg = <0x03e10000 0x10000>; >> + reg-names = "ctl"; >> + >> + #phy-cells = <0>; >> + }; >> + > [...] >> + p2u_12: p2u@03eb0000 { /* NVHS-Lane-0 */ >> + compatible = "nvidia,tegra194-p2u"; >> + reg = <0x03eb0000 0x10000>; >> + reg-names = "ctl"; >> + >> + #phy-cells = <0>; >> + }; > [...] > > Do we perhaps want to include the type of P2U in the label? That would > make it more obvious which ones to list in the PCIe controller nodes' > phys properties. Something like: > > p2u_hsio_0: p2u@3e10000 { > ... > }; > > ... > > p2u_nvhs_0: p2u@3eb0000 { > ... > }; > > ? Also, make sure to drop the leading 0 from unit-addresses. Recent > versions of DTC have checks for that in place and will warn about it in > recent Linux builds. Done. > > [...] >> @@ -1054,4 +1214,293 @@ >> (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; >> interrupt-parent = <&gic>; >> }; >> + >> + pcie@14180000 { >> + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; >> + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX8B>; >> + reg = <0x00 0x14180000 0x0 0x00020000 /* appl registers (128K) */ >> + 0x00 0x38000000 0x0 0x00040000 /* configuration space (256K) */ >> + 0x00 0x38040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ >> + 0x00 0x38080000 0x0 0x00040000>; /* DBI reg space (256K) */ >> + reg-names = "appl", "config", "atu_dma", "dbi"; >> + >> + status = "disabled"; >> + >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + num-lanes = <8>; >> + num-viewport = <8>; >> + linux,pci-domain = <0>; >> + >> + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_0>; >> + clock-names = "core"; >> + >> + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_0_APB>, >> + <&bpmp TEGRA194_RESET_PEX0_CORE_0>; >> + reset-names = "core_apb", "core"; >> + >> + 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 0 72 0x04>; >> + >> + nvidia,bpmp = <&bpmp>; >> + >> + supports-clkreq; >> + nvidia,disable-aspm-states = <0xf>; >> + nvidia,controller-id = <0>; >> + nvidia,aspm-cmrt-us = <60>; >> + nvidia,aspm-pwr-on-t-us = <20>; >> + nvidia,aspm-l0s-entrance-latency-us = <3>; > > Didn't you remove some of these from the bindings? I've removed some but what we see here are still present in bindings. > > Thierry > >> + >> + bus-range = <0x0 0xff>; >> + ranges = <0x81000000 0x0 0x38100000 0x0 0x38100000 0x0 0x00100000 /* downstream I/O (1MB) */ >> + 0xc2000000 0x18 0x00000000 0x18 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */ >> + 0x82000000 0x0 0x40000000 0x1B 0x40000000 0x0 0xC0000000>; /* non-prefetchable memory (3GB) */ >> + }; >> + >> + pcie@14100000 { >> + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; >> + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX1A>; >> + reg = <0x00 0x14100000 0x0 0x00020000 /* appl registers (128K) */ >> + 0x00 0x30000000 0x0 0x00040000 /* configuration space (256K) */ >> + 0x00 0x30040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ >> + 0x00 0x30080000 0x0 0x00040000>; /* DBI reg space (256K) */ >> + reg-names = "appl", "config", "atu_dma", "dbi"; >> + >> + status = "disabled"; >> + >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + num-lanes = <1>; >> + num-viewport = <8>; >> + linux,pci-domain = <1>; >> + >> + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_1>; >> + clock-names = "core"; >> + >> + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_1_APB>, >> + <&bpmp TEGRA194_RESET_PEX0_CORE_1>; >> + reset-names = "core_apb", "core"; >> + >> + interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ >> + <GIC_SPI 46 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 0 45 0x04>; >> + >> + nvidia,bpmp = <&bpmp>; >> + >> + supports-clkreq; >> + nvidia,disable-aspm-states = <0xf>; >> + nvidia,controller-id = <1>; >> + nvidia,aspm-cmrt-us = <60>; >> + nvidia,aspm-pwr-on-t-us = <20>; >> + nvidia,aspm-l0s-entrance-latency-us = <3>; >> + >> + bus-range = <0x0 0xff>; >> + ranges = <0x81000000 0x0 0x30100000 0x0 0x30100000 0x0 0x00100000 /* downstream I/O (1MB) */ >> + 0xc2000000 0x12 0x00000000 0x12 0x00000000 0x0 0x30000000 /* prefetchable memory (768MB) */ >> + 0x82000000 0x0 0x40000000 0x12 0x30000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */ >> + }; >> + >> + pcie@14120000 { >> + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; >> + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX1A>; >> + reg = <0x00 0x14120000 0x0 0x00020000 /* appl registers (128K) */ >> + 0x00 0x32000000 0x0 0x00040000 /* configuration space (256K) */ >> + 0x00 0x32040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ >> + 0x00 0x32080000 0x0 0x00040000>; /* DBI reg space (256K) */ >> + reg-names = "appl", "config", "atu_dma", "dbi"; >> + >> + status = "disabled"; >> + >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + num-lanes = <1>; >> + num-viewport = <8>; >> + linux,pci-domain = <2>; >> + >> + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_2>; >> + clock-names = "core"; >> + >> + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_2_APB>, >> + <&bpmp TEGRA194_RESET_PEX0_CORE_2>; >> + reset-names = "core_apb", "core"; >> + >> + interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ >> + <GIC_SPI 48 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 0 47 0x04>; >> + >> + nvidia,bpmp = <&bpmp>; >> + >> + supports-clkreq; >> + nvidia,disable-aspm-states = <0xf>; >> + nvidia,controller-id = <2>; >> + nvidia,aspm-cmrt-us = <60>; >> + nvidia,aspm-pwr-on-t-us = <20>; >> + nvidia,aspm-l0s-entrance-latency-us = <3>; >> + >> + bus-range = <0x0 0xff>; >> + ranges = <0x81000000 0x0 0x32100000 0x0 0x32100000 0x0 0x00100000 /* downstream I/O (1MB) */ >> + 0xc2000000 0x12 0x40000000 0x12 0x40000000 0x0 0x30000000 /* prefetchable memory (768MB) */ >> + 0x82000000 0x0 0x40000000 0x12 0x70000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */ >> + }; >> + >> + pcie@14140000 { >> + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; >> + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX1A>; >> + reg = <0x00 0x14140000 0x0 0x00020000 /* appl registers (128K) */ >> + 0x00 0x34000000 0x0 0x00040000 /* configuration space (256K) */ >> + 0x00 0x34040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ >> + 0x00 0x34080000 0x0 0x00040000>; /* DBI reg space (256K) */ >> + reg-names = "appl", "config", "atu_dma", "dbi"; >> + >> + status = "disabled"; >> + >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + num-lanes = <1>; >> + num-viewport = <8>; >> + linux,pci-domain = <3>; >> + >> + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_3>; >> + clock-names = "core"; >> + >> + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_3_APB>, >> + <&bpmp TEGRA194_RESET_PEX0_CORE_3>; >> + reset-names = "core_apb", "core"; >> + >> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ >> + <GIC_SPI 50 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 0 49 0x04>; >> + >> + nvidia,bpmp = <&bpmp>; >> + >> + supports-clkreq; >> + nvidia,disable-aspm-states = <0xf>; >> + nvidia,controller-id = <3>; >> + nvidia,aspm-cmrt-us = <60>; >> + nvidia,aspm-pwr-on-t-us = <20>; >> + nvidia,aspm-l0s-entrance-latency-us = <3>; >> + >> + bus-range = <0x0 0xff>; >> + ranges = <0x81000000 0x0 0x34100000 0x0 0x34100000 0x0 0x00100000 /* downstream I/O (1MB) */ >> + 0xc2000000 0x12 0x80000000 0x12 0x80000000 0x0 0x30000000 /* prefetchable memory (768MB) */ >> + 0x82000000 0x0 0x40000000 0x12 0xB0000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */ >> + }; >> + >> + pcie@14160000 { >> + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; >> + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX4A>; >> + reg = <0x00 0x14160000 0x0 0x00020000 /* appl registers (128K) */ >> + 0x00 0x36000000 0x0 0x00040000 /* configuration space (256K) */ >> + 0x00 0x36040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ >> + 0x00 0x36080000 0x0 0x00040000>; /* DBI reg space (256K) */ >> + reg-names = "appl", "config", "atu_dma", "dbi"; >> + >> + status = "disabled"; >> + >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + num-lanes = <4>; >> + num-viewport = <8>; >> + linux,pci-domain = <4>; >> + >> + clocks = <&bpmp TEGRA194_CLK_PEX0_CORE_4>; >> + clock-names = "core"; >> + >> + resets = <&bpmp TEGRA194_RESET_PEX0_CORE_4_APB>, >> + <&bpmp TEGRA194_RESET_PEX0_CORE_4>; >> + reset-names = "core_apb", "core"; >> + >> + interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ >> + <GIC_SPI 52 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 0 51 0x04>; >> + >> + nvidia,bpmp = <&bpmp>; >> + >> + supports-clkreq; >> + nvidia,disable-aspm-states = <0xf>; >> + nvidia,controller-id = <4>; >> + nvidia,aspm-cmrt-us = <60>; >> + nvidia,aspm-pwr-on-t-us = <20>; >> + nvidia,aspm-l0s-entrance-latency-us = <3>; >> + >> + bus-range = <0x0 0xff>; >> + ranges = <0x81000000 0x0 0x36100000 0x0 0x36100000 0x0 0x00100000 /* downstream I/O (1MB) */ >> + 0xc2000000 0x14 0x00000000 0x14 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */ >> + 0x82000000 0x0 0x40000000 0x17 0x40000000 0x0 0xC0000000>; /* non-prefetchable memory (3GB) */ >> + }; >> + >> + pcie@141a0000 { >> + compatible = "nvidia,tegra194-pcie", "snps,dw-pcie"; >> + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_PCIEX8A>; >> + reg = <0x00 0x141a0000 0x0 0x00020000 /* appl registers (128K) */ >> + 0x00 0x3a000000 0x0 0x00040000 /* configuration space (256K) */ >> + 0x00 0x3a040000 0x0 0x00040000 /* iATU_DMA reg space (256K) */ >> + 0x00 0x3a080000 0x0 0x00040000>; /* DBI reg space (256K) */ >> + reg-names = "appl", "config", "atu_dma", "dbi"; >> + >> + status = "disabled"; >> + >> + #address-cells = <3>; >> + #size-cells = <2>; >> + device_type = "pci"; >> + num-lanes = <8>; >> + num-viewport = <8>; >> + linux,pci-domain = <5>; >> + >> + clocks = <&bpmp TEGRA194_CLK_PEX1_CORE_5>, >> + <&bpmp TEGRA194_CLK_PEX1_CORE_5M>; >> + clock-names = "core", "core_m"; >> + >> + resets = <&bpmp TEGRA194_RESET_PEX1_CORE_5_APB>, >> + <&bpmp TEGRA194_RESET_PEX1_CORE_5>; >> + reset-names = "core_apb", "core"; >> + >> + interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ >> + <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ >> + interrupt-names = "intr", "msi"; >> + >> + nvidia,bpmp = <&bpmp>; >> + >> + #interrupt-cells = <1>; >> + interrupt-map-mask = <0 0 0 0>; >> + interrupt-map = <0 0 0 0 &gic 0 53 0x04>; >> + >> + supports-clkreq; >> + nvidia,disable-aspm-states = <0xf>; >> + nvidia,controller-id = <5>; >> + nvidia,aspm-cmrt-us = <60>; >> + nvidia,aspm-pwr-on-t-us = <20>; >> + nvidia,aspm-l0s-entrance-latency-us = <3>; >> + >> + bus-range = <0x0 0xff>; >> + ranges = <0x81000000 0x0 0x3a100000 0x0 0x3a100000 0x0 0x00100000 /* downstream I/O (1MB) */ >> + 0xc2000000 0x1c 0x00000000 0x1c 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */ >> + 0x82000000 0x0 0x40000000 0x1f 0x40000000 0x0 0xC0000000>; /* non-prefetchable memory (3GB) */ >> + }; >> }; >> -- >> 2.7.4 >>
On 4/15/2019 8:42 PM, Thierry Reding wrote: > On Fri, Apr 05, 2019 at 01:24:40AM +0530, Vidya Sagar wrote: >> Enable PCIe controller nodes to enable respective PCIe slots on >> P2972-0000 board. Following is the ownership of slots by different >> PCIe controllers. >> Controller-0 : M.2 Key-M slot >> Controller-1 : On-board Marvell eSATA controller >> Controller-3 : M.2 Key-E slot >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> Changes since [v1]: >> * Dropped 'pcie-' from phy-names property strings >> >> arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 2 +- >> arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts | 50 ++++++++++++++++++++++ >> 2 files changed, 51 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi >> index 246c1ebbd055..13263529125b 100644 >> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi >> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi >> @@ -191,7 +191,7 @@ >> regulator-boot-on; >> }; >> >> - sd3 { >> + vdd_1v8ao: sd3 { >> regulator-name = "VDD_1V8AO"; >> regulator-min-microvolt = <1800000>; >> regulator-max-microvolt = <1800000>; >> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> index b62e96945846..82eb30bceaa6 100644 >> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> @@ -169,4 +169,54 @@ >> }; >> }; >> }; >> + >> + pcie@14180000 { >> + status = "okay"; >> + >> + vddio-pex-ctl-supply = <&vdd_1v8ao>; >> + >> + phys = <&p2u_2>, >> + <&p2u_3>, >> + <&p2u_4>, >> + <&p2u_5>; > > You can use multiple entries on a single line, especially if they are > this short. > >> + phy-names = "p2u-0", "p2u-1", "p2u-2", >> + "p2u-3"; > > Same here. > >> + }; >> + >> + pcie@14100000 { >> + status = "okay"; >> + >> + vddio-pex-ctl-supply = <&vdd_1v8ao>; >> + >> + phys = <&p2u_0>; >> + phy-names = "p2u-0"; >> + }; >> + >> + pcie@14140000 { >> + status = "okay"; >> + >> + vddio-pex-ctl-supply = <&vdd_1v8ao>; >> + >> + phys = <&p2u_7>; >> + phy-names = "p2u-0"; >> + }; >> + >> + pcie@141a0000 { >> + status = "disabled"; >> + >> + vddio-pex-ctl-supply = <&vdd_1v8ao>; >> + >> + phys = <&p2u_12>, >> + <&p2u_13>, >> + <&p2u_14>, >> + <&p2u_15>, >> + <&p2u_16>, >> + <&p2u_17>, >> + <&p2u_18>, >> + <&p2u_19>; >> + >> + phy-names = "p2u-0", "p2u-1", "p2u-2", >> + "p2u-3", "p2u-4", "p2u-5", >> + "p2u-6", "p2u-7"; > > And here. I'll take care of all these in V3 series. > > Thierry > >> + }; >> }; >> -- >> 2.7.4 >>
On 4/15/2019 9:01 PM, Thierry Reding wrote: > On Fri, Apr 05, 2019 at 01:24:41AM +0530, Vidya Sagar wrote: >> Synopsys DesignWare core based PCIe controllers in Tegra 194 SoC interface >> with Universal PHY (UPHY) module through a PIPE2UPHY (P2U) module. >> For each PCIe lane of a controller, there is a P2U unit instantiated at >> hardware level. This driver provides support for the programming required >> for each P2U that is going to be used for a PCIe controller. >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> Changes since [v1]: >> * Added COMPILE_TEST in Kconfig >> * Removed empty phy_ops implementations >> * Modified code according to DT documentation file modifications >> >> drivers/phy/tegra/Kconfig | 7 ++ >> drivers/phy/tegra/Makefile | 1 + >> drivers/phy/tegra/pcie-p2u-tegra194.c | 120 ++++++++++++++++++++++++++++++++++ >> 3 files changed, 128 insertions(+) >> create mode 100644 drivers/phy/tegra/pcie-p2u-tegra194.c >> >> diff --git a/drivers/phy/tegra/Kconfig b/drivers/phy/tegra/Kconfig >> index a3b1de953fb7..eb93ee72ecf0 100644 >> --- a/drivers/phy/tegra/Kconfig >> +++ b/drivers/phy/tegra/Kconfig >> @@ -6,3 +6,10 @@ config PHY_TEGRA_XUSB >> >> To compile this driver as a module, choose M here: the module will >> be called phy-tegra-xusb. >> + >> +config PHY_TEGRA194_PCIE_P2U >> + tristate "NVIDIA Tegra P2U PHY Driver" >> + depends on ARCH_TEGRA || COMPILE_TEST >> + select GENERIC_PHY >> + help >> + Enable this to support the P2U (PIPE to UPHY) that is part of Tegra 19x SOCs. > > This should be using tabs for indentation. Done. > >> diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile >> index 898589238fd9..f85b2c86643d 100644 >> --- a/drivers/phy/tegra/Makefile >> +++ b/drivers/phy/tegra/Makefile >> @@ -4,3 +4,4 @@ phy-tegra-xusb-y += xusb.o >> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_124_SOC) += xusb-tegra124.o >> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o >> phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o >> +obj-$(CONFIG_PHY_TEGRA194_PCIE_P2U) += pcie-p2u-tegra194.o >> diff --git a/drivers/phy/tegra/pcie-p2u-tegra194.c b/drivers/phy/tegra/pcie-p2u-tegra194.c >> new file mode 100644 >> index 000000000000..d4df8bfa9979 >> --- /dev/null >> +++ b/drivers/phy/tegra/pcie-p2u-tegra194.c >> @@ -0,0 +1,120 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * P2U (PIPE to UPHY) driver for Tegra T194 SoC >> + * >> + * Copyright (C) 2019 NVIDIA Corporation. >> + * >> + * Author: Vidya Sagar <vidyas@nvidia.com> >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/phy/phy.h> >> +#include <linux/of.h> >> +#include <linux/err.h> >> +#include <linux/io.h> >> +#include <linux/interrupt.h> >> +#include <linux/delay.h> >> +#include <linux/of_platform.h> >> +#include <soc/tegra/bpmp-abi.h> > > It's a good idea to keep these sorted alphabetically. That makes it a > lot easier to insert new ones in the right place subsequently. Done. > >> + >> +#define P2U_PERIODIC_EQ_CTRL_GEN3 0xc0 >> +#define P2U_PERIODIC_EQ_CTRL_GEN3_PERIODIC_EQ_EN BIT(0) >> +#define P2U_PERIODIC_EQ_CTRL_GEN3_INIT_PRESET_EQ_TRAIN_EN BIT(1) >> +#define P2U_PERIODIC_EQ_CTRL_GEN4 0xc4 >> +#define P2U_PERIODIC_EQ_CTRL_GEN4_INIT_PRESET_EQ_TRAIN_EN BIT(1) >> + >> +#define P2U_RX_DEBOUNCE_TIME 0xa4 >> +#define P2U_RX_DEBOUNCE_TIME_DEBOUNCE_TIMER_MASK 0xFFFF > > Use consistent case for hexadecimal. All other register definitions use > lower-case, so this one should, too. > >> +#define P2U_RX_DEBOUNCE_TIME_DEBOUNCE_TIMER_VAL 160 >> + >> +struct tegra_p2u { >> + void __iomem *base; > > I personally wouldn't bother with the extra padding. A single space is > enough and avoid extra churn if you ever add something here that doesn't > fit into the existing padding space. Done. > >> +}; >> + >> +static int tegra_p2u_power_on(struct phy *x) >> +{ >> + u32 val; >> + struct tegra_p2u *phy = phy_get_drvdata(x); > > It's often common to structure these as "reverse christmas tree" so that > the longest lines go first, followed by shorter lines. Not sure if > Kishon cares, though. Done. Took care of this in pcie-tegra194.c as well. > >> + >> + val = readl(phy->base + P2U_PERIODIC_EQ_CTRL_GEN3); >> + val &= ~P2U_PERIODIC_EQ_CTRL_GEN3_PERIODIC_EQ_EN; >> + val |= P2U_PERIODIC_EQ_CTRL_GEN3_INIT_PRESET_EQ_TRAIN_EN; >> + writel(val, phy->base + P2U_PERIODIC_EQ_CTRL_GEN3); >> + >> + val = readl(phy->base + P2U_PERIODIC_EQ_CTRL_GEN4); >> + val |= P2U_PERIODIC_EQ_CTRL_GEN4_INIT_PRESET_EQ_TRAIN_EN; >> + writel(val, phy->base + P2U_PERIODIC_EQ_CTRL_GEN4); >> + >> + val = readl(phy->base + P2U_RX_DEBOUNCE_TIME); >> + val &= ~P2U_RX_DEBOUNCE_TIME_DEBOUNCE_TIMER_MASK; >> + val |= P2U_RX_DEBOUNCE_TIME_DEBOUNCE_TIMER_VAL; >> + writel(val, phy->base + P2U_RX_DEBOUNCE_TIME); >> + >> + return 0; >> +} >> + >> +static const struct phy_ops ops = { >> + .power_on = tegra_p2u_power_on, >> + .owner = THIS_MODULE, >> +}; >> + >> +static int tegra_p2u_probe(struct platform_device *pdev) >> +{ >> + struct tegra_p2u *phy; >> + struct phy *generic_phy; >> + struct phy_provider *phy_provider; >> + struct device *dev = &pdev->dev; >> + struct resource *res; >> + >> + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL); >> + if (!phy) >> + return -ENOMEM; >> + >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctl"); >> + phy->base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(phy->base)) >> + return PTR_ERR_OR_ZERO(phy->base); >> + >> + platform_set_drvdata(pdev, phy); >> + >> + generic_phy = devm_phy_create(dev, NULL, &ops); >> + if (IS_ERR(generic_phy)) >> + return PTR_ERR_OR_ZERO(generic_phy); >> + >> + phy_set_drvdata(generic_phy, phy); >> + >> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); >> + if (IS_ERR(phy_provider)) >> + return PTR_ERR_OR_ZERO(phy_provider); >> + >> + return 0; >> +} >> + >> +static int tegra_p2u_remove(struct platform_device *pdev) >> +{ >> + return 0; >> +} > > I think you can drop this. I want to make this as a loadable module hence i kep t .remove() implementation. > >> + >> +static const struct of_device_id tegra_p2u_id_table[] = { >> + { >> + .compatible = "nvidia,tegra194-p2u", >> + }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, tegra_p2u_id_table); >> + >> +static struct platform_driver tegra_p2u_driver = { >> + .probe = tegra_p2u_probe, >> + .remove = tegra_p2u_remove, >> + .driver = { >> + .name = "tegra194-p2u", >> + .of_match_table = tegra_p2u_id_table, >> + }, >> +}; >> + >> +module_platform_driver(tegra_p2u_driver); >> + >> +MODULE_AUTHOR("Vidya Sagar <vidyas@nvidia.com>"); >> +MODULE_DESCRIPTION("NVIDIA Tegra PIPE_To_UPHY phy driver"); > > The driver description is somewhat odd here. Perhaps something like: > > "NVIDIA Tegra PIPE to UPHY PHY driver" > > ? Done. > > Thierry > >> +MODULE_LICENSE("GPL v2"); >> -- >> 2.7.4 >>