Re: [PATCHv3 34/35] ARM: dts: dra7: add system control module node

[Tony Lindgren <tony@atomide.com>]

* Tero Kristo <t-kristo@ti.com> [150311 12:57]:
> On 03/11/2015 09:26 PM, Tony Lindgren wrote:
> >* Tero Kristo <t-kristo@ti.com> [150311 12:09]:
> >>On 03/11/2015 07:17 PM, Tony Lindgren wrote:
> >>>Hi Tero,
> >>>
> >>>* Tero Kristo <t-kristo@ti.com> [150225 11:09]:
> >>>>Add node for system control module, and move all the existing system
> >>>>control IO space users under this new node as its children. A new node
> >>>>for scm_conf area is also added.
> >>>...
> >>>
> >>>>--- a/arch/arm/boot/dts/dra7.dtsi
> >>>>+++ b/arch/arm/boot/dts/dra7.dtsi
> >>>>@@ -203,26 +203,47 @@
> >>>>  			};
> >>>>  		};
> >>>>
> >>>>+		scm: scm@4a002000 {
> >>>>+			compatible = "ti,dra7-ctrl", "simple-bus";
> >>>>+			reg = <0x4a002000 0x1400>,
> >>>>+			      <0x4a003400 0x600>,
> >>>>+			      <0x4ae0c000 0x600>;
> >>>>+			#address-cells = <2>;
> >>>>+			#size-cells = <1>;
> >>>>+			ranges = <0 0 0x4a002000 0x1400>,
> >>>>+				 <1 0 0x4a003400 0x600>,
> >>>>+				 <2 0 0x4ae0c000 0x600>;
> >>>>+
> >>>>+			scm_conf: tisyscon@0,0 {
> >>>>+				compatible = "syscon";
> >>>>+				reg = <0 0x0 0x1400>;
> >>>>+				#address-cells = <1>;
> >>>>+				#size-cells = <1>;
> >>>>+			};
> >>>>+
> >>>>+			dra7_pmx_core: pinmux@1,0 {
> >>>>+				compatible = "ti,dra7-padconf",
> >>>>+					     "pinctrl-single";
> >>>>+				reg = <1 0x0 0x0464>;
> >>>>+				#address-cells = <1>;
> >>>>+				#size-cells = <0>;
> >>>>+				#interrupt-cells = <1>;
> >>>>+				interrupt-controller;
> >>>>+				pinctrl-single,register-width = <32>;
> >>>>+				pinctrl-single,function-mask = <0x3fffffff>;
> >>>>+			};
> >>>>+		};
> >>>
> >>>Wouldn't it make more sense to have separate device_scm, core_scm and
> >>>wkup_scm instead of stuffing multiple ranges here?
> >>>
> >>>Or are there other reasons for the multiple ranges?
> >>
> >>Yea that was the alternative I was thinking about, I ended up with this for
> >>some reason. I think personally I liked having them all under the same SCM
> >>part, because they are nicely grouped then, and well, its the same system
> >>control part in the chip. We can split it up easily of course. Should we
> >>have a higher level scm part and then have core_scm and wkup_scm under this
> >>followed by the sub-functions, or just drop the top level scm part
> >>completely?
> >
> >Well I'd model it after the hardware so we can have one or more scm driver
> >instances managing the clock for those blocks. If we squash them together,
> >we won't have a chance to pass interrupts and clocks device tree property
> >to the right driver instance. And for example 5432 TRM has them as separate
> >devices in "Figure 18-1. Control Module Overview".
> >
> >I don't think we need the top level scm to group them under, these are all
> >connected seprately to the interconnect, right?
> 
> Yea, can't really think of any real need for the top-level node.
> 
> >
> >>This same question applies to omap4 + omap5 also. In some part for omap3
> >>also, as it also has pmx_core + pmx_wkup separately, even if they are part
> >>of the same register space.
> >>
> >>Anyway, just a political decision from your side, I am fine either way. :)
> >
> >OK thanks for confirming that, to me it makes sense to set them up as
> >separate instances then.
> 
> All right, you got fair points there, I'll rework this for next revision of
> the set. Had a quick look at OMAP3 TRM and it is also basically listing
> these as separate instances also, so I'll change all OMAP3+.

Oh OK.

Tony