Re: [PATCH 1/2] ARM: dts: add device tree source for imx6sx SoC

+       intc: interrupt-controller@00a01000 {
+               compatible = "arm,cortex-a9-gic";
+               #interrupt-cells = <3>;
+               #address-cells = <1>;
+               #size-cells = <1>;

Any reason for the #address-cells and #size-cells? The node has no
children, and I don't see an interrupt-map.

+               interrupt-controller;
+               reg = <0x00a01000 0x1000>,
+                     <0x00a00100 0x100>;
+       };
+
+       clocks {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               ckil {
+                       compatible = "fsl,imx-ckil", "fixed-clock";
+                       clock-frequency = <32768>;
+               };
+
+               osc {
+                       compatible = "fsl,imx-osc", "fixed-clock";
+                       clock-frequency = <24000000>;
+               };
+       };

What is this clocks container node? It has no compatible property. The
children have no reg entries and thus #address-cells and #size cells are
useless.

Why have this at all? Why not put the clocks directly under the root?

[...]

+                       anatop: anatop@020c8000 {
+                               compatible = "fsl,imx6sx-anatop", "fsl,imx6q-anatop",
+                                               "syscon", "simple-bus";

NAK. That compatible list makes _no_ sense. Why are both simple-bus and
syscon there?

+                               reg = <0x020c8000 0x1000>;
+                               interrupts = <0 49 0x04 0 54 0x04 0 127 0x04>;

Please bracket list entries individually. It's not clear where one
interrupt ends and the next begins.

+                       };
+
+                       snvs@020cc000 {
+                               compatible = "fsl,sec-v4.0-mon", "simple-bus";
+                               #address-cells = <1>;
+                               #size-cells = <1>;
+                               ranges = <0 0x020cc000 0x4000>;
+
+                               snvs-rtc-lp@34 {
+                                       compatible = "fsl,sec-v4.0-mon-rtc-lp";
+                                       reg = <0x34 0x58>;
+                                       interrupts = <0 19 0x04 0 20 0x04>;
+                               };
+                       };

The binding document mentions nothing about fsl,sec-v4.0-mon being a
bus, let alone one compatible with simple-bus. Are the child nodes
usable in isolation, even if the parent node weren't probed?

+                       sdma: sdma@020ec000 {
+                               compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
+                               reg = <0x020ec000 0x4000>;
+                               interrupts = <0 2 0x04>;
+                               clocks = <&clks IMX6SX_CLK_SDMA>, <&clks IMX6SX_CLK_SDMA>;
+                               clock-names = "ipg", "ahb";
+                               #dma-cells = <3>;
+                               fsl,sdma-ram-script-name = "imx/sdma/sdma-imx6sx.bin";

This is insane. Why is this path in the DT at all?

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html