Re: [PATCH v2 2/2] dt-bindings: PCI: Convert PCIe Host/Endpoint in Cadence platform to DT schema

[Kishon Vijay Abraham I <kishon@ti.com>]

Rob,

On 24/02/20 8:56 pm, Rob Herring wrote:
> On Mon, Feb 24, 2020 at 4:14 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>
>> Hi Rob,
>>
>> On 20/02/20 2:02 am, Rob Herring wrote:
>>> On Mon, Feb 17, 2020 at 04:45:19PM +0530, Kishon Vijay Abraham I wrote:
>>>> Include Cadence core DT schema and define the Cadence platform DT schema
>>>> for both Host and Endpoint mode. Note: The Cadence core DT schema could
>>>> be included for other platforms using Cadence PCIe core.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>>  .../bindings/pci/cdns,cdns-pcie-ep.txt        | 27 -------
>>>>  .../bindings/pci/cdns,cdns-pcie-ep.yaml       | 48 ++++++++++++
>>>>  .../bindings/pci/cdns,cdns-pcie-host.txt      | 66 ----------------
>>>>  .../bindings/pci/cdns,cdns-pcie-host.yaml     | 76 +++++++++++++++++++
>>>>  MAINTAINERS                                   |  2 +-
>>>>  5 files changed, 125 insertions(+), 94 deletions(-)
>>>>  delete mode 100644 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt
>>>>  create mode 100644 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>>>>  delete mode 100644 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
>>>>  create mode 100644 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>
>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>> new file mode 100644
>>>> index 000000000000..2f605297f862
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>> @@ -0,0 +1,76 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/pci/cdns,cdns-pcie-host.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Cadence PCIe host controller
>>>> +
>>>> +maintainers:
>>>> +  - Tom Joseph <tjoseph@cadence.com>
>>>> +
>>>> +allOf:
>>>> +  - $ref: /schemas/pci/pci-bus.yaml#
>>>> +  - $ref: "cdns-pcie-host.yaml#"
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: cdns,cdns-pcie-host
>>>> +
>>>> +  reg:
>>>> +    maxItems: 3
>>>> +
>>>> +  reg-names:
>>>> +    items:
>>>> +      - const: reg
>>>> +      - const: cfg
>>>> +      - const: mem
>>>> +
>>>> +  msi-parent: true
>>>> +
>>>> +required:
>>>> +  - reg
>>>> +  - reg-names
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    bus {
>>>> +        #address-cells = <2>;
>>>> +        #size-cells = <2>;
>>>> +
>>>> +        pcie@fb000000 {
>>>> +            compatible = "cdns,cdns-pcie-host";
>>>> +            device_type = "pci";
>>>> +            #address-cells = <3>;
>>>> +            #size-cells = <2>;
>>>> +            bus-range = <0x0 0xff>;
>>>> +            linux,pci-domain = <0>;
>>>> +            cdns,max-outbound-regions = <16>;
>>>> +            cdns,no-bar-match-nbits = <32>;
>>>
>>>> +            vendor-id = /bits/ 16 <0x17cd>;
>>>> +            device-id = /bits/ 16 <0x0200>;
>>>
>>> Please make these 32-bit as that is what the spec says.
>>
>> Can you clarify this is mentioned in which spec? PCI spec has both of
>> these 16 bits and I checked the PCI binding doc but couldn't spot the
>> size of these fields.
>>
>> [1] -> https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
> 
> Section 4.1.2.1. The key point is the type is 'encode-int' which means
> 32-bit. Keep in mind, that 16-bits was not a defined type when this
> spec was written. We added that for FDT.
> 
> Also, look at other instances of reading 'vendor-id' in the kernel.

Thanks for clarifying.

Regards
Kishon