diff mbox series

[v4,2/2] dt-bindings: mips: brcm: convert Broadcom SoCs to schema

Message ID 20221001043855.933528-3-sergio.paracuellos@gmail.com (mailing list archive)
State Superseded
Headers show
Series dt-bindings: mips: add CPU bindings for MIPS architecture | expand

Commit Message

Sergio Paracuellos Oct. 1, 2022, 4:38 a.m. UTC 
Convert Broadcom cable/DSL/settop platforms to YAML schema.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../devicetree/bindings/mips/brcm/soc.yaml    | 110 ++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mips/brcm/soc.yaml

Comments

Krzysztof Kozlowski Oct. 1, 2022, 9:47 a.m. UTC | #1 
On 01/10/2022 06:38, Sergio Paracuellos wrote:
> Convert Broadcom cable/DSL/settop platforms to YAML schema.

Where is the conversion? Conversion is remove+add, but there is no
removal here.

> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../devicetree/bindings/mips/brcm/soc.yaml    | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mips/brcm/soc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mips/brcm/soc.yaml b/Documentation/devicetree/bindings/mips/brcm/soc.yaml
> new file mode 100644
> index 000000000000..0ddf1fe51802
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mips/brcm/soc.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mips/brcm/soc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom cable/DSL/settop platforms Device Tree Bindings

Drop "Device Tree Bindings"

> +
> +maintainers:
> +  - Hauke Mehrtens <hauke@hauke-m.de>
> +  - Rafał Miłecki <zajec5@gmail.com>
> +
> +description: |
> +    Boards Broadcom cable/DSL/settop SoC shall have the following properties.
> +    The experimental -viper variants are for running Linux on the 3384's
> +    BMIPS4355 cable modem CPU instead of the BMIPS5000 application processor.
> +
> +properties:
> +  $nodename:
> +    const: '/'

Blank line.

> +  compatible:
> +    oneOf:
> +      - description: Boards with Broadcom bcm3368 SoC
> +        items:
> +          - const: brcm,bcm3368
> +
> +      - description: Boards with Broadcom bcm3384 SoC
> +        items:
> +          - const: brcm,bcm3384

I don't understand what did you want to achieve here. Either you
document SoC or boards. If boards, where are the actual boards? If SoC,
then why calling it boards, why making it oneOf?


> +
> +      - description: Boards with Broadcom bcm33843 SoC
> +        items:
> +          - const: brcm,bcm33843
> +
> +      - description: Boards with Broadcom bcm3384-viper SoC
> +        items:
> +          - const: brcm,bcm3384-viper
> +
> +      - description: Boards with Broadcom bcm33843-viper SoC
> +        items:
> +          - const: brcm,bcm33843-viper
> +
> +      - description: Boards with Broadcom bcm6328 SoC
> +        items:
> +          - const: brcm,bcm6328
> +
> +      - description: Boards with Broadcom bcm6358 SoC
> +        items:
> +          - const: brcm,bcm6358
> +
> +      - description: Boards with Broadcom bcm6362 SoC
> +        items:
> +          - const: brcm,bcm6362
> +
> +      - description: Boards with Broadcom bcm6368 SoC
> +        items:
> +          - const: brcm,bcm6368
> +
> +      - description: Boards with Broadcom bcm63168 SoC
> +        items:
> +          - const: brcm,bcm63168
> +
> +      - description: Boards with Broadcom bcm63268 SoC
> +        items:
> +          - const: brcm,bcm63268
> +
> +      - description: Boards with Broadcom bcm7125 SoC
> +        items:
> +          - const: brcm,bcm7125
> +
> +      - description: Boards with Broadcom bcm7346 SoC
> +        items:
> +          - const: brcm,bcm7346
> +
> +      - description: Boards with Broadcom bcm7358 SoC
> +        items:
> +          - const: brcm,bcm7358
> +
> +      - description: Boards with Broadcom bcm7360 SoC
> +        items:
> +          - const: brcm,bcm7360
> +
> +      - description: Boards with Broadcom bcm7362 SoC
> +        items:
> +          - const: brcm,bcm7362
> +
> +      - description: Boards with Broadcom bcm7420 SoC
> +        items:
> +          - const: brcm,bcm7420
> +
> +      - description: Boards with Broadcom bcm7425 SoC
> +        items:
> +          - const: brcm,bcm7425
> +
> +  cpus:
> +    $ref: "/schemas/mips/cpus.yaml#"

No need for quotes.

