[RFC PATCH 0/3] Convert imx6 clock to device tree

On Thu, Nov 24, 2011 at 09:49:23AM +0100, Sascha Hauer wrote:

On Thu, Nov 24, 2011 at 10:59:03AM +0800, Shawn Guo wrote:

quoted

On Tue, Nov 22, 2011 at 09:23:14AM +0100, Sascha Hauer wrote:

quoted

On Tue, Nov 22, 2011 at 09:48:53AM +0800, Shawn Guo wrote:

quoted

As I promised to Arnd, I will convert imx6 clock code to common clock
framework and in turn device tree at the earliest time.  Here it is.

Very nice ;) I like the patches.

Is there a way to improve the way how the entities find their base
addresses? Currently the C code figures out the base addresses based
on compatible.

May I understand the problem with doing that?

quoted

Maybe we could put the crm clocks and anatop clocks into
two different nodes instead of putting them all directly under clocks{}.

I do not quite understand this.  Are you suggesting we keep using the
static base address definition in C code for ccm and anatop rather than
retrieving them from device tree?  No, we do not want to do this.  So
if we need to get the addresses from device tree anyhow, using
compatible to find the correct node is the best way to me.

What I meant is that currently there are many clock nodes which do have
a register offset associated with them, but the base address has to be
guessed in C code, like this:

quoted

if (of_device_is_compatible(np, "fsl,imx6q-pll-sys")) {
	base = anatop_base;
	ops = &pll_sys_ops;
} else if (of_device_is_compatible(np, "fsl,imx6q-pll-usb")) {
	base = anatop_base;
	ops = &pll_ops;
	powerup_set_bit = true;
} else if (of_device_is_compatible(np, "fsl,imx6q-pll-av")) {
	base = anatop_base;
	ops = &pll_av_ops;
} else if (of_device_is_compatible(np, "fsl,imx6q-pll-enet")) {
	base = anatop_base;
	ops = &pll_enet_ops;
} else if (of_device_is_compatible(np, "fsl,imx6q-pll")) {
	base = anatop_base;
	ops = &pll_ops;
} else if (of_device_is_compatible(np, "fsl,imx6q-pfd")) {
	base = anatop_base;
	ops = &pfd_ops;
	gate_set_bit = true;
} else if (of_device_is_compatible(np, "fsl,imx6q-ldb-di-clock")) {
	base = ccm_base;
	ops = &ldb_di_clk_ops;
}

Wouldn't it be possible to arrange the devicetree like this:

Yes, it is possible and actually better.  Thanks for the suggestion.

	ccm at 020c4000 {
		compatible = "fsl,imx6q-ccm";
		reg = <0x020c4000 0x4000>;
		interrupts = <0 87 0x04 0 88 0x04>;
		clocks {
			/* put ccm clocks here */
		}
	}

	anatop at 020c8000 {
		compatible = "fsl,imx6q-anatop";
		reg = <0x020c8000 0x1000>;
		interrupts = <0 49 0x04 0 54 0x04 0 127 0x04>;
		clocks {
			/* put anatop clocks here */
		}
	}

Of course this has a high cost on indention levels.

I think I will drop node 'clocks' and list those clock nodes directly
under ccm and anatop nodes to save one level indention.

-- 
Regards,
Shawn