lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240304173204.GA777171-robh@kernel.org>
Date: Mon, 4 Mar 2024 11:32:04 -0600
From: Rob Herring <robh@...nel.org>
To: Frank Li <Frank.Li@....com>
Cc: conor@...nel.org, bhelgaas@...gle.com, conor+dt@...nel.org,
	devicetree@...r.kernel.org, helgaas@...nel.org, imx@...ts.linux.dev,
	krzysztof.kozlowski+dt@...aro.org, kw@...ux.com,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	lpieralisi@...nel.org
Subject: Re: [PATCH v6 1/3] dt-bindings: pci: layerscape-pci: Convert to yaml
 format

On Fri, Mar 01, 2024 at 11:27:39AM -0500, Frank Li wrote:
> Split layerscape-pci.txt into two yaml files: fsl,layerscape-pcie-ep.yaml
> and fsl,layerscape-pcie.yaml.
> yaml files contain the same content as the original txt file.
> 
> Do below changes to pass dtb_binding check:
> - Remove dma-coherent and fsl,pcie-scfg because not every SOC need it.

You mean 'remove from required' right? Because they are still here.

> - Set unevaluatedProperties to true in fsl,layerscape-pcie.yaml.

Sorry, but that's not acceptable either. You need the $ref's in this 
patch.

> 
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
>  .../bindings/pci/fsl,layerscape-pcie-ep.yaml  |  87 +++++++++++++
>  .../bindings/pci/fsl,layerscape-pcie.yaml     | 121 ++++++++++++++++++
>  .../bindings/pci/layerscape-pci.txt           |  79 ------------
>  3 files changed, 208 insertions(+), 79 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
>  create mode 100644 Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
>  delete mode 100644 Documentation/devicetree/bindings/pci/layerscape-pci.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> new file mode 100644
> index 0000000000000..cf517e4e46a33
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie-ep.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/fsl,layerscape-pcie-ep.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Layerscape PCIe Root Complex(RC) controller
> +
> +maintainers:
> +  - Frank Li <Frank.Li@....com>
> +
> +description:
> +  This PCIe RC controller is based on the Synopsys DesignWare PCIe IP
> +  and thus inherits all the common properties defined in snps,dw-pcie.yaml.
> +
> +  This controller derives its clocks from the Reset Configuration Word (RCW)
> +  which is used to describe the PLL settings at the time of chip-reset.
> +
> +  Also as per the available Reference Manuals, there is no specific 'version'
> +  register available in the Freescale PCIe controller register set,
> +  which can allow determining the underlying DesignWare PCIe controller version
> +  information.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - fsl,ls1028a-pcie-ep
> +          - fsl,ls2046a-pcie-ep
> +          - fsl,ls2088a-pcie-ep
> +          - fsl,ls1046a-pcie-ep
> +          - fsl,ls1043a-pcie-ep
> +          - fsl,ls1012a-pcie-ep
> +          - fsl,lx2160ar2-pcie-ep
> +      - const: fsl,ls-pcie-ep
> +
> +  reg:
> +    description: base addresses and lengths of the PCIe controller register blocks.

You need to define how many entries and what they are.

Missing 'reg-names'?

> +
> +  interrupts:
> +    description: A list of interrupt outputs of the controller. Must contain an
> +      entry for each entry in the interrupt-names property.

You need to define how many entries and what they are.

> +
> +  interrupt-names:
> +    minItems: 1
> +    maxItems: 3
> +    description: It could include the following entries.
> +    items:
> +      oneOf:
> +        - description:
> +            Used for interrupt line which reports AER events when
> +            non MSI/MSI-X/INTx mode is used.
> +          const: aer
> +        - description:
> +            Used for interrupt line which reports PME events when
> +            non MSI/MSI-X/INTx mode is used.
> +          const: pme
> +        - description:
> +            Used for SoCs(like ls2080a, lx2160a, ls2080a, ls2088a, ls1088a)
> +            which has a single interrupt line for miscellaneous controller
> +            events(could include AER and PME events).
> +          const: intr

The way this works is the common schema defines all possible names. This 
schema needs to define how many entries, which names you use, and what 
is the order. So you need to add 'pme' and 'aer' to 
snps,dw-pcie-ep.yaml. 

I imagine the order of entries is a mess here, and I don't expect 
there's any new Layerscape platforms coming. So this binding can just 
say:

minItems: 1
maxItems: 3
items:
  enum: [ aer, pme, intr ]

> +
> +  fsl,pcie-scfg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Must include two entries.
> +      The first entry must be a link to the SCFG device node
> +      The second entry is the physical PCIe controller index starting from '0'.
> +      This is used to get SCFG PEXN registers
> +
> +  dma-coherent:
> +    description: Indicates that the hardware IP block can ensure the coherency
> +      of the data transferred from/to the IP block. This can avoid the software
> +      cache flush/invalid actions, and improve the performance significantly