> +    unevaluatedProperties: false
> +
> +    properties:
> +      mips-hpt-frequency:
> +        description: This is common to all CPUs in the system so it lives
> +         under the "cpus" node.

You need to describe what is this. Not where it lives. Because where it
lives, we can easily see from the schema.

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +    required:
> +      - mips-hpt-frequency
> +
> +additionalProperties: true
> +
> +...

Best regards,
Krzysztof
Sergio Paracuellos Oct. 1, 2022, 10:35 a.m. UTC | #2 
Hi Krzysztof,

On Sat, Oct 1, 2022 at 11:47 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 01/10/2022 06:38, Sergio Paracuellos wrote:
> > Convert Broadcom cable/DSL/settop platforms to YAML schema.
>
> Where is the conversion? Conversion is remove+add, but there is no
> removal here.

True, removal is in the other patch since removed doc it is more
related with CPUs.
Will redo this commit message.

>
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../devicetree/bindings/mips/brcm/soc.yaml    | 110 ++++++++++++++++++
> >  1 file changed, 110 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mips/brcm/soc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mips/brcm/soc.yaml b/Documentation/devicetree/bindings/mips/brcm/soc.yaml
> > new file mode 100644
> > index 000000000000..0ddf1fe51802
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mips/brcm/soc.yaml
> > @@ -0,0 +1,110 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mips/brcm/soc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom cable/DSL/settop platforms Device Tree Bindings
>
> Drop "Device Tree Bindings"

Understood.

>
> > +
> > +maintainers:
> > +  - Hauke Mehrtens <hauke@hauke-m.de>
> > +  - Rafał Miłecki <zajec5@gmail.com>
> > +
> > +description: |
> > +    Boards Broadcom cable/DSL/settop SoC shall have the following properties.
> > +    The experimental -viper variants are for running Linux on the 3384's
> > +    BMIPS4355 cable modem CPU instead of the BMIPS5000 application processor.
> > +
> > +properties:
> > +  $nodename:
> > +    const: '/'
>
> Blank line.

Ok.

>
> > +  compatible:
> > +    oneOf:
> > +      - description: Boards with Broadcom bcm3368 SoC
> > +        items:
> > +          - const: brcm,bcm3368
> > +
> > +      - description: Boards with Broadcom bcm3384 SoC
> > +        items:
> > +          - const: brcm,bcm3384
>
> I don't understand what did you want to achieve here. Either you
> document SoC or boards. If boards, where are the actual boards? If SoC,
> then why calling it boards, why making it oneOf?

I agree with description should just say "Broadcom bcm3384 SoC", but I
don't understand what is wrong with oneOf here...

>
>
> > +
> > +      - description: Boards with Broadcom bcm33843 SoC
> > +        items:
> > +          - const: brcm,bcm33843
> > +
> > +      - description: Boards with Broadcom bcm3384-viper SoC
> > +        items:
> > +          - const: brcm,bcm3384-viper
> > +
> > +      - description: Boards with Broadcom bcm33843-viper SoC
> > +        items:
> > +          - const: brcm,bcm33843-viper
> > +
> > +      - description: Boards with Broadcom bcm6328 SoC
> > +        items:
> > +          - const: brcm,bcm6328
> > +
> > +      - description: Boards with Broadcom bcm6358 SoC
> > +        items:
> > +          - const: brcm,bcm6358
> > +
> > +      - description: Boards with Broadcom bcm6362 SoC
> > +        items:
> > +          - const: brcm,bcm6362
> > +
> > +      - description: Boards with Broadcom bcm6368 SoC
> > +        items:
> > +          - const: brcm,bcm6368
> > +
> > +      - description: Boards with Broadcom bcm63168 SoC
> > +        items:
> > +          - const: brcm,bcm63168
> > +
> > +      - description: Boards with Broadcom bcm63268 SoC
> > +        items:
> > +          - const: brcm,bcm63268
> > +
> > +      - description: Boards with Broadcom bcm7125 SoC
> > +        items:
> > +          - const: brcm,bcm7125
> > +
> > +      - description: Boards with Broadcom bcm7346 SoC
> > +        items:
> > +          - const: brcm,bcm7346
> > +
> > +      - description: Boards with Broadcom bcm7358 SoC
> > +        items:
> > +          - const: brcm,bcm7358
> > +
> > +      - description: Boards with Broadcom bcm7360 SoC
> > +        items:
> > +          - const: brcm,bcm7360
> > +
> > +      - description: Boards with Broadcom bcm7362 SoC
> > +        items:
> > +          - const: brcm,bcm7362
> > +
> > +      - description: Boards with Broadcom bcm7420 SoC
> > +        items:
> > +          - const: brcm,bcm7420
> > +
> > +      - description: Boards with Broadcom bcm7425 SoC
> > +        items:
> > +          - const: brcm,bcm7425
> > +
> > +  cpus:
> > +    $ref: "/schemas/mips/cpus.yaml#"
>
> No need for quotes.

