Hi Krzysztof,
[cc: Hauke Mehrtens and Rafał Miłecki as maintainers for
'Documentation/devicetree/bindings/mips/brcm/']
On Wed, Sep 21, 2022 at 9:51 AM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> 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.
I am also not sure.
>
> Anyway, you did not Cc the actual platform maintainers (Rafał and Hauke).
True, sorry for the inconvenience. Added now to CC list.
>
> >
> > 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
Thanks for the clue.
I'll try to do some tests and come back here later :)
Thanks,
Sergio Paracuellos
>
>
> Best regards,
> Krzysztof