Already listed in the common schema, so you can drop.

> +
> +  big-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: If the PEX_LUT and PF register block is in big-endian, specify
> +      this property.
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-names
> +
> diff --git a/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> new file mode 100644
> index 0000000000000..3f2d058701d22
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/fsl,layerscape-pcie.yaml
> @@ -0,0 +1,121 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/fsl,layerscape-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Layerscape PCIe Root Complex(RC) controller
> +
> +maintainers:
> +  - Frank Li <Frank.Li@....com>
> +
> +description:
> +  This PCIe RC controller is based on the Synopsys DesignWare PCIe IP
> +  and thus inherits all the common properties defined in snps,dw-pcie.yaml.
> +
> +  This controller derives its clocks from the Reset Configuration Word (RCW)
> +  which is used to describe the PLL settings at the time of chip-reset.
> +
> +  Also as per the available Reference Manuals, there is no specific 'version'
> +  register available in the Freescale PCIe controller register set,
> +  which can allow determining the underlying DesignWare PCIe controller version
> +  information.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,ls1021a-pcie
> +      - fsl,ls2080a-pcie
> +      - fsl,ls2085a-pcie
> +      - fsl,ls2088a-pcie
> +      - fsl,ls1088a-pcie
> +      - fsl,ls1046a-pcie
> +      - fsl,ls1043a-pcie
> +      - fsl,ls1012a-pcie
> +      - fsl,ls1028a-pcie
> +      - fsl,lx2160a-pcie
> +
> +  reg:
> +    description: base addresses and lengths of the PCIe controller register blocks.

You need to define how many entries and what they are.


> +
> +  interrupts:
> +    description: A list of interrupt outputs of the controller. Must contain an
> +      entry for each entry in the interrupt-names property.

You need to define how many entries and what they are.

> +
> +  interrupt-names:
> +    minItems: 1
> +    maxItems: 3
> +    description: It could include the following entries.
> +    items:
> +      oneOf:
> +        - description:
> +            Used for interrupt line which reports AER events when
> +            non MSI/MSI-X/INTx mode is used.
> +          const: aer
> +        - description:
> +            Used for interrupt line which reports PME events when
> +            non MSI/MSI-X/INTx mode is used.
> +          const: pme
> +        - description:
> +            Used for SoCs(like ls2080a, lx2160a, ls2080a, ls2088a, ls1088a)
> +            which has a single interrupt line for miscellaneous controller
> +            events(could include AER and PME events).
> +          const: intr

Similar comment here, but the names are already defined in 
snps,dw-pcie.yaml.

> +
> +  fsl,pcie-scfg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Must include two entries.
> +      The first entry must be a link to the SCFG device node
> +      The second entry is the physical PCIe controller index starting from '0'.
> +      This is used to get SCFG PEXN registers
> +
> +  dma-coherent:
> +    description: Indicates that the hardware IP block can ensure the coherency
> +      of the data transferred from/to the IP block. This can avoid the software
> +      cache flush/invalid actions, and improve the performance significantly

Drop

> +
> +  big-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: If the PEX_LUT and PF register block is in big-endian, specify
> +      this property.
> +
> +unevaluatedProperties: true
> +
> +required:

> +  - compatible
> +  - reg

Both required in common schema. Drop.

> +  - interrupt-names
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      pcie@...0000 {
> +        compatible = "fsl,ls1088a-pcie";
> +        reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */
> +            <0x20 0x00000000 0x0 0x00002000>; /* configuration space */
> +        reg-names = "regs", "config";
> +        interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
> +        interrupt-names = "aer";
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +        dma-coherent;
> +        device_type = "pci";
> +        bus-range = <0x0 0xff>;
> +        ranges = <0x81000000 0x0 0x00000000 0x20 0x00010000 0x0 0x00010000   /* downstream I/O */
> +                 0x82000000 0x0 0x40000000 0x20 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
> +        msi-parent = <&its>;
> +        #interrupt-cells = <1>;
> +        interrupt-map-mask = <0 0 0 7>;
> +        interrupt-map = <0000 0 0 1 &gic 0 0 0 109 IRQ_TYPE_LEVEL_HIGH>,
> +                        <0000 0 0 2 &gic 0 0 0 110 IRQ_TYPE_LEVEL_HIGH>,
> +                        <0000 0 0 3 &gic 0 0 0 111 IRQ_TYPE_LEVEL_HIGH>,
> +                        <0000 0 0 4 &gic 0 0 0 112 IRQ_TYPE_LEVEL_HIGH>;
> +        iommu-map = <0 &smmu 0 1>; /* Fixed-up by bootloader */
> +      };
> +    };
> +...

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