diff mbox series

[v4,02/12] dt-bindings: PCI: brcmstb: Add 7712 SoC description

Message ID 20240716213131.6036-3-james.quinlan@broadcom.com
State Changes Requested
Headers show
Series PCI: brcnstb: Enable STB 7712 SOC | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Jim Quinlan July 16, 2024, 9:31 p.m. UTC
This adds the description for the 7712 SoC, a Broadcom
STB sibling chip of the RPi 5.  Two new reset controllers
are described.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 .../bindings/pci/brcm,stb-pcie.yaml           | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

Comments

Krzysztof Kozlowski July 17, 2024, 6:52 a.m. UTC | #1
On 16/07/2024 23:31, Jim Quinlan wrote:
> This adds the description for the 7712 SoC, a Broadcom
> STB sibling chip of the RPi 5.  Two new reset controllers
> are described.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> ---
>  .../bindings/pci/brcm,stb-pcie.yaml           | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> index 692f7ed7c98e..90683a0df2c5 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> @@ -21,6 +21,7 @@ properties:
>            - brcm,bcm7425-pcie # Broadcom 7425 MIPs
>            - brcm,bcm7435-pcie # Broadcom 7435 MIPs
>            - brcm,bcm7445-pcie # Broadcom 7445 Arm
> +          - brcm,bcm7712-pcie # Broadcom STB sibling of Rpi 5
>  
>    reg:
>      maxItems: 1
> @@ -100,12 +101,16 @@ properties:
>      items:
>        - description: reset for external PCIe PERST# signal # perst
>        - description: reset for phy reset calibration       # rescal
> +      - description: reset for PCIe/CPU bus bridge         # bridge
> +      - description: reset for soft PCIe core reset        # swinit
>  
>    reset-names:
>      minItems: 1
>      items:
>        - const: perst
>        - const: rescal
> +      - const: bridge
> +      - const: swinit

This does not match at all what you have in allOf:if:then section.

>  
>  required:
>    - compatible
> @@ -159,6 +164,27 @@ allOf:
>          - resets
>          - reset-names
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: brcm,bcm7712-pcie
> +    then:
> +      properties:
> +        resets:
> +          minItems: 3
> +          maxItems: 3
> +
> +        reset-names:
> +          items:
> +            - const: rescal

Look - here it is rescal. Before you said it must be perst.

https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132

Best regards,
Krzysztof
Jim Quinlan July 23, 2024, 9:03 p.m. UTC | #2
On Wed, Jul 17, 2024 at 2:53 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 16/07/2024 23:31, Jim Quinlan wrote:
> > This adds the description for the 7712 SoC, a Broadcom
> > STB sibling chip of the RPi 5.  Two new reset controllers
> > are described.
> >
> > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> > ---
> >  .../bindings/pci/brcm,stb-pcie.yaml           | 26 +++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > index 692f7ed7c98e..90683a0df2c5 100644
> > --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > @@ -21,6 +21,7 @@ properties:
> >            - brcm,bcm7425-pcie # Broadcom 7425 MIPs
> >            - brcm,bcm7435-pcie # Broadcom 7435 MIPs
> >            - brcm,bcm7445-pcie # Broadcom 7445 Arm
> > +          - brcm,bcm7712-pcie # Broadcom STB sibling of Rpi 5
> >
> >    reg:
> >      maxItems: 1
> > @@ -100,12 +101,16 @@ properties:
> >      items:
> >        - description: reset for external PCIe PERST# signal # perst
> >        - description: reset for phy reset calibration       # rescal
> > +      - description: reset for PCIe/CPU bus bridge         # bridge
> > +      - description: reset for soft PCIe core reset        # swinit
> >
> >    reset-names:
> >      minItems: 1
> >      items:
> >        - const: perst
> >        - const: rescal
> > +      - const: bridge
> > +      - const: swinit
>
> This does not match at all what you have in allOf:if:then section.
>
> >
> >  required:
> >    - compatible
> > @@ -159,6 +164,27 @@ allOf:
> >          - resets
> >          - reset-names
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: brcm,bcm7712-pcie
> > +    then:
> > +      properties:
> > +        resets:
> > +          minItems: 3
> > +          maxItems: 3
> > +
> > +        reset-names:
> > +          items:
> > +            - const: rescal
>
> Look - here it is rescal. Before you said it must be perst.
>
> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132

