Re: [PATCH 2/7] dt-bindings: devfreq: Add bindings for generic imx buses

[Leonard Crestez <leonard.crestez@nxp.com>]

On 8/12/2019 10:47 PM, Rob Herring wrote:
> On Mon, Aug 12, 2019 at 12:49 PM Leonard Crestez <leonard.crestez@nxp.com> wrote:

>> Add initial dt bindings for the interconnects inside i.MX chips.
>> Multiple external IPs are involved but SOC integration means the
>> software controllable interfaces are very similar.
>>
>> +description: |
>> +  The i.MX SoC family has multiple buses for which clock frequency (and sometimes
>> +  voltage) can be adjusted.
>> +
>> +  Some of those buses expose register areas mentioned in the memory maps as GPV
>> +  ("Global Programmers View") but not all. Access to this area might be denied for
>> +  normal world.
>> +
>> +  The buses are based on externally licensed IPs such as ARM NIC-301 and Arteris
>> +  FlexNOC but DT bindings are specific to the integration of these bus
>> +  interconnect IPs into imx SOCs.
> 
> No need to use the interconnect binding?

Separate RFC: https://patchwork.kernel.org/patch/11078673/

The interconnect is represented by a separate "virtual" node which might 
not be OK. There was also a recent RFC from samsung which turns devfreq 
nodes into interconnect providers:
     https://patchwork.kernel.org/cover/11054417/

Is that preferable?

>> +required:
>> +  - compatible
>> +  - clocks
> 
> reg?

This is deliberately optional: for some NICs the GPV register area is 
not exposed in the memory map. This is unusual but an accurate 
description of the hardware.

The current driver doesn't even attempt to map registers, it only 
adjusts the clock.

>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/imx8mm-clock.h>
>> +    noc: noc@32700000 {
>> +            compatible = "fsl,imx8mm-noc", "fsl,imx8m-noc";
> 
> Doesn't match the schema. (Well, it does with 'contains', but
> fsl,imx8mm-noc is not documented.)

I'm confused about how per-SOC compatible strings works with validation. 
There is a rule that every SOC dtsi needs to add soc prefix to all 
device nodes but of_device_id in driver code doesn't need to be updated.

Without using "contains" on the "compatible" property then all 
SOC-specific compatible strings would need to be mentioned in every yaml 
files. Unless I'm missing something this means updating update every 
binding file for each new SOC?

I guess it can be useful because it also validates the compatible 
sequence itself.

For this current example something like this seems to work:

   compatible:
     oneOf:
       - items:
         - enum:
           - fsl,imx8mm-nic
           - fsl,imx8mq-nic
         - const: fsl,imx8m-nic
       - items:
         - enum:
           - fsl,imx8mm-noc
           - fsl,imx8mq-noc
         - const: fsl,imx8m-noc

--
Regards,
Leonard