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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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