Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture

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

On 19/09/2022 15:41, Sergio Paracuellos wrote:
> Hi Krzysztof,
> 
> On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 19/09/2022 14:29, Sergio Paracuellos wrote:
>>>>
>>>> else mips-hpt-frequency: false
>>>>
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>> +
>>>>> +additionalProperties: true
>>>>
>>>> and this is why you did not notice errors...
>>>
>>> Current arch/mips/boot/dts folder dts files are a mess for cpu nodes,
>>> so I set additionalProperties to true and only make required for
>>> 'compatible'. What should be the correct approach?
>>
>> This is okay, but it caused you did not notice errors...
>>
>>>
>>>>
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>> +    cpus {
>>>>> +      #size-cells = <0>;
>>>>> +      #address-cells = <1>;
>>>>> +
>>>>> +      cpu@0 {
>>>>> +        device_type = "cpu";
>>>>> +        compatible = "mips,mips1004Kc";
>>>>> +        reg = <0>;
>>>>> +      };
>>>>> +
>>>>> +      cpu@1 {
>>>>> +        device_type = "cpu";
>>>>> +        compatible = "mips,mips1004Kc";
>>>>> +        reg = <1>;
>>>>> +      };
>>>>> +    };
>>>>> +
>>>>> +  - |
>>>>> +    // Example 2 (BMIPS CPU)
>>>>> +    cpus {
>>>>> +      #address-cells = <1>;
>>>>> +      #size-cells = <0>;
>>>>> +
>>>>> +      mips-hpt-frequency = <150000000>;
>>>>
>>>> Does not match your bindings. Are you sure you tested the patches?
>>>
>>> Yes I did:
>>>
>>> $ make dt_binding_check
>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/mips/cpus.yaml
>>>   LINT    Documentation/devicetree/bindings
>>>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>>>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>>>   DTEX    Documentation/devicetree/bindings/mips/cpus.example.dts
>>>   DTC     Documentation/devicetree/bindings/mips/cpus.example.dtb
>>> ' CHECK   Documentation/devicetree/bindings/mips/cpus.example.dtb
>>>
>>> Can you please point me to a sample of how to make required in a
>>> parent node of cpu@X property 'mips-hpt-frequency' only for some
>>> compatible strings inside the node? What can this be properly
>>> expressed using schema??
>>> I was looking and testing different things for a while without success at all.
>>
>> You either define new schema for /cpus node (and match by name, define
>> children etc) or include it in schema for top-level properties. The
>> first is tricky, because the cpus node does not have compatible (like
>> nvidia,tegra194-ccplex.yaml).
>>
>> The second should work, but then it's a bit cluttered (top-level mixed
>> with cpus).
> 
> I don't know if I am understanding you but maybe it is because my
> explanation about the requirement was not good at all. So let me
> explain a bit better.
> 
> This is the normal way of definition of cpus for BMIPS:

I know, I checked the DTS.

> 
> cpus {
>       #address-cells = <1>;
>       #size-cells = <0>;
> 
>       mips-hpt-frequency = <150000000>;
> 
>       cpu@0 {
>         compatible = "brcm,bmips4350";
>         device_type = "cpu";
>         reg = <0>;
>       };
> 
>       cpu@1 {
>         compatible = "brcm,bmips4350";
>         device_type = "cpu";
>         reg = <1>;
>       };
>     };
> 
> What I need to say in schema is that 'mips-hpt-frequency' must be only
> present if cpu@0 and cpu@1 nodes contain a compatible matching
> brcm,bmips*. In the same cpu@0 or cpu@1 node
> the following below will be sufficient. How can I express the same but
> referring that 'mips-hpt-frequency' must be on the parent node?

As I said you had two ways. In your current patch, I think you cannot.

> Because as it is below the validator complains because
> 'mips-hpt-frequency'
> is not present in cpu@0 and cpu@1 nodes:
> 
> allOf:
>    - if:
>         properties:
>            compatible:
>                enum:
>                    - brcm,bmips3300
>                    - brcm,bmips4350
>                    - brcm,bmips4380
>                    - brcm,bmips5000
>                    - brcm,bmips5200
>      then:
>         required:
>            - mips-hpt-frequency
>      else:
>         properties:
>            mips-hpt-frequency: false
> 

Best regards,
Krzysztof