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

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

On 20/09/2022 07:51, Sergio Paracuellos wrote:
> Hi Krzysztof,
> 
> On Mon, Sep 19, 2022 at 6:08 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> 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).
> 
> Ok so if I am understanding correctly having two schemas is a way to go:
> 
> One for brcm,bmips-cpus.yaml (since there is no compatible, should
> this be a valid name for this?) containing something like:
> 
> properties:
>   $nodename:
>      const: cpus
> 
>      mips-hpt-frequency:
>         $ref: /schemas/types.yaml#/definitions/uint32
>         description: |
>            This is common to all CPUs in the system so it lives
>             under the "cpus" node.
> 
> additionalProperties: true

Almost. Such schema will allow mips-hpt-frequency in each cpus node,
everywhere. On every board and architecture.

You need to limit it per top-level compatibles.

You can also wait a week and maybe Rob will have some ideas.

> 
> examples:
>   - |
>      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>;
>         };
>   };
> 
> And the other as 'cpus.yaml' having:

Yes.

Best regards,
Krzysztof