| Message ID | 20210602120752.46154-1-manivannan.sadhasivam@linaro.org |
|---|---|
| Headers | show |
| Series | Add Qualcomm PCIe Endpoint driver support | expand |
On Wed, Jun 2, 2021 at 7:08 AM Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > Add devicetree binding for Qualcomm PCIe EP controller used in platforms > like SDX55. The EP controller is based on the Designware core with > Qualcomm specific wrappers. Is the block EP only or configurable EP or host? > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 139 ++++++++++++++++++ > 1 file changed, 139 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > new file mode 100644 > index 000000000000..0f9140e93bcb > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > @@ -0,0 +1,139 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm PCIe Endpoint Controller binding > + > +maintainers: > + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > + > +allOf: > + - $ref: "pci-ep.yaml#" > + > +properties: > + compatible: > + const: qcom,pcie-ep SoC specific please. > + > + reg: > + items: > + - description: Designware PCIe registers > + - description: External local bus interface registers > + - description: Address Translation Unit (ATU) registers > + - description: Memory region used to map remote RC address space > + - description: Qualcomm specific PARF configuration registers > + - description: Qualcomm specific TCSR registers > + > + reg-names: > + items: > + - const: dbi > + - const: elbi > + - const: atu > + - const: addr_space > + - const: parf > + - const: tcsr This should be in the same order as the host side. Unfortunately, that's not consistent, but to pick one: reg-names = "parf", "dbi", "elbi", "atu", "config"; > + > + clocks: > + items: > + - description: PCIe CFG AHB clock > + - description: PCIe Auxiliary clock > + - description: PCIe Master AXI clock > + - description: PCIe Slave AXI clock > + - description: PCIe Reference clock > + - description: PCIe Sleep clock > + - description: PCIe Slave Q2A AXI clock > + > + clock-names: > + items: > + - const: cfg > + - const: aux > + - const: bus_master > + - const: bus_slave > + - const: ref > + - const: sleep > + - const: slave_q2a Again, try to keep the same ordering. I have to wonder where 'pipe' clock is that most of the QCom implementations have? > + > + interrupts: > + maxItems: 1 > + description: PCIe Global interrupt > + > + interrupt-names: > + const: int_global 'int_' is redundant, drop. > + > + perst-gpios: > + description: PCIe endpoint reset GPIO An input, right? > + maxItems: 1 > + > + wake-gpios: > + description: PCIe endpoint wake GPIO > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + reset-names: > + const: core_reset Not yet another name. We already have 'pci' and 'core' in the cases of a single reset. > + > + power-domains: > + maxItems: 1 > + > + phys: > + maxItems: 1 > + > + phy-names: > + const: pciephy > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - clock-names > + - interrupts > + - interrupt-names > + - perst-gpios > + - resets > + - reset-names > + - power-domains > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/qcom,gcc-sdx55.h> > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + pcie_ep: pcie-ep@40000000 { > + compatible = "qcom,pcie-ep"; > + > + reg = <0x40000000 0xf1d>, > + <0x40000f20 0xc8>, > + <0x40001000 0x1000>, > + <0x42000000 0x1000>, > + <0x01c00000 0x3000>, > + <0x01fcb000 0x1000>; > + reg-names = "dbi", "elbi", "atu", "addr_space", "parf", "tcsr"; > + > + clocks = <&gcc GCC_PCIE_CFG_AHB_CLK>, > + <&gcc GCC_PCIE_AUX_CLK>, > + <&gcc GCC_PCIE_MSTR_AXI_CLK>, > + <&gcc GCC_PCIE_SLV_AXI_CLK>, > + <&gcc GCC_PCIE_0_CLKREF_CLK>, > + <&gcc GCC_PCIE_SLEEP_CLK>, > + <&gcc GCC_PCIE_SLV_Q2A_AXI_CLK>; > + clock-names = "cfg", "aux", "bus_master", "bus_slave", > + "ref", "sleep", "slave_q2a"; > + > + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "int_global"; > + perst-gpios = <&tlmm 57 GPIO_ACTIVE_HIGH>; > + wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>; > + resets = <&gcc GCC_PCIE_BCR>; > + reset-names = "core_reset"; > + power-domains = <&gcc PCIE_GDSC>; > + phys = <&pcie0_lane>; > + phy-names = "pciephy"; > + max-link-speed = <3>; > + num-lanes = <2>; Should be documented. I'd assume the max is less than 16 which is presumably what pcie-ep.yaml allows. > + }; > -- > 2.25.1 >
On Wed, Jun 02, 2021 at 09:22:29AM -0500, Rob Herring wrote: > On Wed, Jun 2, 2021 at 7:08 AM Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org> wrote: > > > > Add devicetree binding for Qualcomm PCIe EP controller used in platforms > > like SDX55. The EP controller is based on the Designware core with > > Qualcomm specific wrappers. > > Is the block EP only or configurable EP or host? > Configurable core. We already support the RC mode in a separate driver and binding. I initially thought about merging both in a single driver & binding but that seemed unnecessarily complex, so settled with this. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 139 ++++++++++++++++++ > > 1 file changed, 139 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > > > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > > new file mode 100644 > > index 000000000000..0f9140e93bcb > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > > @@ -0,0 +1,139 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm PCIe Endpoint Controller binding > > + > > +maintainers: > > + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > + > > +allOf: > > + - $ref: "pci-ep.yaml#" > > + > > +properties: > > + compatible: > > + const: qcom,pcie-ep > > SoC specific please. > Okay > > + > > + reg: > > + items: > > + - description: Designware PCIe registers > > + - description: External local bus interface registers > > + - description: Address Translation Unit (ATU) registers > > + - description: Memory region used to map remote RC address space > > + - description: Qualcomm specific PARF configuration registers > > + - description: Qualcomm specific TCSR registers > > + > > + reg-names: > > + items: > > + - const: dbi > > + - const: elbi > > + - const: atu > > + - const: addr_space > > + - const: parf > > + - const: tcsr > > This should be in the same order as the host side. Unfortunately, > that's not consistent, but to pick one: > > reg-names = "parf", "dbi", "elbi", "atu", "config"; > Okay, I'll align with the RC binding. > > > + > > + clocks: > > + items: > > + - description: PCIe CFG AHB clock > > + - description: PCIe Auxiliary clock > > + - description: PCIe Master AXI clock > > + - description: PCIe Slave AXI clock > > + - description: PCIe Reference clock > > + - description: PCIe Sleep clock > > + - description: PCIe Slave Q2A AXI clock > > + > > + clock-names: > > + items: > > + - const: cfg > > + - const: aux > > + - const: bus_master > > + - const: bus_slave > > + - const: ref > > + - const: sleep > > + - const: slave_q2a > > Again, try to keep the same ordering. > > I have to wonder where 'pipe' clock is that most of the QCom > implementations have? > Pipe clock is managed by the PHY driver. So the PCIe drivers need not to worry about it. > > + > > + interrupts: > > + maxItems: 1 > > + description: PCIe Global interrupt > > + > > + interrupt-names: > > + const: int_global > > 'int_' is redundant, drop. > Okay > > + > > + perst-gpios: > > + description: PCIe endpoint reset GPIO > > An input, right? > Yes, will mention. > > + maxItems: 1 > > + > > + wake-gpios: > > + description: PCIe endpoint wake GPIO > > + maxItems: 1 > > + > > + resets: > > + maxItems: 1 > > + > > + reset-names: > > + const: core_reset > > Not yet another name. We already have 'pci' and 'core' in the cases of > a single reset. > Okay > > + > > + power-domains: > > + maxItems: 1 > > + > > + phys: > > + maxItems: 1 > > + > > + phy-names: > > + const: pciephy > > + > > +required: > > + - compatible > > + - reg > > + - reg-names > > + - clocks > > + - clock-names > > + - interrupts > > + - interrupt-names > > + - perst-gpios > > + - resets > > + - reset-names > > + - power-domains > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/qcom,gcc-sdx55.h> > > + #include <dt-bindings/gpio/gpio.h> > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + pcie_ep: pcie-ep@40000000 { > > + compatible = "qcom,pcie-ep"; > > + > > + reg = <0x40000000 0xf1d>, > > + <0x40000f20 0xc8>, > > + <0x40001000 0x1000>, > > + <0x42000000 0x1000>, > > + <0x01c00000 0x3000>, > > + <0x01fcb000 0x1000>; > > + reg-names = "dbi", "elbi", "atu", "addr_space", "parf", "tcsr"; > > + > > + clocks = <&gcc GCC_PCIE_CFG_AHB_CLK>, > > + <&gcc GCC_PCIE_AUX_CLK>, > > + <&gcc GCC_PCIE_MSTR_AXI_CLK>, > > + <&gcc GCC_PCIE_SLV_AXI_CLK>, > > + <&gcc GCC_PCIE_0_CLKREF_CLK>, > > + <&gcc GCC_PCIE_SLEEP_CLK>, > > + <&gcc GCC_PCIE_SLV_Q2A_AXI_CLK>; > > + clock-names = "cfg", "aux", "bus_master", "bus_slave", > > + "ref", "sleep", "slave_q2a"; > > + > > + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-names = "int_global"; > > + perst-gpios = <&tlmm 57 GPIO_ACTIVE_HIGH>; > > + wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>; > > + resets = <&gcc GCC_PCIE_BCR>; > > + reset-names = "core_reset"; > > + power-domains = <&gcc PCIE_GDSC>; > > + phys = <&pcie0_lane>; > > + phy-names = "pciephy"; > > + max-link-speed = <3>; > > + num-lanes = <2>; > > Should be documented. I'd assume the max is less than 16 which is > presumably what pcie-ep.yaml allows. > okay Thanks, Mani > > + }; > > -- > > 2.25.1 > >
Hello, This series adds support for Qualcomm PCIe Endpoint controller found in platforms like SDX55. The Endpoint controller is based on the designware core with additional Qualcomm wrappers around the core. The driver is added separately unlike other Designware based drivers that combine RC and EP in a single driver. This is done to avoid complexity and to maintain this driver autonomously. The driver has been validated with an out of tree MHI function driver on SDX55 based Telit FN980 EVB connected to x86 host machine over PCIe. Thanks, Mani Manivannan Sadhasivam (3): dt-bindings: pci: Add devicetree binding for Qualcomm PCIe EP controller PCI: dwc: Add Qualcomm PCIe Endpoint controller driver MAINTAINERS: Add entry for Qualcomm PCIe Endpoint driver and binding .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 139 +++ MAINTAINERS | 10 +- drivers/pci/controller/dwc/Kconfig | 10 + drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-qcom-ep.c | 788 ++++++++++++++++++ 5 files changed, 947 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml create mode 100644 drivers/pci/controller/dwc/pcie-qcom-ep.c -- 2.25.1