On 21/09/2022 09:18, Sergio Paracuellos wrote:
> Hi Krzysztof,
>
> On Wed, Sep 21, 2022 at 8:42 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
>>
>> On 20/09/2022 07:51, Sergio Paracuellos wrote:
>>> Hi Krzysztof,
>>>
>>> On Mon, Sep 19, 2022 at 6:08 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@xxxxxxxxxx> 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@xxxxxxxxxx> 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.
>
> Yes, that is what I thought since no compatible to match this is
> included in current node.
>
>>
>> You need to limit it per top-level compatibles.
>
> Any sample of how to do this? So this bmips SoCs use compatible
> strings that are described in:
> https://elixir.bootlin.com/linux/v6.0-rc5/source/Documentation/devicetree/bindings/mips/brcm/soc.txt
Could be something like this:
https://lore.kernel.org/all/20220830065744.161163-2-krzysztof.kozlowski@xxxxxxxxxx/
which is a part of top-level schema or add a new one. The new one will
duplicate the compatibles and parts from that one there, so maybe better
to keep it in top-level?
I am not sure, any suggestions are welcome. Also platform/architecture
maintainers might have their preference to organize it.
Anyway, you did not Cc the actual platform maintainers (Rafał and Hauke).
>
> Can the top level compatible string be used in some way to filter this
> easily from this new 'brcm,bmips-cpus.yaml'
Yes. If schema matches the top level compatible, then in allOf:if:then
you can add restriction to disallow it for other variants:
For example:
https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212
https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml#L152
Best regards,
Krzysztof