Hello Krzysztof,

The difference between my commits and the above example is that the
above example has no "desc" line(s) to describe the clocks  -- How
would you add this?  Or are you okay with (a) no description or (b)
using a "#comment..." next to the clock's name?

Regards,
Jim Quinlan
Broadcom STB/CM
>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski July 24, 2024, 6:02 a.m. UTC | #3
On 23/07/2024 23:03, Jim Quinlan wrote:
> On Wed, Jul 17, 2024 at 2:53 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 16/07/2024 23:31, Jim Quinlan wrote:
>>> This adds the description for the 7712 SoC, a Broadcom
>>> STB sibling chip of the RPi 5.  Two new reset controllers
>>> are described.
>>>
>>> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
>>> ---
>>>  .../bindings/pci/brcm,stb-pcie.yaml           | 26 +++++++++++++++++++
>>>  1 file changed, 26 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
>>> index 692f7ed7c98e..90683a0df2c5 100644
>>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
>>> @@ -21,6 +21,7 @@ properties:
>>>            - brcm,bcm7425-pcie # Broadcom 7425 MIPs
>>>            - brcm,bcm7435-pcie # Broadcom 7435 MIPs
>>>            - brcm,bcm7445-pcie # Broadcom 7445 Arm
>>> +          - brcm,bcm7712-pcie # Broadcom STB sibling of Rpi 5
>>>
>>>    reg:
>>>      maxItems: 1
>>> @@ -100,12 +101,16 @@ properties:
>>>      items:
>>>        - description: reset for external PCIe PERST# signal # perst
>>>        - description: reset for phy reset calibration       # rescal
>>> +      - description: reset for PCIe/CPU bus bridge         # bridge
>>> +      - description: reset for soft PCIe core reset        # swinit
>>>
>>>    reset-names:
>>>      minItems: 1
>>>      items:
>>>        - const: perst
>>>        - const: rescal
>>> +      - const: bridge
>>> +      - const: swinit
>>
>> This does not match at all what you have in allOf:if:then section.
>>
>>>
>>>  required:
>>>    - compatible
>>> @@ -159,6 +164,27 @@ allOf:
>>>          - resets
>>>          - reset-names
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: brcm,bcm7712-pcie
>>> +    then:
>>> +      properties:
>>> +        resets:
>>> +          minItems: 3
>>> +          maxItems: 3
>>> +
>>> +        reset-names:
>>> +          items:
>>> +            - const: rescal
>>
>> Look - here it is rescal. Before you said it must be perst.
>>
>> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132
> 
> Hello Krzysztof,
> 
> The difference between my commits and the above example is that the
> above example has no "desc" line(s) to describe the clocks  -- How

Which does not really matter to illustrate the concept where you define
the widest constraints and where you narrow them.

> would you add this?  Or are you okay with (a) no description or (b)
> using a "#comment..." next to the clock's name?



Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 692f7ed7c98e..90683a0df2c5 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -21,6 +21,7 @@  properties:
           - brcm,bcm7425-pcie # Broadcom 7425 MIPs
           - brcm,bcm7435-pcie # Broadcom 7435 MIPs
           - brcm,bcm7445-pcie # Broadcom 7445 Arm
+          - brcm,bcm7712-pcie # Broadcom STB sibling of Rpi 5
 
   reg:
     maxItems: 1
@@ -100,12 +101,16 @@  properties:
     items:
       - description: reset for external PCIe PERST# signal # perst
       - description: reset for phy reset calibration       # rescal
+      - description: reset for PCIe/CPU bus bridge         # bridge
+      - description: reset for soft PCIe core reset        # swinit
 
   reset-names:
     minItems: 1
     items:
       - const: perst
       - const: rescal
+      - const: bridge
+      - const: swinit
 
 required:
   - compatible
@@ -159,6 +164,27 @@  allOf:
         - resets
         - reset-names
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,bcm7712-pcie
+    then:
+      properties:
+        resets:
+          minItems: 3
+          maxItems: 3
+
+        reset-names:
+          items:
+            - const: rescal
+            - const: bridge
+            - const: swinit
+
+      required:
+        - resets
+        - reset-names
+
 unevaluatedProperties: false
 
 examples: