On Mon, Aug 12, 2019 at 7:32 PM Leonard Crestez <leonard.crestez@xxxxxxx> wrote:
>
> On 8/12/2019 10:47 PM, Rob Herring wrote:
> > On Mon, Aug 12, 2019 at 12:49 PM Leonard Crestez <leonard.crestez@xxxxxxx> 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?
Virtual nodes are not OK.
>
> >> +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...
>
> >> +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
