[PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi

[PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi

[Andrew Lunn]

From: Michael Walle <michael@walle.cc>

Populate the devices with auxdata to set the device names which are used by
clkdev to lookup the clocks.

Signed-off-by: Michael Walle <micheal@walle.cc>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/kirkwood.dtsi   |    9 +++++++++
 arch/arm/mach-kirkwood/board-dt.c |    8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index 8eab7c4..7d282ab 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -42,6 +42,15 @@
 			interrupts = <53>;
 		};
 
+		spi at 10600 {
+			compatible = "marvell,orion-spi";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			cell-index = <0>;
+			reg = <0x10600 0x28>;
+			status = "disabled";
+		};
+
 		nand at 3000000 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index fa51586..0942139 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -26,6 +26,11 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
 	{ }
 };
 
+struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
+	OF_DEV_AUXDATA("marvell,orion-spi", 0xf1010600, "orion_spi.0", NULL),
+	{},
+};
+
 static void __init kirkwood_dt_init(void)
 {
 	pr_info("Kirkwood: %s, TCLK=%d.\n", kirkwood_id(), kirkwood_tclk);
@@ -69,7 +74,8 @@ static void __init kirkwood_dt_init(void)
 	if (of_machine_is_compatible("raidsonic,ib-nas62x0"))
 		ib62x0_init();
 
-	of_platform_populate(NULL, kirkwood_dt_match_table, NULL, NULL);
+	of_platform_populate(NULL, kirkwood_dt_match_table,
+			     kirkwood_auxdata_lookup, NULL);
 }
 
 static const char *kirkwood_dt_board_compat[] = {
-- 
1.7.10

[PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi

[Jason Cooper]

On Sun, Jun 10, 2012 at 12:31:57PM +0200, Andrew Lunn wrote:
> From: Michael Walle <michael@walle.cc>
> 
> Populate the devices with auxdata to set the device names which are used by
> clkdev to lookup the clocks.
> 
> Signed-off-by: Michael Walle <micheal@walle.cc>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  arch/arm/boot/dts/kirkwood.dtsi   |    9 +++++++++
>  arch/arm/mach-kirkwood/board-dt.c |    8 +++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
> index 8eab7c4..7d282ab 100644
> --- a/arch/arm/boot/dts/kirkwood.dtsi
> +++ b/arch/arm/boot/dts/kirkwood.dtsi
> @@ -42,6 +42,15 @@
>  			interrupts = <53>;
>  		};
>  
> +		spi at 10600 {
> +			compatible = "marvell,orion-spi";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			cell-index = <0>;
> +			reg = <0x10600 0x28>;
> +			status = "disabled";
> +		};
> +
>  		nand at 3000000 {
>  			#address-cells = <1>;
>  			#size-cells = <1>;
> diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
> index fa51586..0942139 100644
> --- a/arch/arm/mach-kirkwood/board-dt.c
> +++ b/arch/arm/mach-kirkwood/board-dt.c
> @@ -26,6 +26,11 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
>  	{ }
>  };
>  
> +struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
> +	OF_DEV_AUXDATA("marvell,orion-spi", 0xf1010600, "orion_spi.0", NULL),

Isn't this ---------------------------------^^^^^^^^^^ defined somewhere?

This is done about 1/4 of the time (56 / 187) in the kernel.  But
honestly, I'd prefer it to be named.

Other than that,

Acked-by: Jason Cooper <jason@lakedaemon.net>

> +	{},
> +};
> +
>  static void __init kirkwood_dt_init(void)
>  {
>  	pr_info("Kirkwood: %s, TCLK=%d.\n", kirkwood_id(), kirkwood_tclk);
> @@ -69,7 +74,8 @@ static void __init kirkwood_dt_init(void)
>  	if (of_machine_is_compatible("raidsonic,ib-nas62x0"))
>  		ib62x0_init();
>  
> -	of_platform_populate(NULL, kirkwood_dt_match_table, NULL, NULL);
> +	of_platform_populate(NULL, kirkwood_dt_match_table,
> +			     kirkwood_auxdata_lookup, NULL);
>  }
>  
>  static const char *kirkwood_dt_board_compat[] = {
> -- 
> 1.7.10
> 

[PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi

[Andrew Lunn]

> > +		spi at 10600 {
> > +			compatible = "marvell,orion-spi";
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			cell-index = <0>;
> > +			reg = <0x10600 0x28>;
> > +			status = "disabled";
> > +		};
> > +

> > index fa51586..0942139 100644
> > --- a/arch/arm/mach-kirkwood/board-dt.c
> > +++ b/arch/arm/mach-kirkwood/board-dt.c
> > @@ -26,6 +26,11 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
> >  	{ }
> >  };
> >  
> > +struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
> > +	OF_DEV_AUXDATA("marvell,orion-spi", 0xf1010600, "orion_spi.0", NULL),
> 
> Isn't this ---------------------------------^^^^^^^^^^ defined somewhere?

Hi Jason

Yes, there is a #define for this. However, in this case, i think the
number actually tells you more. The dtsi file has the number, not some
symbolic representation. Also, some platforms, e.g. Dove, have more
than one spi controller. Having the number there makes it easier to
see the mapping between the node in DT and the name used in the
platform device. If we use the #define it means making an indirection
via kirkwood.h for a human to see the mapping.

However, i can change it...

	 Andrew

[PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi

[Jason Cooper]

On Tue, Jun 12, 2012 at 02:04:33PM +0200, Andrew Lunn wrote:
> > > +		spi at 10600 {
> > > +			compatible = "marvell,orion-spi";
> > > +			#address-cells = <1>;
> > > +			#size-cells = <0>;
> > > +			cell-index = <0>;
> > > +			reg = <0x10600 0x28>;
> > > +			status = "disabled";
> > > +		};
> > > +
> 
> > > index fa51586..0942139 100644
> > > --- a/arch/arm/mach-kirkwood/board-dt.c
> > > +++ b/arch/arm/mach-kirkwood/board-dt.c
> > > @@ -26,6 +26,11 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
> > >  	{ }
> > >  };
> > >  
> > > +struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
> > > +	OF_DEV_AUXDATA("marvell,orion-spi", 0xf1010600, "orion_spi.0", NULL),
> > 
> > Isn't this ---------------------------------^^^^^^^^^^ defined somewhere?
> 
> Yes, there is a #define for this. However, in this case, i think the
> number actually tells you more. The dtsi file has the number, not some
> symbolic representation. Also, some platforms, e.g. Dove, have more
> than one spi controller. Having the number there makes it easier to
> see the mapping between the node in DT and the name used in the
> platform device. If we use the #define it means making an indirection
> via kirkwood.h for a human to see the mapping.
> 
> However, i can change it...

Nope, good point.  If no one else pipes up, leave it as is.

thx,

Jason.

[PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi

[Arnd Bergmann]

On Sunday 10 June 2012, Andrew Lunn wrote:
> @@ -26,6 +26,11 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
>         { }
>  };
>  
> +struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
> +       OF_DEV_AUXDATA("marvell,orion-spi", 0xf1010600, "orion_spi.0", NULL),
> +       {},
> +};
> +
>  static void __init kirkwood_dt_init(void)
>  {

How about instead adding the clkdev for "f1010600.spi" so you don't need the auxdata?

	Arnd

[PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi

[Andrew Lunn]

On Wed, Jun 13, 2012 at 09:10:30PM +0000, Arnd Bergmann wrote:
> On Sunday 10 June 2012, Andrew Lunn wrote:
> > @@ -26,6 +26,11 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
> >         { }
> >  };
> >  
> > +struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
> > +       OF_DEV_AUXDATA("marvell,orion-spi", 0xf1010600, "orion_spi.0", NULL),
> > +       {},
> > +};
> > +
> >  static void __init kirkwood_dt_init(void)
> >  {
> 
> How about instead adding the clkdev for "f1010600.spi" so you don't need the auxdata?

We either have auxdata and clean clock code, or no auxdata and messy
clock code with lots of aliases.

The auxdata is also not limited to the name of the clocks. It allows
me to diff the kernel log for a DT boot and a none DT boot of the same
box and see they are identical. I've found a few typo's that way, and
not been hindered by noise of the devices changing name is useful.

Once we have the clock tree described in DT, then i think it makes
sense to remove these auxdata entries.

      Andrew

[PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi

[Arnd Bergmann]

On Thursday 14 June 2012, Andrew Lunn wrote:
> We either have auxdata and clean clock code, or no auxdata and messy
> clock code with lots of aliases.
> 
> The auxdata is also not limited to the name of the clocks. It allows
> me to diff the kernel log for a DT boot and a none DT boot of the same
> box and see they are identical. I've found a few typo's that way, and
> not been hindered by noise of the devices changing name is useful.
> 
> Once we have the clock tree described in DT, then i think it makes
> sense to remove these auxdata entries.
> 

Ok, fair enough.

	Arnd