diff mbox series

[3/5] dt-bindings: PCI: kirin: Add support for Kirin970

Message ID 2cf7bd80d0b54f7658a64febf79d3a36e70aba86.1627559126.git.mchehab+huawei@kernel.org (mailing list archive)
State Superseded
Delegated to: Lorenzo Pieralisi
Headers show
Series DT schema changes for HiKey970 PCIe hardware to work | expand

Commit Message

Mauro Carvalho Chehab July 29, 2021, 11:56 a.m. UTC
Add a new compatible, plus the new bindings needed by
HiKey970 board.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/pci/hisilicon,kirin-pcie.yaml    | 61 ++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

Comments

Rob Herring (Arm) July 29, 2021, 2:50 p.m. UTC | #1
On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Add a new compatible, plus the new bindings needed by
> HiKey970 board.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  .../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
>
Rob Herring (Arm) July 29, 2021, 3:20 p.m. UTC | #2
On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Add a new compatible, plus the new bindings needed by
> HiKey970 board.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  .../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
Mauro Carvalho Chehab July 29, 2021, 7:03 p.m. UTC | #3
Em Thu, 29 Jul 2021 09:20:15 -0600
Rob Herring <robh@kernel.org> escreveu:

> On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Add a new compatible, plus the new bindings needed by
> > HiKey970 board.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> >  .../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
Rob Herring (Arm) Aug. 2, 2021, 10:50 p.m. UTC | #4
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 <robh@kernel.org> escreveu:
> 
> > On Thu, Jul 29, 2021 at 5:56 AM Mauro Carvalho Chehab
> > <mchehab+huawei@kernel.org> wrote:
> > >
> > > Add a new compatible, plus the new bindings needed by
> > > HiKey970 board.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > ---
> > >  .../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
>
Mauro Carvalho Chehab Aug. 3, 2021, 4:33 a.m. UTC | #5
Em Mon, 2 Aug 2021 16:50:53 -0600
Rob Herring <robh@kernel.org> 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 <robh@kernel.org> 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
diff mbox series

Patch

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;
+        };
+      };
     };