linux-kernel - Re: [PATCH 06/10] dt-bindings: PCI: Add bindings support for Tesla FSD SoC

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: <20250521-capable-affable-numbat-b0ce84@kuoka>
Date: Wed, 21 May 2025 11:37:00 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Shradha Todi <shradha.t@...sung.com>
Cc: linux-pci@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.or, linux-kernel@...r.kernel.org, 
	linux-phy@...ts.infradead.org, manivannan.sadhasivam@...aro.org, lpieralisi@...nel.org, 
	kw@...ux.com, robh@...nel.org, bhelgaas@...gle.com, jingoohan1@...il.com, 
	krzk+dt@...nel.org, conor+dt@...nel.org, alim.akhtar@...sung.com, vkoul@...nel.org, 
	kishon@...nel.org, arnd@...db.de, m.szyprowski@...sung.com, jh80.chung@...sung.com
Subject: Re: [PATCH 06/10] dt-bindings: PCI: Add bindings support for Tesla
 FSD SoC

On Mon, May 19, 2025 at 01:01:48AM GMT, Shradha Todi wrote:
> Document the PCIe controller device tree bindings for Tesla FSD
> SoC for both RC and EP.
> 
> Signed-off-by: Shradha Todi <shradha.t@...sung.com>
> ---
>  .../bindings/pci/samsung,exynos-pcie-ep.yaml  |  66 ++++++
>  .../bindings/pci/samsung,exynos-pcie.yaml     | 199 ++++++++++++------
>  2 files changed, 198 insertions(+), 67 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
> new file mode 100644
> index 000000000000..5d4a9067f727
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml

Filename matching compatible.

> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/samsung,exynos-pcie-ep.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung SoC series PCIe Endpoint Controller
> +
> +maintainers:
> +  - Shradha Todi <shradha.t@...sung.co>
> +
> +description: |+
> +  Samsung SoCs PCIe endpoint controller is based on the Synopsys DesignWare
> +  PCIe IP and thus inherits all the common properties defined in
> +  snps,dw-pcie-ep.yaml.
> +
> +properties:
> +  compatible:
> +    oneOf:

Drop

> +      - enum:
> +          - tesla,fsd-pcie-ep
> +
> +allOf:
> +  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - tesla,fsd-pcie-ep

What is the point of this if:? There are no other variants.

Also, missing constraints for all the properties. This is really
incomplete.

> +    then:
> +      properties:
> +        samsung,syscon-pcie:
> +          description: phandle for system control registers, used to
> +                       control signals at system level

Where is the type defined? Look how such properties are described -
there are plenty of examples.

> +
> +      required:
> +        - samsung,syscon-pcie
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/fsd-clk.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        pcieep0: pcie-ep@...00000 {
> +            compatible = "tesla,fsd-pcie-ep";
> +            reg = <0x0 0x168b0000 0x0 0x1000>,
> +                  <0x0 0x16a00000 0x0 0x2000>,
> +                  <0x0 0x16a01000 0x0 0x80>,
> +                  <0x0 0x17000000 0x0 0xff0000>;
> +            reg-names = "elbi", "dbi", "dbi2", "addr_space";
> +            clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
> +            clock-names = "aux", "dbi", "mstr", "slv";
> +            num-lanes = <4>;
> +            samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
> +            phys = <&pciephy1>;
> +        };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> index f20ed7e709f7..a3803bf0ef84 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> @@ -11,78 +11,113 @@ maintainers:
>    - Jaehoon Chung <jh80.chung@...sung.com>
>  
>  description: |+
> -  Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare
> +  Samsung SoCs PCIe host controller is based on the Synopsys DesignWare
>    PCIe IP and thus inherits all the common properties defined in
>    snps,dw-pcie.yaml.
>  
> -allOf:
> -  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> -
>  properties:
>    compatible:
> -    const: samsung,exynos5433-pcie
> -
> -  reg:
> -    items:
> -      - description: Data Bus Interface (DBI) registers.
> -      - description: External Local Bus interface (ELBI) registers.
> -      - description: PCIe configuration space region.
> -

No, I do not understand any of this change. Properties are defined in
top-level. Why all this is being removed?


Best regards,
Krzysztof