| Message ID | 20201027060011.25893-2-wan.ahmad.zainie.wan.mohamad@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: keembay: Add support for Intel Keem Bay | expand |
On Tue, 27 Oct 2020 14:00:10 +0800, Wan Ahmad Zainie wrote: > Document DT bindings for PCIe controller found on Intel Keem Bay SoC. > > Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> > --- > .../bindings/pci/intel,keembay-pcie-ep.yaml | 86 +++++++++++++ > .../bindings/pci/intel,keembay-pcie.yaml | 120 ++++++++++++++++++ > 2 files changed, 206 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > create mode 100644 Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml:14:7: [warning] wrong indentation: expected 4 but found 6 (indentation) ./Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml:17:7: [warning] wrong indentation: expected 4 but found 6 (indentation) dtschema/dtc warnings/errors: See https://patchwork.ozlabs.org/patch/1388284 The base for the patch is generally the last rc1. Any dependencies should be noted. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On Tue, Oct 27, 2020 at 02:00:10PM +0800, Wan Ahmad Zainie wrote: > Document DT bindings for PCIe controller found on Intel Keem Bay SoC. > > Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> > --- > .../bindings/pci/intel,keembay-pcie-ep.yaml | 86 +++++++++++++ > .../bindings/pci/intel,keembay-pcie.yaml | 120 ++++++++++++++++++ > 2 files changed, 206 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > create mode 100644 Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > diff --git a/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > new file mode 100644 > index 000000000000..11962c205744 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > @@ -0,0 +1,86 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/pci/intel,keembay-pcie-ep.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Intel Keem Bay PCIe EP controller > + > +maintainers: > + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> > + > +properties: > + compatible: > + const: intel,keembay-pcie-ep > + > + reg: > + items: > + - description: DesignWare PCIe registers > + - description: PCIe configuration space > + - description: Keem Bay specific registers > + > + reg-names: > + items: > + - const: dbi > + - const: addr_space > + - const: apb > + > + interrupts: > + items: > + - description: PCIe interrupt > + - description: PCIe event interrupt > + - description: PCIe error interrupt > + - description: PCIe memory access interrupt > + > + interrupt-names: > + items: > + - const: intr > + - const: ev_intr > + - const: err_intr > + - const: mem_access_intr '_intr' is redundant. Drop it. You'll need a better name for the first one though. > + > + num-ib-windows: > + description: Number of inbound address translation windows > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + num-ob-windows: > + description: Number of outbound address translation windows > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + num-lanes: > + description: Number of lanes to use. > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 1, 2, 4, 8 ] > + > +required: > + - compatible > + - reg > + - reg-names > + - interrupts > + - interrupt-names > + - num-ib-windows > + - num-ob-windows > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + - | > + pcie-ep@37000000 { > + compatible = "intel,keembay-pcie-ep"; > + reg = <0x37000000 0x00800000>, > + <0x36000000 0x01000000>, > + <0x37800000 0x00000200>; > + reg-names = "dbi", "addr_space", "apb"; > + interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 108 IRQ_TYPE_EDGE_RISING>, > + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "intr", "ev_intr", "err_intr", > + "mem_access_intr"; > + num-ib-windows = <4>; > + num-ob-windows = <4>; > + num-lanes = <2>; > + }; > diff --git a/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > new file mode 100644 > index 000000000000..49e5d3d35bd4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > @@ -0,0 +1,120 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/pci/intel,keembay-pcie.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Intel Keem Bay PCIe RC controller > + > +maintainers: > + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> > + > +allOf: > + - $ref: /schemas/pci/pci-bus.yaml# > + > +properties: > + compatible: > + const: intel,keembay-pcie > + > + device_type: > + const: pci > + > + "#address-cells": > + const: 3 > + > + "#size-cells": > + const: 2 Can drop these 3 as pci-bus.yaml defines them. > + > + ranges: > + maxItems: 1 > + > + reset-gpios: > + maxItems: 1 > + > + reg: > + items: > + - description: DesignWare PCIe registers > + - description: PCIe configuration space > + - description: Keem Bay specific registers > + > + reg-names: > + items: > + - const: dbi > + - const: config > + - const: apb > + > + clocks: > + items: > + - description: bus clock > + - description: auxiliary clock The EP doesn't have clocks? You should have roughly the same resources for RC and EP modes. > + > + clock-names: > + items: > + - const: master > + - const: aux > + > + interrupts: > + items: > + - description: PCIe interrupt > + - description: PCIe event interrupt > + - description: PCIe error interrupt > + > + interrupt-names: > + items: > + - const: intr > + - const: ev_intr > + - const: err_intr > + > + num-lanes: > + description: Number of lanes to use. > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 1, 2, 4, 8 ] > + > + num-viewport: > + description: Number of view ports configured in hardware. > + $ref: /schemas/types.yaml#/definitions/uint32 > + default: 2 Pretty sure it's not 2 if num-ib-windows and num-ob-windows are 4. > + > +required: > + - compatible > + - device_type > + - "#address-cells" > + - "#size-cells" Can drop these too. > + - reg > + - reg-names > + - ranges > + - clocks > + - interrupts > + - interrupt-names > + - reset-gpios > + > +additionalProperties: false Use 'unevaluatedProperties: false' instead. > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/gpio/gpio.h> > + #define KEEM_BAY_A53_PCIE > + #define KEEM_BAY_A53_AUX_PCIE > + pcie@37000000 { > + compatible = "intel,keembay-pcie"; > + reg = <0x37000000 0x00800000>, > + <0x36e00000 0x00200000>, > + <0x37800000 0x00000200>; > + reg-names = "dbi", "config", "apb"; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges = <0x02000000 0 0x36000000 0x36000000 0 0x00e00000>; > + interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "intr", "ev_intr", "err_intr"; > + clocks = <&scmi_clk KEEM_BAY_A53_PCIE>, > + <&scmi_clk KEEM_BAY_A53_AUX_PCIE>; > + clock-names = "master", "aux"; > + reset-gpios = <&pca2 9 GPIO_ACTIVE_LOW>; > + num-viewport = <4>; > + num-lanes = <2>; > + }; > -- > 2.17.1 >
Hi Rob. Thanks for the review. > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Wednesday, October 28, 2020 10:42 PM > To: Wan Mohamad, Wan Ahmad Zainie > <wan.ahmad.zainie.wan.mohamad@intel.com> > Cc: bhelgaas@google.com; lorenzo.pieralisi@arm.com; linux- > pci@vger.kernel.org; devicetree@vger.kernel.org; > andriy.shevchenko@linux.intel.com; mgross@linux.intel.com; Raja > Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com> > Subject: Re: [PATCH 1/2] dt-bindings: PCI: Add Intel Keem Bay PCIe controller > > On Tue, Oct 27, 2020 at 02:00:10PM +0800, Wan Ahmad Zainie wrote: > > Document DT bindings for PCIe controller found on Intel Keem Bay SoC. > > > > Signed-off-by: Wan Ahmad Zainie > > <wan.ahmad.zainie.wan.mohamad@intel.com> > > --- > > .../bindings/pci/intel,keembay-pcie-ep.yaml | 86 +++++++++++++ > > .../bindings/pci/intel,keembay-pcie.yaml | 120 ++++++++++++++++++ > > 2 files changed, 206 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > > create mode 100644 > > Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > > b/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > > new file mode 100644 > > index 000000000000..11962c205744 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie- > ep.yaml > > @@ -0,0 +1,86 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/pci/intel,keembay-pcie-ep.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > + > > +title: Intel Keem Bay PCIe EP controller > > + > > +maintainers: > > + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> > > + > > +properties: > > + compatible: > > + const: intel,keembay-pcie-ep Fixed in v2, wrong indentation as per report. > > + > > + reg: > > + items: > > + - description: DesignWare PCIe registers > > + - description: PCIe configuration space > > + - description: Keem Bay specific registers > > + > > + reg-names: > > + items: > > + - const: dbi > > + - const: addr_space > > + - const: apb > > + > > + interrupts: > > + items: > > + - description: PCIe interrupt > > + - description: PCIe event interrupt > > + - description: PCIe error interrupt > > + - description: PCIe memory access interrupt > > + > > + interrupt-names: > > + items: > > + - const: intr > > + - const: ev_intr > > + - const: err_intr > > + - const: mem_access_intr > > '_intr' is redundant. Drop it. You'll need a better name for the first one > though. I will drop _intr in v2. I will send out once I get suitable name from Keem Bay data book. > > > + > > + num-ib-windows: > > + description: Number of inbound address translation windows > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + > > + num-ob-windows: > > + description: Number of outbound address translation windows > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + > > + num-lanes: > > + description: Number of lanes to use. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [ 1, 2, 4, 8 ] > > + > > +required: > > + - compatible > > + - reg > > + - reg-names > > + - interrupts > > + - interrupt-names > > + - num-ib-windows > > + - num-ob-windows > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/interrupt-controller/irq.h> > > + - | > > + pcie-ep@37000000 { > > + compatible = "intel,keembay-pcie-ep"; > > + reg = <0x37000000 0x00800000>, > > + <0x36000000 0x01000000>, > > + <0x37800000 0x00000200>; > > + reg-names = "dbi", "addr_space", "apb"; > > + interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 108 IRQ_TYPE_EDGE_RISING>, > > + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-names = "intr", "ev_intr", "err_intr", > > + "mem_access_intr"; > > + num-ib-windows = <4>; > > + num-ob-windows = <4>; > > + num-lanes = <2>; > > + }; > > diff --git > > a/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > new file mode 100644 > > index 000000000000..49e5d3d35bd4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > @@ -0,0 +1,120 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/pci/intel,keembay-pcie.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > + > > +title: Intel Keem Bay PCIe RC controller > > + > > +maintainers: > > + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> > > + > > +allOf: > > + - $ref: /schemas/pci/pci-bus.yaml# > > + > > +properties: > > + compatible: > > + const: intel,keembay-pcie Wrong indentation as per report. I will fix in v2. > > > + > > + device_type: > > + const: pci > > + > > + "#address-cells": > > + const: 3 > > + > > + "#size-cells": > > + const: 2 > > Can drop these 3 as pci-bus.yaml defines them. I will drop these 3 in v2. > > > + > > + ranges: > > + maxItems: 1 > > + > > + reset-gpios: > > + maxItems: 1 > > + > > + reg: > > + items: > > + - description: DesignWare PCIe registers > > + - description: PCIe configuration space > > + - description: Keem Bay specific registers > > + > > + reg-names: > > + items: > > + - const: dbi > > + - const: config > > + - const: apb > > + > > + clocks: > > + items: > > + - description: bus clock > > + - description: auxiliary clock > > The EP doesn't have clocks? You should have roughly the same resources for > RC and EP modes. For Keem Bay, EP mode link initialization is done in boot firmware. This include setup the clocks. That's why I do not include clocks for EP. > > > + > > + clock-names: > > + items: > > + - const: master > > + - const: aux > > + > > + interrupts: > > + items: > > + - description: PCIe interrupt > > + - description: PCIe event interrupt > > + - description: PCIe error interrupt > > + > > + interrupt-names: > > + items: > > + - const: intr > > + - const: ev_intr > > + - const: err_intr > > + > > + num-lanes: > > + description: Number of lanes to use. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [ 1, 2, 4, 8 ] > > + > > + num-viewport: > > + description: Number of view ports configured in hardware. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + default: 2 > > Pretty sure it's not 2 if num-ib-windows and num-ob-windows are 4. As per pcie-designware-host.c, default value is 2, if it is not set. My example and the DT in my system is 4. I will fix in v2, by using const: 4. Should I drop default? > > > + > > +required: > > + - compatible > > > + - device_type > > + - "#address-cells" > > + - "#size-cells" > > Can drop these too. I will drop them in v2. > > > + - reg > > + - reg-names > > + - ranges > > + - clocks > > + - interrupts > > + - interrupt-names > > + - reset-gpios > > + > > +additionalProperties: false > > Use 'unevaluatedProperties: false' instead. I will fix in v2. > > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/interrupt-controller/irq.h> > > + #include <dt-bindings/gpio/gpio.h> > > + #define KEEM_BAY_A53_PCIE > > + #define KEEM_BAY_A53_AUX_PCIE > > + pcie@37000000 { > > + compatible = "intel,keembay-pcie"; > > + reg = <0x37000000 0x00800000>, > > + <0x36e00000 0x00200000>, > > + <0x37800000 0x00000200>; > > + reg-names = "dbi", "config", "apb"; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + device_type = "pci"; > > + ranges = <0x02000000 0 0x36000000 0x36000000 0 0x00e00000>; > > + interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-names = "intr", "ev_intr", "err_intr"; > > + clocks = <&scmi_clk KEEM_BAY_A53_PCIE>, > > + <&scmi_clk KEEM_BAY_A53_AUX_PCIE>; > > + clock-names = "master", "aux"; > > + reset-gpios = <&pca2 9 GPIO_ACTIVE_LOW>; > > + num-viewport = <4>; > > + num-lanes = <2>; > > + }; > > -- > > 2.17.1 > > Best regards, Zainie
On Fri, Oct 30, 2020 at 8:05 AM Wan Mohamad, Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> wrote: > > Hi Rob. > > Thanks for the review. > > > -----Original Message----- > > From: Rob Herring <robh@kernel.org> > > Sent: Wednesday, October 28, 2020 10:42 PM > > To: Wan Mohamad, Wan Ahmad Zainie > > <wan.ahmad.zainie.wan.mohamad@intel.com> > > Cc: bhelgaas@google.com; lorenzo.pieralisi@arm.com; linux- > > pci@vger.kernel.org; devicetree@vger.kernel.org; > > andriy.shevchenko@linux.intel.com; mgross@linux.intel.com; Raja > > Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com> > > Subject: Re: [PATCH 1/2] dt-bindings: PCI: Add Intel Keem Bay PCIe controller > > > > On Tue, Oct 27, 2020 at 02:00:10PM +0800, Wan Ahmad Zainie wrote: > > > Document DT bindings for PCIe controller found on Intel Keem Bay SoC. > > > > > > Signed-off-by: Wan Ahmad Zainie > > > <wan.ahmad.zainie.wan.mohamad@intel.com> > > > --- > > > .../bindings/pci/intel,keembay-pcie-ep.yaml | 86 +++++++++++++ > > > .../bindings/pci/intel,keembay-pcie.yaml | 120 ++++++++++++++++++ > > > 2 files changed, 206 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > > > create mode 100644 > > > Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > > > b/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml > > > new file mode 100644 > > > index 000000000000..11962c205744 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie- > > ep.yaml > > > @@ -0,0 +1,86 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > > +--- > > > +$id: "http://devicetree.org/schemas/pci/intel,keembay-pcie-ep.yaml#" > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > > + > > > +title: Intel Keem Bay PCIe EP controller > > > + > > > +maintainers: > > > + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> > > > + > > > +properties: > > > + compatible: > > > + const: intel,keembay-pcie-ep > > Fixed in v2, wrong indentation as per report. > > > > + > > > + reg: > > > + items: > > > + - description: DesignWare PCIe registers > > > + - description: PCIe configuration space > > > + - description: Keem Bay specific registers > > > + > > > + reg-names: > > > + items: > > > + - const: dbi > > > + - const: addr_space > > > + - const: apb > > > + > > > + interrupts: > > > + items: > > > + - description: PCIe interrupt > > > + - description: PCIe event interrupt > > > + - description: PCIe error interrupt > > > + - description: PCIe memory access interrupt > > > + > > > + interrupt-names: > > > + items: > > > + - const: intr > > > + - const: ev_intr > > > + - const: err_intr > > > + - const: mem_access_intr > > > > '_intr' is redundant. Drop it. You'll need a better name for the first one > > though. > > I will drop _intr in v2. > I will send out once I get suitable name from Keem Bay data book. > > > > > > + > > > + num-ib-windows: > > > + description: Number of inbound address translation windows > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + > > > + num-ob-windows: > > > + description: Number of outbound address translation windows > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + > > > + num-lanes: > > > + description: Number of lanes to use. > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + enum: [ 1, 2, 4, 8 ] > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - reg-names > > > + - interrupts > > > + - interrupt-names > > > + - num-ib-windows > > > + - num-ob-windows > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > > + #include <dt-bindings/interrupt-controller/irq.h> > > > + - | > > > + pcie-ep@37000000 { > > > + compatible = "intel,keembay-pcie-ep"; > > > + reg = <0x37000000 0x00800000>, > > > + <0x36000000 0x01000000>, > > > + <0x37800000 0x00000200>; > > > + reg-names = "dbi", "addr_space", "apb"; > > > + interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>, > > > + <GIC_SPI 108 IRQ_TYPE_EDGE_RISING>, > > > + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>, > > > + <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; > > > + interrupt-names = "intr", "ev_intr", "err_intr", > > > + "mem_access_intr"; > > > + num-ib-windows = <4>; > > > + num-ob-windows = <4>; > > > + num-lanes = <2>; > > > + }; > > > diff --git > > > a/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > > b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > > new file mode 100644 > > > index 000000000000..49e5d3d35bd4 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml > > > @@ -0,0 +1,120 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > > +--- > > > +$id: "http://devicetree.org/schemas/pci/intel,keembay-pcie.yaml#" > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > > + > > > +title: Intel Keem Bay PCIe RC controller > > > + > > > +maintainers: > > > + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> > > > + > > > +allOf: > > > + - $ref: /schemas/pci/pci-bus.yaml# > > > + > > > +properties: > > > + compatible: > > > + const: intel,keembay-pcie > > Wrong indentation as per report. > I will fix in v2. > > > > > > + > > > + device_type: > > > + const: pci > > > + > > > + "#address-cells": > > > + const: 3 > > > + > > > + "#size-cells": > > > + const: 2 > > > > Can drop these 3 as pci-bus.yaml defines them. > > I will drop these 3 in v2. > > > > > > + > > > + ranges: > > > + maxItems: 1 > > > + > > > + reset-gpios: > > > + maxItems: 1 > > > + > > > + reg: > > > + items: > > > + - description: DesignWare PCIe registers > > > + - description: PCIe configuration space > > > + - description: Keem Bay specific registers > > > + > > > + reg-names: > > > + items: > > > + - const: dbi > > > + - const: config > > > + - const: apb > > > + > > > + clocks: > > > + items: > > > + - description: bus clock > > > + - description: auxiliary clock > > > > The EP doesn't have clocks? You should have roughly the same resources for > > RC and EP modes. > > For Keem Bay, EP mode link initialization is done in boot firmware. > This include setup the clocks. > That's why I do not include clocks for EP. > > > > > > + > > > + clock-names: > > > + items: > > > + - const: master > > > + - const: aux > > > + > > > + interrupts: > > > + items: > > > + - description: PCIe interrupt > > > + - description: PCIe event interrupt > > > + - description: PCIe error interrupt > > > + > > > + interrupt-names: > > > + items: > > > + - const: intr > > > + - const: ev_intr > > > + - const: err_intr > > > + > > > + num-lanes: > > > + description: Number of lanes to use. > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + enum: [ 1, 2, 4, 8 ] > > > + > > > + num-viewport: > > > + description: Number of view ports configured in hardware. > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + default: 2 > > > > Pretty sure it's not 2 if num-ib-windows and num-ob-windows are 4. > > As per pcie-designware-host.c, default value is 2, if it is not set. Yes, that's true. > My example and the DT in my system is 4. > I will fix in v2, by using const: 4. > Should I drop default? Yes. BTW, I'm going to make all 3 properties obsolete. I'm working on a patch to detect all this. It's pretty straight-forward, just see how many registers are writable. The WIP patch is on my for-kernelci branch. The problem with these properties is they are defined as RC and EP specific, but they are really fixed h/w config independent of the mode. And num-viewport is incomplete because the inbound and outbound sizes are independent. The driver just currently doesn't use inbound windows for RC mode. Also, the driver claims there can be up to 256 windows, but I'm not really sure that's right. There's 2 platforms upstream (ls1088a and ls208xa) claiming 256 windows in DT, but testing with the detection code indicates they only have 16 IB and 16 OB windows. Perhaps if you have the DWC manual you could confirm what's possible. Rob
Hi Rob. > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Friday, October 30, 2020 10:56 PM > To: Wan Mohamad, Wan Ahmad Zainie > <wan.ahmad.zainie.wan.mohamad@intel.com> > Cc: bhelgaas@google.com; lorenzo.pieralisi@arm.com; linux- > pci@vger.kernel.org; devicetree@vger.kernel.org; > andriy.shevchenko@linux.intel.com; mgross@linux.intel.com; Raja > Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com> > Subject: Re: [PATCH 1/2] dt-bindings: PCI: Add Intel Keem Bay PCIe controller > > On Fri, Oct 30, 2020 at 8:05 AM Wan Mohamad, Wan Ahmad Zainie > <wan.ahmad.zainie.wan.mohamad@intel.com> wrote: > > > > Hi Rob. > > > > Thanks for the review. > > > > > -----Original Message----- > > > From: Rob Herring <robh@kernel.org> > > > Sent: Wednesday, October 28, 2020 10:42 PM > > > To: Wan Mohamad, Wan Ahmad Zainie > > > <wan.ahmad.zainie.wan.mohamad@intel.com> > > > Cc: bhelgaas@google.com; lorenzo.pieralisi@arm.com; linux- > > > pci@vger.kernel.org; devicetree@vger.kernel.org; > > > andriy.shevchenko@linux.intel.com; mgross@linux.intel.com; Raja > > > Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com> > > > Subject: Re: [PATCH 1/2] dt-bindings: PCI: Add Intel Keem Bay PCIe > > > controller > > > > > > On Tue, Oct 27, 2020 at 02:00:10PM +0800, Wan Ahmad Zainie wrote: ... > > > > + num-viewport: > > > > + description: Number of view ports configured in hardware. > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > + default: 2 > > > > > > Pretty sure it's not 2 if num-ib-windows and num-ob-windows are 4. > > > > As per pcie-designware-host.c, default value is 2, if it is not set. > > Yes, that's true. > > > My example and the DT in my system is 4. > > I will fix in v2, by using const: 4. > > Should I drop default? > > Yes. > > BTW, I'm going to make all 3 properties obsolete. I'm working on a patch to > detect all this. It's pretty straight-forward, just see how many registers are > writable. The WIP patch is on my for-kernelci branch. I will take note on this. I also take a look into for-kernelci branch. I will spend some time to try it out with my patch. > > The problem with these properties is they are defined as RC and EP specific, > but they are really fixed h/w config independent of the mode. And num- > viewport is incomplete because the inbound and outbound sizes are > independent. The driver just currently doesn't use inbound windows for RC > mode. Also, the driver claims there can be up to 256 windows, but I'm not > really sure that's right. There's 2 platforms upstream (ls1088a and ls208xa) > claiming 256 windows in DT, but testing with the detection code indicates > they only have 16 IB and 16 OB windows. Perhaps if you have the DWC > manual you could confirm what's possible. Unfortunately, I don't have details on DWC manual. As for Keem Bay, from the information in its databook, it is synthesized with 8 IB and 8 OB windows. The values that I used for DT is based on recommendation from our boot firmware team. > > Rob Best regards, Zainie
diff --git a/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml new file mode 100644 index 000000000000..11962c205744 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml @@ -0,0 +1,86 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/pci/intel,keembay-pcie-ep.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Intel Keem Bay PCIe EP controller + +maintainers: + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> + +properties: + compatible: + const: intel,keembay-pcie-ep + + reg: + items: + - description: DesignWare PCIe registers + - description: PCIe configuration space + - description: Keem Bay specific registers + + reg-names: + items: + - const: dbi + - const: addr_space + - const: apb + + interrupts: + items: + - description: PCIe interrupt + - description: PCIe event interrupt + - description: PCIe error interrupt + - description: PCIe memory access interrupt + + interrupt-names: + items: + - const: intr + - const: ev_intr + - const: err_intr + - const: mem_access_intr + + num-ib-windows: + description: Number of inbound address translation windows + $ref: /schemas/types.yaml#/definitions/uint32 + + num-ob-windows: + description: Number of outbound address translation windows + $ref: /schemas/types.yaml#/definitions/uint32 + + num-lanes: + description: Number of lanes to use. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 1, 2, 4, 8 ] + +required: + - compatible + - reg + - reg-names + - interrupts + - interrupt-names + - num-ib-windows + - num-ob-windows + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + - | + pcie-ep@37000000 { + compatible = "intel,keembay-pcie-ep"; + reg = <0x37000000 0x00800000>, + <0x36000000 0x01000000>, + <0x37800000 0x00000200>; + reg-names = "dbi", "addr_space", "apb"; + interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 108 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "intr", "ev_intr", "err_intr", + "mem_access_intr"; + num-ib-windows = <4>; + num-ob-windows = <4>; + num-lanes = <2>; + }; diff --git a/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml new file mode 100644 index 000000000000..49e5d3d35bd4 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml @@ -0,0 +1,120 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/pci/intel,keembay-pcie.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Intel Keem Bay PCIe RC controller + +maintainers: + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +properties: + compatible: + const: intel,keembay-pcie + + device_type: + const: pci + + "#address-cells": + const: 3 + + "#size-cells": + const: 2 + + ranges: + maxItems: 1 + + reset-gpios: + maxItems: 1 + + reg: + items: + - description: DesignWare PCIe registers + - description: PCIe configuration space + - description: Keem Bay specific registers + + reg-names: + items: + - const: dbi + - const: config + - const: apb + + clocks: + items: + - description: bus clock + - description: auxiliary clock + + clock-names: + items: + - const: master + - const: aux + + interrupts: + items: + - description: PCIe interrupt + - description: PCIe event interrupt + - description: PCIe error interrupt + + interrupt-names: + items: + - const: intr + - const: ev_intr + - const: err_intr + + num-lanes: + description: Number of lanes to use. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 1, 2, 4, 8 ] + + num-viewport: + description: Number of view ports configured in hardware. + $ref: /schemas/types.yaml#/definitions/uint32 + default: 2 + +required: + - compatible + - device_type + - "#address-cells" + - "#size-cells" + - reg + - reg-names + - ranges + - clocks + - interrupts + - interrupt-names + - reset-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/gpio/gpio.h> + #define KEEM_BAY_A53_PCIE + #define KEEM_BAY_A53_AUX_PCIE + pcie@37000000 { + compatible = "intel,keembay-pcie"; + reg = <0x37000000 0x00800000>, + <0x36e00000 0x00200000>, + <0x37800000 0x00000200>; + reg-names = "dbi", "config", "apb"; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges = <0x02000000 0 0x36000000 0x36000000 0 0x00e00000>; + interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "intr", "ev_intr", "err_intr"; + clocks = <&scmi_clk KEEM_BAY_A53_PCIE>, + <&scmi_clk KEEM_BAY_A53_AUX_PCIE>; + clock-names = "master", "aux"; + reset-gpios = <&pca2 9 GPIO_ACTIVE_LOW>; + num-viewport = <4>; + num-lanes = <2>; + };
Document DT bindings for PCIe controller found on Intel Keem Bay SoC. Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> --- .../bindings/pci/intel,keembay-pcie-ep.yaml | 86 +++++++++++++ .../bindings/pci/intel,keembay-pcie.yaml | 120 ++++++++++++++++++ 2 files changed, 206 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/intel,keembay-pcie-ep.yaml create mode 100644 Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml