[v1,1/3] dt-binding: pci: add JH7110 PCIe dt-binding documents.

  Add PCIe controller driver dt-binding documents
for StarFive JH7110 SoC platform.

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 .../bindings/pci/starfive,jh7110-pcie.yaml    | 163 ++++++++++++++++++
 1 file changed, 163 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
  

On 06/04/2023 13:11, Minda Chen wrote:
> Add PCIe controller driver dt-binding documents
> for StarFive JH7110 SoC platform.

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching). Missing: 's'

Subject: drop second/last, redundant "dt-binding documents". The
"dt-bindings" prefix is already stating that these are bindings and
documentation.

Drop also full stop.

> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../bindings/pci/starfive,jh7110-pcie.yaml    | 163 ++++++++++++++++++
>  1 file changed, 163 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml b/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
> new file mode 100644
> index 000000000000..fa4829766195
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
> @@ -0,0 +1,163 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/starfive,jh7110-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 PCIe 2.0 host controller
> +
> +maintainers:
> +  - Minda Chen <minda.chen@starfivetech.com>
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-pcie
> +
> +  reg:
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: reg
> +      - const: config
> +
> +  msi-parent: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 4
> +
> +  clock-names:
> +    items:
> +      - const: noc
> +      - const: tl
> +      - const: axi_mst0
> +      - const: apb
> +
> +  resets:
> +    items:
> +      - description: AXI MST0 reset
> +      - description: AXI SLAVE reset
> +      - description: AXI SLAVE0 reset
> +      - description: PCIE BRIDGE reset
> +      - description: PCIE CORE reset
> +      - description: PCIE APB reset
> +
> +  reset-names:
> +    items:
> +      - const: mst0
> +      - const: slv0
> +      - const: slv
> +      - const: brg
> +      - const: core
> +      - const: apb
> +
> +  starfive,stg-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: phandle to System Register Controller stg_syscon node.
> +        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> +        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> +        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> +        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> +    description:
> +      The phandle to System Register Controller syscon node and the offset
> +      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
> +      for PCIe.
> +
> +  pwren-gpios:
> +    description: Should specify the GPIO for controlling the PCI bus device power on.

What are these? Different than defined in gpio-consumer-common?

> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  phys:
> +    maxItems: 1
> +
> +  interrupt-controller:
> +    type: object
> +    properties:
> +      '#address-cells':
> +        const: 0
> +
> +      '#interrupt-cells':
> +        const: 1
> +
> +      interrupt-controller: true
> +
> +    required:
> +      - '#address-cells'
> +      - '#interrupt-cells'
> +      - interrupt-controller
> +
> +    additionalProperties: false
> +
> +required:
> +  - reg
> +  - reg-names
> +  - "#interrupt-cells"

Keep consistent quotes - either ' or "

Are you sure this is correct? You have interrupt controller as child node.


> +  - interrupts
> +  - interrupt-map-mask
> +  - interrupt-map
> +  - clocks
> +  - clock-names
> +  - resets
> +  - msi-controller
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcie0: pcie@2B000000 {

Lowercase hex. Everywhere.

> +            compatible = "starfive,jh7110-pcie";
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            #interrupt-cells = <1>;
> +            reg = <0x0 0x2B000000 0x0 0x1000000>,
> +                  <0x9 0x40000000 0x0 0x10000000>;

reg (and reg-names and ranges) is always second property.

> +            reg-names = "reg", "config";
> +            device_type = "pci";
> +            starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
> +            bus-range = <0x0 0xff>;
> +            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
> +                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;
> +            interrupt-parent = <&plic>;
> +            interrupts = <56>;
> +            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> +            interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc0 0x1>,
> +                            <0x0 0x0 0x0 0x2 &pcie_intc0 0x2>,
> +                            <0x0 0x0 0x0 0x3 &pcie_intc0 0x3>,
> +                            <0x0 0x0 0x0 0x4 &pcie_intc0 0x4>;


Best regards,
Krzysztof
  

On Thu, Apr 06, 2023 at 08:24:55PM +0200, Krzysztof Kozlowski wrote:
> On 06/04/2023 13:11, Minda Chen wrote:
> > +
> > +  interrupt-controller:
> > +    type: object
> > +    properties:
> > +      '#address-cells':
> > +        const: 0
> > +
> > +      '#interrupt-cells':
> > +        const: 1
> > +
> > +      interrupt-controller: true
> > +
> > +    required:
> > +      - '#address-cells'
> > +      - '#interrupt-cells'
> > +      - interrupt-controller
> > +
> > +    additionalProperties: false
> > +
> > +required:
> > +  - reg
> > +  - reg-names
> > +  - "#interrupt-cells"
> 
> Keep consistent quotes - either ' or "
> 
> Are you sure this is correct? You have interrupt controller as child node.

I know existing stuff in-tree is far from a guarantee that it'll be
right, but this does at least follow what we've got for PolarFire SoC:
Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml

Both PLDA and both RISC-V w/ a PLIC as the interrupt controller, so in
similar waters.
This note existed in the original text form binding of the Microchip
PCI controller:
| +NOTE:
| +The core provides a single interrupt for both INTx/MSI messages. So,
| +create an interrupt controller node to support 'interrupt-map' DT
| +functionality.  The driver will create an IRQ domain for this map, decode
| +the four INTx interrupts in ISR and route them to this domain.

Given the similarities, I figure the same requirement applies here too.
Minda?

Cheers,
Conor.
  

On Thu, Apr 06, 2023 at 07:35:09PM +0100, Conor Dooley wrote:
> On Thu, Apr 06, 2023 at 08:24:55PM +0200, Krzysztof Kozlowski wrote:
> > On 06/04/2023 13:11, Minda Chen wrote:
> > > +
> > > +  interrupt-controller:
> > > +    type: object
> > > +    properties:
> > > +      '#address-cells':
> > > +        const: 0
> > > +
> > > +      '#interrupt-cells':
> > > +        const: 1
> > > +
> > > +      interrupt-controller: true
> > > +
> > > +    required:
> > > +      - '#address-cells'
> > > +      - '#interrupt-cells'
> > > +      - interrupt-controller
> > > +
> > > +    additionalProperties: false
> > > +
> > > +required:
> > > +  - reg
> > > +  - reg-names
> > > +  - "#interrupt-cells"
> > 
> > Keep consistent quotes - either ' or "
> > 
> > Are you sure this is correct? You have interrupt controller as child node.
> 
> I know existing stuff in-tree is far from a guarantee that it'll be
> right, but this does at least follow what we've got for PolarFire SoC:
> Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
> 
> Both PLDA and both RISC-V w/ a PLIC as the interrupt controller, so in
> similar waters.
> This note existed in the original text form binding of the Microchip
> PCI controller:
> | +NOTE:
> | +The core provides a single interrupt for both INTx/MSI messages. So,
> | +create an interrupt controller node to support 'interrupt-map' DT
> | +functionality.  The driver will create an IRQ domain for this map, decode
> | +the four INTx interrupts in ISR and route them to this domain.
> 
> Given the similarities, I figure the same requirement applies here too.
> Minda?

Further, if, as I currently suspect, there's a lot of commonality here,
should the binding as well as the driver be split into common pdla bits
and microchip/starfive specific ones?

Suppose that's more one for you Krzysztof.

Cheers,
Conor.
  

On 06/04/2023 20:45, Conor Dooley wrote:
> On Thu, Apr 06, 2023 at 07:35:09PM +0100, Conor Dooley wrote:
>> On Thu, Apr 06, 2023 at 08:24:55PM +0200, Krzysztof Kozlowski wrote:
>>> On 06/04/2023 13:11, Minda Chen wrote:
>>>> +
>>>> +  interrupt-controller:
>>>> +    type: object
>>>> +    properties:
>>>> +      '#address-cells':
>>>> +        const: 0
>>>> +
>>>> +      '#interrupt-cells':
>>>> +        const: 1
>>>> +
>>>> +      interrupt-controller: true
>>>> +
>>>> +    required:
>>>> +      - '#address-cells'
>>>> +      - '#interrupt-cells'
>>>> +      - interrupt-controller
>>>> +
>>>> +    additionalProperties: false
>>>> +
>>>> +required:
>>>> +  - reg
>>>> +  - reg-names
>>>> +  - "#interrupt-cells"
>>>
>>> Keep consistent quotes - either ' or "
>>>
>>> Are you sure this is correct? You have interrupt controller as child node.
>>
>> I know existing stuff in-tree is far from a guarantee that it'll be
>> right, but this does at least follow what we've got for PolarFire SoC:
>> Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
>>
>> Both PLDA and both RISC-V w/ a PLIC as the interrupt controller, so in
>> similar waters.
>> This note existed in the original text form binding of the Microchip
>> PCI controller:
>> | +NOTE:
>> | +The core provides a single interrupt for both INTx/MSI messages. So,
>> | +create an interrupt controller node to support 'interrupt-map' DT
>> | +functionality.  The driver will create an IRQ domain for this map, decode
>> | +the four INTx interrupts in ISR and route them to this domain.
>>
>> Given the similarities, I figure the same requirement applies here too.
>> Minda?
> 
> Further, if, as I currently suspect, there's a lot of commonality here,
> should the binding as well as the driver be split into common pdla bits
> and microchip/starfive specific ones?
> 
> Suppose that's more one for you Krzysztof.

Yeah, looks like only clocks and resets are different. At the end it
depends how much code you would remove...

Best regards,
Krzysztof
  

On 2023/4/7 2:35, Conor Dooley wrote:
> On Thu, Apr 06, 2023 at 08:24:55PM +0200, Krzysztof Kozlowski wrote:
>> On 06/04/2023 13:11, Minda Chen wrote:
>> > +
>> > +  interrupt-controller:
>> > +    type: object
>> > +    properties:
>> > +      '#address-cells':
>> > +        const: 0
>> > +
>> > +      '#interrupt-cells':
>> > +        const: 1
>> > +
>> > +      interrupt-controller: true
>> > +
>> > +    required:
>> > +      - '#address-cells'
>> > +      - '#interrupt-cells'
>> > +      - interrupt-controller
>> > +
>> > +    additionalProperties: false
>> > +
>> > +required:
>> > +  - reg
>> > +  - reg-names
>> > +  - "#interrupt-cells"
>> 
>> Keep consistent quotes - either ' or "
>> 
>> Are you sure this is correct? You have interrupt controller as child node.
> 
> I know existing stuff in-tree is far from a guarantee that it'll be
> right, but this does at least follow what we've got for PolarFire SoC:
> Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
> 
> Both PLDA and both RISC-V w/ a PLIC as the interrupt controller, so in
> similar waters.
> This note existed in the original text form binding of the Microchip
> PCI controller:
> | +NOTE:
> | +The core provides a single interrupt for both INTx/MSI messages. So,
> | +create an interrupt controller node to support 'interrupt-map' DT
> | +functionality.  The driver will create an IRQ domain for this map, decode
> | +the four INTx interrupts in ISR and route them to this domain.
> 
> Given the similarities, I figure the same requirement applies here too.
> Minda?
> 
> Cheers,
> Conor.

Yes, Thanks very much.
  

On 2023/4/7 2:24, Krzysztof Kozlowski wrote:
> On 06/04/2023 13:11, Minda Chen wrote:
>> Add PCIe controller driver dt-binding documents
>> for StarFive JH7110 SoC platform.
> 
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching). Missing: 's'
> 
> Subject: drop second/last, redundant "dt-binding documents". The
> "dt-bindings" prefix is already stating that these are bindings and
> documentation.
> 
> Drop also full stop.
> 
ok, thanks
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  .../bindings/pci/starfive,jh7110-pcie.yaml    | 163 ++++++++++++++++++
>>  1 file changed, 163 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml b/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
>> new file mode 100644
>> index 000000000000..fa4829766195
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
>> @@ -0,0 +1,163 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pci/starfive,jh7110-pcie.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive JH7110 PCIe 2.0 host controller
>> +
>> +maintainers:
>> +  - Minda Chen <minda.chen@starfivetech.com>
>> +
>> +allOf:
>> +  - $ref: /schemas/pci/pci-bus.yaml#
>> +  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-pcie
>> +
>> +  reg:
>> +    maxItems: 2
>> +
>> +  reg-names:
>> +    items:
>> +      - const: reg
>> +      - const: config
>> +
>> +  msi-parent: true
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 4
>> +
>> +  clock-names:
>> +    items:
>> +      - const: noc
>> +      - const: tl
>> +      - const: axi_mst0
>> +      - const: apb
>> +
>> +  resets:
>> +    items:
>> +      - description: AXI MST0 reset
>> +      - description: AXI SLAVE reset
>> +      - description: AXI SLAVE0 reset
>> +      - description: PCIE BRIDGE reset
>> +      - description: PCIE CORE reset
>> +      - description: PCIE APB reset
>> +
>> +  reset-names:
>> +    items:
>> +      - const: mst0
>> +      - const: slv0
>> +      - const: slv
>> +      - const: brg
>> +      - const: core
>> +      - const: apb
>> +
>> +  starfive,stg-syscon:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    items:
>> +      items:
>> +        - description: phandle to System Register Controller stg_syscon node.
>> +        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>> +        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>> +        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>> +        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>> +    description:
>> +      The phandle to System Register Controller syscon node and the offset
>> +      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
>> +      for PCIe.
>> +
>> +  pwren-gpios:
>> +    description: Should specify the GPIO for controlling the PCI bus device power on.
> 
> What are these? Different than defined in gpio-consumer-common?
> 
power gpio board level configuration. It it not a requried property
>> +    maxItems: 1
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +
>> +  phys:
>> +    maxItems: 1
>> +
>> +  interrupt-controller:
>> +    type: object
>> +    properties:
>> +      '#address-cells':
>> +        const: 0
>> +
>> +      '#interrupt-cells':
>> +        const: 1
>> +
>> +      interrupt-controller: true
>> +
>> +    required:
>> +      - '#address-cells'
>> +      - '#interrupt-cells'
>> +      - interrupt-controller
>> +
>> +    additionalProperties: false
>> +
>> +required:
>> +  - reg
>> +  - reg-names
>> +  - "#interrupt-cells"
> 
> Keep consistent quotes - either ' or "
> 
> Are you sure this is correct? You have interrupt controller as child node.
> 
> 
>> +  - interrupts
>> +  - interrupt-map-mask
>> +  - interrupt-map
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - msi-controller
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    bus {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        pcie0: pcie@2B000000 {
> 
> Lowercase hex. Everywhere.
> 
ok
>> +            compatible = "starfive,jh7110-pcie";
>> +            #address-cells = <3>;
>> +            #size-cells = <2>;
>> +            #interrupt-cells = <1>;
>> +            reg = <0x0 0x2B000000 0x0 0x1000000>,
>> +                  <0x9 0x40000000 0x0 0x10000000>;
> 
> reg (and reg-names and ranges) is always second property.
> 
ok
>> +            reg-names = "reg", "config";
>> +            device_type = "pci";
>> +            starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
>> +            bus-range = <0x0 0xff>;
>> +            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
>> +                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;
>> +            interrupt-parent = <&plic>;
>> +            interrupts = <56>;
>> +            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
>> +            interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc0 0x1>,
>> +                            <0x0 0x0 0x0 0x2 &pcie_intc0 0x2>,
>> +                            <0x0 0x0 0x0 0x3 &pcie_intc0 0x3>,
>> +                            <0x0 0x0 0x0 0x4 &pcie_intc0 0x4>;
> 
> 
> Best regards,
> Krzysztof
>
  

On 10/04/2023 11:05, Minda Chen wrote:
>>> +
>>> +  starfive,stg-syscon:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    items:
>>> +      items:
>>> +        - description: phandle to System Register Controller stg_syscon node.
>>> +        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>>> +        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>>> +        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>>> +        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>>> +    description:
>>> +      The phandle to System Register Controller syscon node and the offset
>>> +      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
>>> +      for PCIe.
>>> +
>>> +  pwren-gpios:
>>> +    description: Should specify the GPIO for controlling the PCI bus device power on.
>>
>> What are these? Different than defined in gpio-consumer-common?
>>
> power gpio board level configuration. It it not a requried property

What is "board level configuration"? Again - is it different than
powerdown-gpios from gpio-consumer-common.yaml?



Best regards,
Krzysztof
  

On 2023/4/10 23:21, Krzysztof Kozlowski wrote:
> On 10/04/2023 11:05, Minda Chen wrote:
>>>> +
>>>> +  starfive,stg-syscon:
>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>> +    items:
>>>> +      items:
>>>> +        - description: phandle to System Register Controller stg_syscon node.
>>>> +        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>>>> +        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>>>> +        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>>>> +        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
>>>> +    description:
>>>> +      The phandle to System Register Controller syscon node and the offset
>>>> +      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
>>>> +      for PCIe.
>>>> +
>>>> +  pwren-gpios:
>>>> +    description: Should specify the GPIO for controlling the PCI bus device power on.
>>>
>>> What are these? Different than defined in gpio-consumer-common?
>>>
>> power gpio board level configuration. It it not a requried property
> 
> What is "board level configuration"? Again - is it different than
> powerdown-gpios from gpio-consumer-common.yaml?
> 
> 
I am sorry. I will change to powerdown-gpios follow gpio-consumer-common.yaml
> 
> Best regards,
> Krzysztof
>
  

On Thu, Apr 6, 2023 at 7:59 PM Minda Chen <minda.chen@starfivetech.com> wrote:
>
> Add PCIe controller driver dt-binding documents
> for StarFive JH7110 SoC platform.
>
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../bindings/pci/starfive,jh7110-pcie.yaml    | 163 ++++++++++++++++++
>  1 file changed, 163 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
>
> diff --git a/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml b/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
> new file mode 100644
> index 000000000000..fa4829766195
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
> @@ -0,0 +1,163 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/starfive,jh7110-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 PCIe 2.0 host controller
> +
> +maintainers:
> +  - Minda Chen <minda.chen@starfivetech.com>
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-pcie

Since this is a PLDA IP that is likely to be reused by other vendors,
should this indicate the PLDA in the string?

Or the file name should be renamed to something like the synopsis dw-pcie?

> +
> +  reg:
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: reg
> +      - const: config
> +
> +  msi-parent: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 4
> +
> +  clock-names:
> +    items:
> +      - const: noc
> +      - const: tl
> +      - const: axi_mst0
> +      - const: apb
> +
> +  resets:
> +    items:
> +      - description: AXI MST0 reset
> +      - description: AXI SLAVE reset
> +      - description: AXI SLAVE0 reset
> +      - description: PCIE BRIDGE reset
> +      - description: PCIE CORE reset
> +      - description: PCIE APB reset
> +
> +  reset-names:
> +    items:
> +      - const: mst0
> +      - const: slv0
> +      - const: slv
> +      - const: brg
> +      - const: core
> +      - const: apb
> +
> +  starfive,stg-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: phandle to System Register Controller stg_syscon node.
> +        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> +        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> +        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> +        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> +    description:
> +      The phandle to System Register Controller syscon node and the offset
> +      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
> +      for PCIe.
> +
> +  pwren-gpios:
> +    description: Should specify the GPIO for controlling the PCI bus device power on.
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  phys:
> +    maxItems: 1
> +
> +  interrupt-controller:
> +    type: object
> +    properties:
> +      '#address-cells':
> +        const: 0
> +
> +      '#interrupt-cells':
> +        const: 1
> +
> +      interrupt-controller: true
> +
> +    required:
> +      - '#address-cells'
> +      - '#interrupt-cells'
> +      - interrupt-controller
> +
> +    additionalProperties: false
> +
> +required:
> +  - reg
> +  - reg-names
> +  - "#interrupt-cells"
> +  - interrupts
> +  - interrupt-map-mask
> +  - interrupt-map
> +  - clocks
> +  - clock-names
> +  - resets
> +  - msi-controller
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcie0: pcie@2B000000 {
> +            compatible = "starfive,jh7110-pcie";
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            #interrupt-cells = <1>;
> +            reg = <0x0 0x2B000000 0x0 0x1000000>,
> +                  <0x9 0x40000000 0x0 0x10000000>;
> +            reg-names = "reg", "config";
> +            device_type = "pci";
> +            starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
> +            bus-range = <0x0 0xff>;
> +            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
> +                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;
> +            interrupt-parent = <&plic>;
> +            interrupts = <56>;
> +            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> +            interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc0 0x1>,
> +                            <0x0 0x0 0x0 0x2 &pcie_intc0 0x2>,
> +                            <0x0 0x0 0x0 0x3 &pcie_intc0 0x3>,
> +                            <0x0 0x0 0x0 0x4 &pcie_intc0 0x4>;
> +            msi-parent = <&pcie0>;
> +            msi-controller;
> +            clocks = <&syscrg 86>,
> +                     <&stgcrg 10>,
> +                     <&stgcrg 8>,
> +                     <&stgcrg 9>;
> +            clock-names = "noc", "tl", "axi_mst0", "apb";
> +            resets = <&stgcrg 11>,
> +                     <&stgcrg 12>,
> +                     <&stgcrg 13>,
> +                     <&stgcrg 14>,
> +                     <&stgcrg 15>,
> +                     <&stgcrg 16>;
> +
> +            pcie_intc0: interrupt-controller {
> +                #address-cells = <0>;
> +                #interrupt-cells = <1>;
> +                interrupt-controller;
> +            };
> +        };
> +    };
> --

Regards,
Bin