Thread (38 messages) 38 messages, 5 authors, 2024-07-13

Re: [PATCH v2 01/12] dt-bindings: PCI: Add Broadcom STB 7712 SOC, update maintainer

From: Jim Quinlan <hidden>
Date: 2024-07-12 20:13:28
Also in: linux-devicetree, linux-pci, lkml

On Sun, Jul 7, 2024 at 7:58 AM Krzysztof Kozlowski [off-list ref] wrote:
On 05/07/2024 22:02, Jim Quinlan wrote:
quoted
On Thu, Jul 4, 2024 at 2:40 AM Krzysztof Kozlowski [off-list ref] wrote:
quoted
On 03/07/2024 20:02, Jim Quinlan wrote:
quoted
- 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.
quoted
- 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.
quoted
Signed-off-by: Jim Quinlan <redacted>
---
 .../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?
That's correct and you are right - it passes the checks. Recent dtschema
changed the logic behind this. I am not sure if the new approach will
stay, I would find explicit minItems here more obvious and readable, so:
resets:
  minItems: 1
  items:
    - .........
    - .........
    - .........
    - .........

quoted
quoted
quoted
+
+  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.
Your conditional in allOf:if:then has different order.
Different order then what, and ordering by chip or by reset name?
quoted
I was instructed to separate the descriptions from the rules, or at
least that's what I thought I was asked.
quoted
quoted
+
 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.
I don't understand that explanation. Why this particular variant works
with 1, 2, 3 or 4 resets in the same time?
What do you mean in the "same time"?  The resets are just not present
in most of our PCIe HW.  In two chips there is only 1 reset in the HW,
and in the 7712 there are 3 resets in the HW.   You asked me to
describe all of the resets first at the top level and I have done
that.  But none of our chips ever use all four.

Regards,
Jim Quinlan
Broadcom STB/CM

Constraints are supposed to be precise / exact.

Best regards,
Krzysztof

Attachments

  • smime.p7s [application/pkcs7-signature] 4210 bytes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help