Understood.

>
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      mips-hpt-frequency:
> > +        description: This is common to all CPUs in the system so it lives
> > +         under the "cpus" node.
>
> You need to describe what is this. Not where it lives. Because where it
> lives, we can easily see from the schema.

I have just copied this from the previous documented bmips text file. I guess
writing the following will be better:

properties:
      mips-hpt-frequency:
        description: MIPS counter high precision timer frequency.
         This is common to all CPUs in the system so it lives
         under the "cpus" node.
        $ref: /schemas/types.yaml#/definitions/uint32

>
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +    required:
> > +      - mips-hpt-frequency
> > +
> > +additionalProperties: true
> > +
> > +...
>
> Best regards,
> Krzysztof
>

Thanks,
    Sergio Paracuellos
Krzysztof Kozlowski Oct. 2, 2022, 8:03 a.m. UTC | #3 
On 01/10/2022 12:35, Sergio Paracuellos wrote:
>>
>>> +  compatible:
>>> +    oneOf:
>>> +      - description: Boards with Broadcom bcm3368 SoC
>>> +        items:
>>> +          - const: brcm,bcm3368
>>> +
>>> +      - description: Boards with Broadcom bcm3384 SoC
>>> +        items:
>>> +          - const: brcm,bcm3384
>>
>> I don't understand what did you want to achieve here. Either you
>> document SoC or boards. If boards, where are the actual boards? If SoC,
>> then why calling it boards, why making it oneOf?
> 
> I agree with description should just say "Broadcom bcm3384 SoC", but I
> don't understand what is wrong with oneOf here...

If you document SoCs, this should be just an enum because it will take
20% of that lines. Much smaller, easier to read.

In the same time (for documenting SoCs) all the descriptions are
redundant. We know that this is "Broadcom bcm33843 SoC" because
compatible is "brcm,bcm33843".

> 
>>
>>
>>> +
>>> +      - description: Boards with Broadcom bcm33843 SoC
>>> +        items:
>>> +          - const: brcm,bcm33843
>>> +

(...)

> 
>>
>>> +    unevaluatedProperties: false
>>> +
>>> +    properties:
>>> +      mips-hpt-frequency:
>>> +        description: This is common to all CPUs in the system so it lives
>>> +         under the "cpus" node.
>>
>> You need to describe what is this. Not where it lives. Because where it
>> lives, we can easily see from the schema.
> 
> I have just copied this from the previous documented bmips text file. I guess
> writing the following will be better:
> 
> properties:
>       mips-hpt-frequency:
>         description: MIPS counter high precision timer frequency.
>          This is common to all CPUs in the system so it lives
>          under the "cpus" node.
>         $ref: /schemas/types.yaml#/definitions/uint32

Much better, thanks.


Best regards,
Krzysztof
Sergio Paracuellos Oct. 2, 2022, 8:55 a.m. UTC | #4 
Hi Krzysztof,

On Sun, Oct 2, 2022 at 10:03 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 01/10/2022 12:35, Sergio Paracuellos wrote:
> >>
> >>> +  compatible:
> >>> +    oneOf:
> >>> +      - description: Boards with Broadcom bcm3368 SoC
> >>> +        items:
> >>> +          - const: brcm,bcm3368
> >>> +
> >>> +      - description: Boards with Broadcom bcm3384 SoC
> >>> +        items:
> >>> +          - const: brcm,bcm3384
> >>
> >> I don't understand what did you want to achieve here. Either you
> >> document SoC or boards. If boards, where are the actual boards? If SoC,
> >> then why calling it boards, why making it oneOf?
> >
> > I agree with description should just say "Broadcom bcm3384 SoC", but I
> > don't understand what is wrong with oneOf here...
>
> If you document SoCs, this should be just an enum because it will take
> 20% of that lines. Much smaller, easier to read.
>
> In the same time (for documenting SoCs) all the descriptions are
> redundant. We know that this is "Broadcom bcm33843 SoC" because
> compatible is "brcm,bcm33843".

