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

On Mon, Aug 12, 2019 at 7:32 PM Leonard Crestez [off-list ref] wrote:

On 8/12/2019 10:47 PM, Rob Herring wrote:

quoted

On Mon, Aug 12, 2019 at 12:49 PM Leonard Crestez [off-list ref] wrote:

quoted

quoted

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?

Virtual nodes are not OK.

quoted

quoted

+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.

Different h/w blocks should have different compatibles. GPV is an Arm
thing and I'd expect FlexNOC to be different.

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

Irrelevant to the binding...

quoted

quoted

+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?

Yes. The main exception is if various SoCs are just packaging,
binning, or fuse differences.

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

Right. Order matters.

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

Looks correct.

Rob