Re: [PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi
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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi
On Wed, Jun 13, 2012 at 09:10:30PM +, 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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi
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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi
On Tue, Jun 12, 2012 at 02:04:33PM +0200, Andrew Lunn wrote: > > > + spi@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. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi
> > + spi@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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] ARM: kirkwood: use devicetree for orion-spi
On Sun, Jun 10, 2012 at 12:31:57PM +0200, Andrew Lunn wrote: > From: Michael Walle > > Populate the devices with auxdata to set the device names which are used by > clkdev to lookup the clocks. > > Signed-off-by: Michael Walle > Signed-off-by: Andrew Lunn > --- > 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@10600 { > + compatible = "marvell,orion-spi"; > + #address-cells = <1>; > + #size-cells = <0>; > + cell-index = <0>; > + reg = <0x10600 0x28>; > + status = "disabled"; > + }; > + > nand@300 { > #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 > + {}, > +}; > + > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html