Hi Rob,
After our discussions, I'm opting to submit first the DT bindings for the
Kirin 970 PCIe support.
Patch 1 is there just because patch 2 needs. You already acked on it.
Patch 5 is also there just as an example of the entire stuff added to
the DTS file.
The core of this series are patches 2 to 4. They contain the conversion
of the kirin-pcie.txt file to the DT schema, and adds the needed
bindings.
Currently, it generates some warnings:
Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml: pcie@f5000000: pcie@4,0:compatible: None of ['pciclass,0604'] are valid under the given schema
From schema: Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml: pcie@4,0: reset-gpios: [[4294967295, 1, 0]] is too short
Not sure how/where to fix those. Perhaps at the pci-bus.yaml?
Please review.
Thanks!
Mauro
Manivannan Sadhasivam (1):
arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller
hardware
Mauro Carvalho Chehab (4):
dt-bindings: PCI: kirin: Fix compatible string
dt-bindings: PCI: kirin: convert kirin-pcie.txt to yaml
dt-bindings: PCI: kirin: Add support for Kirin970
dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
.../bindings/pci/hisilicon,kirin-pcie.yaml | 145 ++++++++++++++++++
.../devicetree/bindings/pci/kirin-pcie.txt | 50 ------
.../devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +-
.../phy/hisilicon,phy-hi3670-pcie.yaml | 86 +++++++++++
MAINTAINERS | 2 +-
arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 99 ++++++++++++
6 files changed, 332 insertions(+), 52 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt
create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
--
2.31.1
From: Manivannan Sadhasivam <[email protected]>
Add DTS bindings for the HiKey 970 board's PCIe hardware.
Co-developed-by: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 99 +++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
index 20698cfd0637..2cf19c8960f3 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
@@ -176,6 +176,12 @@ sctrl: sctrl@fff0a000 {
#clock-cells = <1>;
};
+ pmctrl: pmctrl@fff31000 {
+ compatible = "hisilicon,hi3670-pmctrl", "syscon";
+ reg = <0x0 0xfff31000 0x0 0x1000>;
+ #clock-cells = <1>;
+ };
+
iomcu: iomcu@ffd7e000 {
compatible = "hisilicon,hi3670-iomcu", "syscon";
reg = <0x0 0xffd7e000 0x0 0x1000>;
@@ -659,6 +665,99 @@ gpio28: gpio@fff1d000 {
clock-names = "apb_pclk";
};
+ its_pcie: interrupt-controller@f4000000 {
+ compatible = "arm,gic-v3-its";
+ msi-controller;
+ reg = <0x0 0xf5100000 0x0 0x100000>;
+ };
+
+ pcie_phy: pcie-phy@fc000000 {
+ compatible = "hisilicon,hi970-pcie-phy";
+ reg = <0x0 0xfc000000 0x0 0x80000>;
+
+ phy-supply = <&ldo33>;
+
+ clocks = <&crg_ctrl HI3670_CLK_GATE_PCIEPHY_REF>,
+ <&crg_ctrl HI3670_CLK_GATE_PCIEAUX>,
+ <&crg_ctrl HI3670_PCLK_GATE_PCIE_PHY>,
+ <&crg_ctrl HI3670_PCLK_GATE_PCIE_SYS>,
+ <&crg_ctrl HI3670_ACLK_GATE_PCIE>;
+ clock-names = "phy_ref", "aux",
+ "apb_phy", "apb_sys",
+ "aclk";
+
+ /* vboost iboost pre post main */
+ hisilicon,eye-diagram-param = <0xffffffff 0xffffffff
+ 0xffffffff 0xffffffff
+ 0xffffffff>;
+
+ #phy-cells = <0>;
+ };
+
+ pcie@f4000000 {
+ compatible = "hisilicon,kirin970-pcie";
+ reg = <0x0 0xf4000000 0x0 0x1000000>,
+ <0x0 0xfc180000 0x0 0x1000>,
+ <0x0 0xf5000000 0x0 0x2000>;
+ reg-names = "dbi", "apb", "config";
+ bus-range = <0x0 0x1>;
+ msi-parent = <&its_pcie>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ phys = <&pcie_phy>;
+ ranges = <0x02000000 0x0 0x00000000
+ 0x0 0xf6000000
+ 0x0 0x02000000>;
+ num-lanes = <1>;
+ #interrupt-cells = <1>;
+ interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi";
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0x0 0 0 1
+ &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
+ <0x0 0 0 2
+ &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+ <0x0 0 0 3
+ &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
+ <0x0 0 0 4
+ &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
+ reset-gpios = <&gpio7 0 0 >;
+
+ pcie@4,0 { // Lane 4: M.2
+ reg = <0 0 0 0 0>;
+ compatible = "pciclass,0604";
+ device_type = "pci";
+ reset-gpios = <&gpio7 1 0>;
+ clkreq-gpios = <&gpio27 3 0 >;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+ };
+
+ pcie@5,0 { // Lane 5: Mini PCIe
+ reg = <0 0 0 0 0>;
+ compatible = "pciclass,0604";
+ device_type = "pci";
+ reset-gpios = <&gpio7 2 0>;
+ clkreq-gpios = <&gpio17 0 0 >;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+ };
+
+ pcie@7,0 { // Lane 7: Ethernet
+ reg = <0 0 0 0 0>;
+ compatible = "pciclass,0604";
+ device_type = "pci";
+ reset-gpios = <&gpio7 3 0>;
+ clkreq-gpios = <&gpio20 0 0 >;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+ };
+ };
+
/* UFS */
ufs: ufs@ff3c0000 {
compatible = "hisilicon,hi3670-ufs", "jedec,ufs-2.1";
--
2.31.1
Add a new compatible, plus the new bindings needed by
HiKey970 board.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
.../bindings/pci/hisilicon,kirin-pcie.yaml | 61 ++++++++++++++++++-
1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
index 90cab09e8d4b..bb0c3a081d68 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
@@ -24,11 +24,13 @@ properties:
contains:
enum:
- hisilicon,kirin960-pcie
+ - hisilicon,kirin970-pcie
reg:
description: |
Should contain dbi, apb, config registers location and length.
- For HiKey960, it should also contain phy.
+ For HiKey960, it should also contain phy. All other devices
+ should use a separate phy driver.
minItems: 3
maxItems: 4
@@ -47,6 +49,7 @@ examples:
- |
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/clock/hi3660-clock.h>
+ #include <dt-bindings/clock/hi3670-clock.h>
soc {
#address-cells = <2>;
@@ -83,4 +86,60 @@ examples:
clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
"pcie_apb_sys", "pcie_aclk";
};
+
+ pcie@f5000000 {
+ compatible = "hisilicon,kirin970-pcie";
+ reg = <0x0 0xf4000000 0x0 0x1000000>,
+ <0x0 0xfc180000 0x0 0x1000>,
+ <0x0 0xf5000000 0x0 0x2000>;
+ reg-names = "dbi", "apb", "config";
+ bus-range = <0x0 0x1>;
+ msi-parent = <&its_pcie>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ phys = <&pcie_phy>;
+ ranges = <0x02000000 0x0 0x00000000
+ 0x0 0xf6000000
+ 0x0 0x02000000>;
+ num-lanes = <1>;
+ #interrupt-cells = <1>;
+ interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi";
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
+ <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+ <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
+ <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
+ pcie@4,0 { // Lane 4: M.2
+ reg = <0 0 0 0 0>;
+ compatible = "pciclass,0604";
+ device_type = "pci";
+ reset-gpios = <&gpio7 1 0>;
+ clkreq-gpios = <&gpio27 3 0 >;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+ };
+ pcie@5,0 { // Lane 5: Mini PCIe
+ reg = <0 0 0 0 0>;
+ compatible = "pciclass,0604";
+ device_type = "pci";
+ reset-gpios = <&gpio7 2 0>;
+ clkreq-gpios = <&gpio17 0 0 >;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+ };
+ pcie@7,0 { // Lane 7: Ethernet
+ reg = <0 0 0 0 0>;
+ compatible = "pciclass,0604";
+ device_type = "pci";
+ reset-gpios = <&gpio7 3 0>;
+ clkreq-gpios = <&gpio20 0 0 >;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+ };
+ };
};
--
2.31.1
On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
<[email protected]> wrote:
>
> Add a new compatible, plus the new bindings needed by
> HiKey970 board.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> .../bindings/pci/hisilicon,kirin-pcie.yaml | 61 ++++++++++++++++++-
> 1 file changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> index 90cab09e8d4b..bb0c3a081d68 100644
> --- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> @@ -24,11 +24,13 @@ properties:
> contains:
> enum:
> - hisilicon,kirin960-pcie
> + - hisilicon,kirin970-pcie
>
> reg:
> description: |
> Should contain dbi, apb, config registers location and length.
> - For HiKey960, it should also contain phy.
> + For HiKey960, it should also contain phy. All other devices
> + should use a separate phy driver.
> minItems: 3
> maxItems: 4
>
> @@ -47,6 +49,7 @@ examples:
> - |
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/clock/hi3660-clock.h>
> + #include <dt-bindings/clock/hi3670-clock.h>
>
> soc {
> #address-cells = <2>;
> @@ -83,4 +86,60 @@ examples:
> clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
> "pcie_apb_sys", "pcie_aclk";
> };
> +
> + pcie@f5000000 {
> + compatible = "hisilicon,kirin970-pcie";
> + reg = <0x0 0xf4000000 0x0 0x1000000>,
> + <0x0 0xfc180000 0x0 0x1000>,
> + <0x0 0xf5000000 0x0 0x2000>;
> + reg-names = "dbi", "apb", "config";
> + bus-range = <0x0 0x1>;
> + msi-parent = <&its_pcie>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + phys = <&pcie_phy>;
> + ranges = <0x02000000 0x0 0x00000000
> + 0x0 0xf6000000
> + 0x0 0x02000000>;
> + num-lanes = <1>;
> + #interrupt-cells = <1>;
> + interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi";
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> + <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> + <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> + <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> + pcie@4,0 { // Lane 4: M.2
You are missing a level here. You need the upstream bridge device. I
figured this out for you, why am I having to correct it?
Isn't this supposed to be Device 1 as 1 and 4 are swapped in terms of
lane number and device number.
> + reg = <0 0 0 0 0>;
Not the right address. I would have expected a dtc warning on this.
> + compatible = "pciclass,0604";
> + device_type = "pci";
> + reset-gpios = <&gpio7 1 0>;
> + clkreq-gpios = <&gpio27 3 0 >;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges;
> + };
> + pcie@5,0 { // Lane 5: Mini PCIe
It's device 5 not lane 5.
> + reg = <0 0 0 0 0>;
> + compatible = "pciclass,0604";
> + device_type = "pci";
> + reset-gpios = <&gpio7 2 0>;
> + clkreq-gpios = <&gpio17 0 0 >;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges;
> + };
> + pcie@7,0 { // Lane 7: Ethernet
> + reg = <0 0 0 0 0>;
> + compatible = "pciclass,0604";
> + device_type = "pci";
> + reset-gpios = <&gpio7 3 0>;
> + clkreq-gpios = <&gpio20 0 0 >;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges;
> + };
> + };
> };
> --
> 2.31.1
>
On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
<[email protected]> wrote:
>
> Add a new compatible, plus the new bindings needed by
> HiKey970 board.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> .../bindings/pci/hisilicon,kirin-pcie.yaml | 61 ++++++++++++++++++-
> 1 file changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> index 90cab09e8d4b..bb0c3a081d68 100644
> --- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> @@ -24,11 +24,13 @@ properties:
> contains:
> enum:
> - hisilicon,kirin960-pcie
> + - hisilicon,kirin970-pcie
>
> reg:
> description: |
> Should contain dbi, apb, config registers location and length.
> - For HiKey960, it should also contain phy.
> + For HiKey960, it should also contain phy. All other devices
> + should use a separate phy driver.
> minItems: 3
> maxItems: 4
>
> @@ -47,6 +49,7 @@ examples:
> - |
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/clock/hi3660-clock.h>
> + #include <dt-bindings/clock/hi3670-clock.h>
>
> soc {
> #address-cells = <2>;
> @@ -83,4 +86,60 @@ examples:
> clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
> "pcie_apb_sys", "pcie_aclk";
> };
> +
> + pcie@f5000000 {
> + compatible = "hisilicon,kirin970-pcie";
> + reg = <0x0 0xf4000000 0x0 0x1000000>,
> + <0x0 0xfc180000 0x0 0x1000>,
> + <0x0 0xf5000000 0x0 0x2000>;
> + reg-names = "dbi", "apb", "config";
> + bus-range = <0x0 0x1>;
> + msi-parent = <&its_pcie>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + phys = <&pcie_phy>;
> + ranges = <0x02000000 0x0 0x00000000
> + 0x0 0xf6000000
> + 0x0 0x02000000>;
> + num-lanes = <1>;
> + #interrupt-cells = <1>;
> + interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi";
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> + <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> + <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> + <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> + pcie@4,0 { // Lane 4: M.2
> + reg = <0 0 0 0 0>;
> + compatible = "pciclass,0604";
> + device_type = "pci";
> + reset-gpios = <&gpio7 1 0>;
> + clkreq-gpios = <&gpio27 3 0 >;
Looking at the schematics some more, this is not right. CLKREQ# is an
input from the device, and they are not connected to any GPIO (just
pulled high) on hikey970. These GPIOs are simply clock enables and
very much specific to hikey. So I'd call this 'hisilicon,clken-gpios'
and you can just stick them in the host bridge node.
I think the way the board should have been designed is the CLKREQ#
signals to the clock driver chip OE signals. Then there'd be no s/w
control needed.
Rob
On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
<[email protected]> wrote:
>
> Hi Rob,
>
> After our discussions, I'm opting to submit first the DT bindings for the
> Kirin 970 PCIe support.
>
> Patch 1 is there just because patch 2 needs. You already acked on it.
>
> Patch 5 is also there just as an example of the entire stuff added to
> the DTS file.
>
> The core of this series are patches 2 to 4. They contain the conversion
> of the kirin-pcie.txt file to the DT schema, and adds the needed
> bindings.
>
> Currently, it generates some warnings:
>
> Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml: pcie@f5000000: pcie@4,0:compatible: None of ['pciclass,0604'] are valid under the given schema
> From schema: Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
This should be fixed now in dtschema master.
> Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml: pcie@4,0: reset-gpios: [[4294967295, 1, 0]] is too short
Your proposed change to pci-bus.yaml is wrong. It's requiring 4
entries. You need 'minItems: 1'.
Rob
Em Thu, 29 Jul 2021 09:20:15 -0600
Rob Herring <[email protected]> escreveu:
> On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
> <[email protected]> wrote:
> >
> > Add a new compatible, plus the new bindings needed by
> > HiKey970 board.
> >
> > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> > ---
> > .../bindings/pci/hisilicon,kirin-pcie.yaml | 61 ++++++++++++++++++-
> > 1 file changed, 60 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > index 90cab09e8d4b..bb0c3a081d68 100644
> > --- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > @@ -24,11 +24,13 @@ properties:
> > contains:
> > enum:
> > - hisilicon,kirin960-pcie
> > + - hisilicon,kirin970-pcie
> >
> > reg:
> > description: |
> > Should contain dbi, apb, config registers location and length.
> > - For HiKey960, it should also contain phy.
> > + For HiKey960, it should also contain phy. All other devices
> > + should use a separate phy driver.
> > minItems: 3
> > maxItems: 4
> >
> > @@ -47,6 +49,7 @@ examples:
> > - |
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > #include <dt-bindings/clock/hi3660-clock.h>
> > + #include <dt-bindings/clock/hi3670-clock.h>
> >
> > soc {
> > #address-cells = <2>;
> > @@ -83,4 +86,60 @@ examples:
> > clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
> > "pcie_apb_sys", "pcie_aclk";
> > };
> > +
> > + pcie@f5000000 {
> > + compatible = "hisilicon,kirin970-pcie";
> > + reg = <0x0 0xf4000000 0x0 0x1000000>,
> > + <0x0 0xfc180000 0x0 0x1000>,
> > + <0x0 0xf5000000 0x0 0x2000>;
> > + reg-names = "dbi", "apb", "config";
> > + bus-range = <0x0 0x1>;
> > + msi-parent = <&its_pcie>;
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + device_type = "pci";
> > + phys = <&pcie_phy>;
> > + ranges = <0x02000000 0x0 0x00000000
> > + 0x0 0xf6000000
> > + 0x0 0x02000000>;
> > + num-lanes = <1>;
> > + #interrupt-cells = <1>;
> > + interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-names = "msi";
> > + interrupt-map-mask = <0 0 0 7>;
> > + interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> > + <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> > + <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> > + <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> > + pcie@4,0 { // Lane 4: M.2
> > + reg = <0 0 0 0 0>;
> > + compatible = "pciclass,0604";
> > + device_type = "pci";
> > + reset-gpios = <&gpio7 1 0>;
> > + clkreq-gpios = <&gpio27 3 0 >;
>
> Looking at the schematics some more, this is not right. CLKREQ# is an
> input from the device, and they are not connected to any GPIO (just
> pulled high) on hikey970. These GPIOs are simply clock enables and
> very much specific to hikey. So I'd call this 'hisilicon,clken-gpios'
> and you can just stick them in the host bridge node.
>
Ok. If I understood your review, the schema will then be:
pcie@f4000000 {
compatible = "hisilicon,kirin970-pcie";
reg = <0x0 0xf4000000 0x0 0x1000000>,
<0x0 0xfc180000 0x0 0x1000>,
<0x0 0xf5000000 0x0 0x2000>;
reg-names = "dbi", "apb", "config";
bus-range = <0x0 0x1>;
msi-parent = <&its_pcie>;
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
phys = <&pcie_phy>;
ranges = <0x02000000 0x0 0x00000000
0x0 0xf6000000
0x0 0x02000000>;
num-lanes = <1>;
#interrupt-cells = <1>;
interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "msi";
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
<0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
<0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
<0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
reset-gpios = <&gpio7 0 0>;
pcie@0 { // Lane 0: upstream
reg = <0 0 0 0 0>;
compatible = "pciclass,0604";
device_type = "pci";
#address-cells = <3>;
#size-cells = <2>;
hisilicon,clken-gpios = <&gpio27 3 0 >, <&gpio17 0 0 >, <&gpio20 6 0 >;
ranges;
pcie@1,0 { // Lane 4: M.2
reg = <0x800 0 0 0 0>;
compatible = "pciclass,0604";
device_type = "pci";
reset-gpios = <&gpio3 1 0>;
#address-cells = <3>;
#size-cells = <2>;
ranges;
};
pcie@5,0 { // Lane 5: Mini PCIe
reg = <0x2800 0 0 0 0>;
compatible = "pciclass,0604";
device_type = "pci";
reset-gpios = <&gpio27 4 0 >;
#address-cells = <3>;
#size-cells = <2>;
ranges;
};
pcie@7,0 { // Lane 7: Ethernet
reg = <0x3800 0 0 0 0>;
compatible = "pciclass,0604";
device_type = "pci";
reset-gpios = <&gpio25 2 0 >;
#address-cells = <3>;
#size-cells = <2>;
ranges;
};
};
};
};
Right?
After updating the dt-schema from your git tree, the above doesn't
generate warnings anymore.
Thanks,
Mauro
On Thu, Jul 29, 2021 at 09:03:37PM +0200, Mauro Carvalho Chehab wrote:
> Em Thu, 29 Jul 2021 09:20:15 -0600
> Rob Herring <[email protected]> escreveu:
>
> > On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
> > <[email protected]> wrote:
> > >
> > > Add a new compatible, plus the new bindings needed by
> > > HiKey970 board.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> > > ---
> > > .../bindings/pci/hisilicon,kirin-pcie.yaml | 61 ++++++++++++++++++-
> > > 1 file changed, 60 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > > index 90cab09e8d4b..bb0c3a081d68 100644
> > > --- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > > @@ -24,11 +24,13 @@ properties:
> > > contains:
> > > enum:
> > > - hisilicon,kirin960-pcie
> > > + - hisilicon,kirin970-pcie
> > >
> > > reg:
> > > description: |
> > > Should contain dbi, apb, config registers location and length.
> > > - For HiKey960, it should also contain phy.
> > > + For HiKey960, it should also contain phy. All other devices
> > > + should use a separate phy driver.
> > > minItems: 3
> > > maxItems: 4
> > >
> > > @@ -47,6 +49,7 @@ examples:
> > > - |
> > > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > #include <dt-bindings/clock/hi3660-clock.h>
> > > + #include <dt-bindings/clock/hi3670-clock.h>
> > >
> > > soc {
> > > #address-cells = <2>;
> > > @@ -83,4 +86,60 @@ examples:
> > > clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
> > > "pcie_apb_sys", "pcie_aclk";
> > > };
> > > +
> > > + pcie@f5000000 {
> > > + compatible = "hisilicon,kirin970-pcie";
> > > + reg = <0x0 0xf4000000 0x0 0x1000000>,
> > > + <0x0 0xfc180000 0x0 0x1000>,
> > > + <0x0 0xf5000000 0x0 0x2000>;
> > > + reg-names = "dbi", "apb", "config";
> > > + bus-range = <0x0 0x1>;
> > > + msi-parent = <&its_pcie>;
> > > + #address-cells = <3>;
> > > + #size-cells = <2>;
> > > + device_type = "pci";
> > > + phys = <&pcie_phy>;
> > > + ranges = <0x02000000 0x0 0x00000000
> > > + 0x0 0xf6000000
> > > + 0x0 0x02000000>;
> > > + num-lanes = <1>;
> > > + #interrupt-cells = <1>;
> > > + interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> > > + interrupt-names = "msi";
> > > + interrupt-map-mask = <0 0 0 7>;
> > > + interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> > > + <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> > > + <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> > > + <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> > > + pcie@4,0 { // Lane 4: M.2
> > > + reg = <0 0 0 0 0>;
> > > + compatible = "pciclass,0604";
> > > + device_type = "pci";
> > > + reset-gpios = <&gpio7 1 0>;
> > > + clkreq-gpios = <&gpio27 3 0 >;
> >
> > Looking at the schematics some more, this is not right. CLKREQ# is an
> > input from the device, and they are not connected to any GPIO (just
> > pulled high) on hikey970. These GPIOs are simply clock enables and
> > very much specific to hikey. So I'd call this 'hisilicon,clken-gpios'
> > and you can just stick them in the host bridge node.
> >
>
> Ok. If I understood your review, the schema will then be:
>
> pcie@f4000000 {
> compatible = "hisilicon,kirin970-pcie";
> reg = <0x0 0xf4000000 0x0 0x1000000>,
> <0x0 0xfc180000 0x0 0x1000>,
> <0x0 0xf5000000 0x0 0x2000>;
> reg-names = "dbi", "apb", "config";
> bus-range = <0x0 0x1>;
> msi-parent = <&its_pcie>;
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> phys = <&pcie_phy>;
> ranges = <0x02000000 0x0 0x00000000
> 0x0 0xf6000000
> 0x0 0x02000000>;
> num-lanes = <1>;
> #interrupt-cells = <1>;
> interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-names = "msi";
> interrupt-map-mask = <0 0 0 7>;
> interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> reset-gpios = <&gpio7 0 0>;
>
> pcie@0 { // Lane 0: upstream
> reg = <0 0 0 0 0>;
> compatible = "pciclass,0604";
> device_type = "pci";
> #address-cells = <3>;
> #size-cells = <2>;
> hisilicon,clken-gpios = <&gpio27 3 0 >, <&gpio17 0 0 >, <&gpio20 6 0 >;
Up one more level.
> ranges;
>
> pcie@1,0 { // Lane 4: M.2
> reg = <0x800 0 0 0 0>;
> compatible = "pciclass,0604";
> device_type = "pci";
> reset-gpios = <&gpio3 1 0>;
> #address-cells = <3>;
> #size-cells = <2>;
> ranges;
> };
>
> pcie@5,0 { // Lane 5: Mini PCIe
> reg = <0x2800 0 0 0 0>;
> compatible = "pciclass,0604";
> device_type = "pci";
> reset-gpios = <&gpio27 4 0 >;
> #address-cells = <3>;
> #size-cells = <2>;
> ranges;
> };
>
> pcie@7,0 { // Lane 7: Ethernet
Port 7 is lane 6 and Port 9 is lane 7. So I think it should be 'Lane 6'.
> reg = <0x3800 0 0 0 0>;
> compatible = "pciclass,0604";
> device_type = "pci";
> reset-gpios = <&gpio25 2 0 >;
> #address-cells = <3>;
> #size-cells = <2>;
> ranges;
> };
> };
> };
> };
>
> Right?
>
> After updating the dt-schema from your git tree, the above doesn't
> generate warnings anymore.
>
> Thanks,
> Mauro
>
Em Mon, 2 Aug 2021 16:50:53 -0600
Rob Herring <[email protected]> escreveu:
> On Thu, Jul 29, 2021 at 09:03:37PM +0200, Mauro Carvalho Chehab wrote:
> > Em Thu, 29 Jul 2021 09:20:15 -0600
> > Rob Herring <[email protected]> escreveu:
> >
> >
> > Ok. If I understood your review, the schema will then be:
> >
> > pcie@f4000000 {
> > compatible = "hisilicon,kirin970-pcie";
> > reg = <0x0 0xf4000000 0x0 0x1000000>,
> > <0x0 0xfc180000 0x0 0x1000>,
> > <0x0 0xf5000000 0x0 0x2000>;
> > reg-names = "dbi", "apb", "config";
> > bus-range = <0x0 0x1>;
> > msi-parent = <&its_pcie>;
> > #address-cells = <3>;
> > #size-cells = <2>;
> > device_type = "pci";
> > phys = <&pcie_phy>;
> > ranges = <0x02000000 0x0 0x00000000
> > 0x0 0xf6000000
> > 0x0 0x02000000>;
> > num-lanes = <1>;
> > #interrupt-cells = <1>;
> > interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> > interrupt-names = "msi";
> > interrupt-map-mask = <0 0 0 7>;
> > interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> > <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> > <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> > <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> > reset-gpios = <&gpio7 0 0>;
> >
> > pcie@0 { // Lane 0: upstream
> > reg = <0 0 0 0 0>;
> > compatible = "pciclass,0604";
> > device_type = "pci";
> > #address-cells = <3>;
> > #size-cells = <2>;
> > hisilicon,clken-gpios = <&gpio27 3 0 >, <&gpio17 0 0 >, <&gpio20 6 0 >;
>
> Up one more level.
Yeah. This is at the upper level at the newer series:
[PATCH v2 3/4] dt-bindings: PCI: kirin: Add support for Kirin970
https://lore.kernel.org/lkml/93a42a6317eed3b0eb6a35b6d4c484e106cb2793.1627637448.git.mchehab+huawei@kernel.org/
> > ranges;
> >
> > pcie@1,0 { // Lane 4: M.2
> > reg = <0x800 0 0 0 0>;
> > compatible = "pciclass,0604";
> > device_type = "pci";
> > reset-gpios = <&gpio3 1 0>;
> > #address-cells = <3>;
> > #size-cells = <2>;
> > ranges;
> > };
> >
> > pcie@5,0 { // Lane 5: Mini PCIe
> > reg = <0x2800 0 0 0 0>;
> > compatible = "pciclass,0604";
> > device_type = "pci";
> > reset-gpios = <&gpio27 4 0 >;
> > #address-cells = <3>;
> > #size-cells = <2>;
> > ranges;
> > };
> >
> > pcie@7,0 { // Lane 7: Ethernet
>
> Port 7 is lane 6 and Port 9 is lane 7. So I think it should be 'Lane 6'.
True. I'll fix it on v3.
>
> > reg = <0x3800 0 0 0 0>;
> > compatible = "pciclass,0604";
> > device_type = "pci";
> > reset-gpios = <&gpio25 2 0 >;
> > #address-cells = <3>;
> > #size-cells = <2>;
> > ranges;
> > };
> > };
> > };
> > };
> >
> > Right?
> >
> > After updating the dt-schema from your git tree, the above doesn't
> > generate warnings anymore.
> >
> > Thanks,
> > Mauro
> >
Thanks,
Mauro