> -----Original Message----- > From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> > Sent: 21 May 2025 15:07 > To: Shradha Todi <shradha.t@xxxxxxxxxxx> > Cc: linux-pci@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-samsung-soc@xxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; linux-phy@xxxxxxxxxxxxxxxxxxx; manivannan.sadhasivam@xxxxxxxxxx; lpieralisi@xxxxxxxxxx; > kw@xxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx; jingoohan1@xxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; > alim.akhtar@xxxxxxxxxxx; vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; arnd@xxxxxxxx; m.szyprowski@xxxxxxxxxxx; > jh80.chung@xxxxxxxxxxx > 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@xxxxxxxxxxx> > > --- > > .../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.yam > > +++ l > > Filename matching compatible. Okay, will change it to tesla,fsd-pcie-ep.yaml > > > @@ -0,0 +1,66 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +https://protect2.fireeye.com/v1/url?k=011d92c7-5e86abcb-011c1988-000b > > +abff3563-f87bc6d1cb527c28&q=1&e=3d0e8e81-bcdc-412b-ba41-5d5936c37c73& > > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fpci%2Fsamsung%2Cexynos-pcie > > +-ep.yaml%23 > > +$schema: > > +https://protect2.fireeye.com/v1/url?k=dc0b3b6d-83900261-dc0ab022-000b > > +abff3563-91c2c3470c50d358&q=1&e=3d0e8e81-bcdc-412b-ba41-5d5936c37c73& > > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23 > > + > > +title: Samsung SoC series PCIe Endpoint Controller > > + > > +maintainers: > > + - Shradha Todi <shradha.t@xxxxxxxxxx> > > + > > +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. > Will add the constraints > > + 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@16a00000 { > > + 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@xxxxxxxxxxx> > > > > 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? > I changed the binding file to include both FSD and exynos which have quite a few different DT properties and constraints. I understand I should keep the common properties like reg, phys defined in top-level. Will do that. > > Best regards, > Krzysztof
