| Message ID | 20240703180300.42959-2-james.quinlan@broadcom.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: brcnstb: Enable STB 7712 SOC | expand |
On 03/07/2024 20:02, Jim Quinlan wrote: > - Update maintainer; Nicolas hasn't been active and it > makes more sense to have a Broadcom maintainer > - Add a driver compatible string for the new STB SOC 7712 You meant device? Bindings are for hardware. > - Add two new resets for the 7712: "bridge", for the > the bridge between the PCIe core and the memory bus; > "swinit", the PCIe core reset. > - Order the compatible strings alphabetically > - Restructure the reset controllers so that the definitions > appear first before any rules that govern them. Please split cleanups from new device support. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > --- > .../bindings/pci/brcm,stb-pcie.yaml | 44 +++++++++++++++---- > 1 file changed, 36 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > index 11f8ea33240c..a070f35d28d7 100644 > --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > title: Brcmstb PCIe Host Controller > > maintainers: > - - Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > + - Jim Quinlan <james.quinlan@broadcom.com> > > properties: > compatible: > @@ -16,11 +16,12 @@ properties: > - brcm,bcm2711-pcie # The Raspberry Pi 4 > - brcm,bcm4908-pcie > - brcm,bcm7211-pcie # Broadcom STB version of RPi4 > - - brcm,bcm7278-pcie # Broadcom 7278 Arm > - brcm,bcm7216-pcie # Broadcom 7216 Arm > - - brcm,bcm7445-pcie # Broadcom 7445 Arm > + - brcm,bcm7278-pcie # Broadcom 7278 Arm > - brcm,bcm7425-pcie # Broadcom 7425 MIPs > - brcm,bcm7435-pcie # Broadcom 7435 MIPs > + - brcm,bcm7445-pcie # Broadcom 7445 Arm > + - brcm,bcm7712-pcie # STB sibling SOC of Raspberry Pi 5 > > reg: > maxItems: 1 > @@ -95,6 +96,20 @@ properties: > minItems: 1 > maxItems: 3 > > + resets: > + items: > + - description: reset for phy calibration > + - description: reset for PCIe/CPU bus bridge > + - description: reset for soft PCIe core reset > + - description: reset for PERST# PCIe signal This won't work and I doubt you tested your code. You miss minItems. > + > + reset-names: > + items: > + - const: rescal > + - const: bridge > + - const: swinit > + - const: perst This does not match what you have in conditional, so just keep min and max Items here. > + > required: > - compatible > - reg > @@ -118,13 +133,10 @@ allOf: > then: > properties: > resets: > - items: > - - description: reset controller handling the PERST# signal > - > + minItems: 1 maxItems instead. Why three resets should be valid? > reset-names: > items: > - const: perst > - > required: > - resets > - reset-names > @@ -136,12 +148,28 @@ allOf: > then: > properties: > resets: > + minItems: 1 > + reset-names: > items: > - - description: phandle pointing to the RESCAL reset controller > + - const: rescal > + required: > + - resets > + - reset-names Why? > + - if: > + properties: > + compatible: > + contains: > + const: brcm,bcm7712-pcie > + then: > + properties: > + resets: > + minItems: 3 Again, you do not have 4 items here. > > reset-names: > items: > - const: rescal > + - const: bridge > + - const: swinit > > required: > - resets Best regards, Krzysztof
On Thu, Jul 4, 2024 at 2:40 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 03/07/2024 20:02, Jim Quinlan wrote: > > - Update maintainer; Nicolas hasn't been active and it > > makes more sense to have a Broadcom maintainer > > - Add a driver compatible string for the new STB SOC 7712 > > You meant device? Bindings are for hardware. > > > - Add two new resets for the 7712: "bridge", for the > > the bridge between the PCIe core and the memory bus; > > "swinit", the PCIe core reset. > > - Order the compatible strings alphabetically > > - Restructure the reset controllers so that the definitions > > appear first before any rules that govern them. > > Please split cleanups from new device support. > > > > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > > --- > > .../bindings/pci/brcm,stb-pcie.yaml | 44 +++++++++++++++---- > > 1 file changed, 36 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > index 11f8ea33240c..a070f35d28d7 100644 > > --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > > title: Brcmstb PCIe Host Controller > > > > maintainers: > > - - Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > + - Jim Quinlan <james.quinlan@broadcom.com> > > > > properties: > > compatible: > > @@ -16,11 +16,12 @@ properties: > > - brcm,bcm2711-pcie # The Raspberry Pi 4 > > - brcm,bcm4908-pcie > > - brcm,bcm7211-pcie # Broadcom STB version of RPi4 > > - - brcm,bcm7278-pcie # Broadcom 7278 Arm > > - brcm,bcm7216-pcie # Broadcom 7216 Arm > > - - brcm,bcm7445-pcie # Broadcom 7445 Arm > > + - brcm,bcm7278-pcie # Broadcom 7278 Arm > > - brcm,bcm7425-pcie # Broadcom 7425 MIPs > > - brcm,bcm7435-pcie # Broadcom 7435 MIPs > > + - brcm,bcm7445-pcie # Broadcom 7445 Arm > > + - brcm,bcm7712-pcie # STB sibling SOC of Raspberry Pi 5 > > > > reg: > > maxItems: 1 > > @@ -95,6 +96,20 @@ properties: > > minItems: 1 > > maxItems: 3 > > > > + resets: > > + items: > > + - description: reset for phy calibration > > + - description: reset for PCIe/CPU bus bridge > > + - description: reset for soft PCIe core reset > > + - description: reset for PERST# PCIe signal > > This won't work and I doubt you tested your code. You miss minItems. I did test my code and there were no errors. I perform the following test: make ARCH=arm64 dt_binding_check DT_CHECKER_FLAGS=-m DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml Is this incorrect? > > > + > > + reset-names: > > + items: > > + - const: rescal > > + - const: bridge > > + - const: swinit > > + - const: perst > > This does not match what you have in conditional, so just keep min and > max Items here. I'm not sure what you mean. One chips uses a single reset, another chip uses a different single reset, and the third (7712) uses three of the four resets. I was instructed to separate the descriptions from the rules, or at least that's what I thought I was asked. > > > > + > > required: > > - compatible > > - reg > > @@ -118,13 +133,10 @@ allOf: > > then: > > properties: > > resets: > > - items: > > - - description: reset controller handling the PERST# signal > > - > > + minItems: 1 > > maxItems instead. Why three resets should be valid? See above. Note that I was just instructed to separate the rules from the descriptions. In doing so I placed all of the reset descripts on the top and then the rules below. There are four possible resets but no single chip uses all of them and three chips use one or three of them. Please advise. Thanks, Jim Quinlan Broadcom STB/CM > > > > reset-names: > > items: > > - const: perst > > - > > required: > > - resets > > - reset-names > > @@ -136,12 +148,28 @@ allOf: > > then: > > properties: > > resets: > > + minItems: 1 > > + reset-names: > > items: > > - - description: phandle pointing to the RESCAL reset controller > > + - const: rescal > > + required: > > + - resets > > + - reset-names > > Why? > > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: brcm,bcm7712-pcie > > + then: > > + properties: > > + resets: > > + minItems: 3 > > Again, you do not have 4 items here. > > > > > reset-names: > > items: > > - const: rescal > > + - const: bridge > > + - const: swinit > > > > required: > > - resets > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml index 11f8ea33240c..a070f35d28d7 100644 --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Brcmstb PCIe Host Controller maintainers: - - Nicolas Saenz Julienne <nsaenzjulienne@suse.de> + - Jim Quinlan <james.quinlan@broadcom.com> properties: compatible: @@ -16,11 +16,12 @@ properties: - brcm,bcm2711-pcie # The Raspberry Pi 4 - brcm,bcm4908-pcie - brcm,bcm7211-pcie # Broadcom STB version of RPi4 - - brcm,bcm7278-pcie # Broadcom 7278 Arm - brcm,bcm7216-pcie # Broadcom 7216 Arm - - brcm,bcm7445-pcie # Broadcom 7445 Arm + - brcm,bcm7278-pcie # Broadcom 7278 Arm - brcm,bcm7425-pcie # Broadcom 7425 MIPs - brcm,bcm7435-pcie # Broadcom 7435 MIPs + - brcm,bcm7445-pcie # Broadcom 7445 Arm + - brcm,bcm7712-pcie # STB sibling SOC of Raspberry Pi 5 reg: maxItems: 1 @@ -95,6 +96,20 @@ properties: minItems: 1 maxItems: 3 + resets: + items: + - description: reset for phy calibration + - description: reset for PCIe/CPU bus bridge + - description: reset for soft PCIe core reset + - description: reset for PERST# PCIe signal + + reset-names: + items: + - const: rescal + - const: bridge + - const: swinit + - const: perst + required: - compatible - reg @@ -118,13 +133,10 @@ allOf: then: properties: resets: - items: - - description: reset controller handling the PERST# signal - + minItems: 1 reset-names: items: - const: perst - required: - resets - reset-names @@ -136,12 +148,28 @@ allOf: then: properties: resets: + minItems: 1 + reset-names: items: - - description: phandle pointing to the RESCAL reset controller + - const: rescal + required: + - resets + - reset-names + - if: + properties: + compatible: + contains: + const: brcm,bcm7712-pcie + then: + properties: + resets: + minItems: 3 reset-names: items: - const: rescal + - const: bridge + - const: swinit required: - resets
- Update maintainer; Nicolas hasn't been active and it makes more sense to have a Broadcom maintainer - Add a driver compatible string for the new STB SOC 7712 - Add two new resets for the 7712: "bridge", for the the bridge between the PCIe core and the memory bus; "swinit", the PCIe core reset. - Order the compatible strings alphabetically - Restructure the reset controllers so that the definitions appear first before any rules that govern them. Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> --- .../bindings/pci/brcm,stb-pcie.yaml | 44 +++++++++++++++---- 1 file changed, 36 insertions(+), 8 deletions(-)