I see. Ok. I will convert this into an enum and will drop all the descriptions.

>
> >
> >>
> >>
> >>> +
> >>> +      - description: Boards with Broadcom bcm33843 SoC
> >>> +        items:
> >>> +          - const: brcm,bcm33843
> >>> +
>
> (...)
>
> >
> >>
> >>> +    unevaluatedProperties: false
> >>> +
> >>> +    properties:
> >>> +      mips-hpt-frequency:
> >>> +        description: This is common to all CPUs in the system so it lives
> >>> +         under the "cpus" node.
> >>
> >> You need to describe what is this. Not where it lives. Because where it
> >> lives, we can easily see from the schema.
> >
> > I have just copied this from the previous documented bmips text file. I guess
> > writing the following will be better:
> >
> > properties:
> >       mips-hpt-frequency:
> >         description: MIPS counter high precision timer frequency.
> >          This is common to all CPUs in the system so it lives
> >          under the "cpus" node.
> >         $ref: /schemas/types.yaml#/definitions/uint32
>
> Much better, thanks.

Thanks,
    Sergio Paracuellos
>
>
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mips/brcm/soc.yaml b/Documentation/devicetree/bindings/mips/brcm/soc.yaml
new file mode 100644
index 000000000000..0ddf1fe51802
--- /dev/null
+++ b/Documentation/devicetree/bindings/mips/brcm/soc.yaml
@@ -0,0 +1,110 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mips/brcm/soc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom cable/DSL/settop platforms Device Tree Bindings
+
+maintainers:
+  - Hauke Mehrtens <hauke@hauke-m.de>
+  - Rafał Miłecki <zajec5@gmail.com>
+
+description: |
+    Boards Broadcom cable/DSL/settop SoC shall have the following properties.
+    The experimental -viper variants are for running Linux on the 3384's
+    BMIPS4355 cable modem CPU instead of the BMIPS5000 application processor.
+
+properties:
+  $nodename:
+    const: '/'
+  compatible:
+    oneOf:
+      - description: Boards with Broadcom bcm3368 SoC
+        items:
+          - const: brcm,bcm3368
+
+      - description: Boards with Broadcom bcm3384 SoC
+        items:
+          - const: brcm,bcm3384
+
+      - description: Boards with Broadcom bcm33843 SoC
+        items:
+          - const: brcm,bcm33843
+
+      - description: Boards with Broadcom bcm3384-viper SoC
+        items:
+          - const: brcm,bcm3384-viper
+
+      - description: Boards with Broadcom bcm33843-viper SoC
+        items:
+          - const: brcm,bcm33843-viper
+
+      - description: Boards with Broadcom bcm6328 SoC
+        items:
+          - const: brcm,bcm6328
+
+      - description: Boards with Broadcom bcm6358 SoC
+        items:
+          - const: brcm,bcm6358
+
+      - description: Boards with Broadcom bcm6362 SoC
+        items:
+          - const: brcm,bcm6362
+
+      - description: Boards with Broadcom bcm6368 SoC
+        items:
+          - const: brcm,bcm6368
+
+      - description: Boards with Broadcom bcm63168 SoC
+        items:
+          - const: brcm,bcm63168
+
+      - description: Boards with Broadcom bcm63268 SoC
+        items:
+          - const: brcm,bcm63268
+
+      - description: Boards with Broadcom bcm7125 SoC
+        items:
+          - const: brcm,bcm7125
+
+      - description: Boards with Broadcom bcm7346 SoC
+        items:
+          - const: brcm,bcm7346
+
+      - description: Boards with Broadcom bcm7358 SoC
+        items:
+          - const: brcm,bcm7358
+
+      - description: Boards with Broadcom bcm7360 SoC
+        items:
+          - const: brcm,bcm7360
+
+      - description: Boards with Broadcom bcm7362 SoC
+        items:
+          - const: brcm,bcm7362
+
+      - description: Boards with Broadcom bcm7420 SoC
+        items:
+          - const: brcm,bcm7420
+
+      - description: Boards with Broadcom bcm7425 SoC
+        items:
+          - const: brcm,bcm7425
+
+  cpus:
+    $ref: "/schemas/mips/cpus.yaml#"
+    unevaluatedProperties: false
+
+    properties:
+      mips-hpt-frequency:
+        description: This is common to all CPUs in the system so it lives
+         under the "cpus" node.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+    required:
+      - mips-hpt-frequency
+
+additionalProperties: true
+
+...