[PATCH 0/7] ARM: mackerel: extended DT support

[PATCH 0/7] ARM: mackerel: extended DT support

[Guennadi Liakhovetski]

The purpose of this patch set is to continue mackerel migration to DT 
booting, while preserving the ability to boot without a DT image. Further 
devices will be migrated from platform data to DT as their drivers become 
ready for such migration.

Guennadi Liakhovetski (7):
  ARM: sh7372: add missing "#interrupt-cells" properties
  ARM: mackerel: include the correct .dtsi file
  ARM: sh7372: support mixed DT and board code interrupt controller
    init
  ARM: sh7372: add clock lookup entries for DT-based devices
  ARM: sh7372: allow boards supporting booting with or without DT
  ARM: mackerel: support booting with or without DT
  ARM: mackerel: add more devices to DT

 arch/arm/boot/dts/sh7372-mackerel.dts   |   98 ++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/sh7372.dtsi           |    2 +
 arch/arm/mach-shmobile/board-mackerel.c |   84 +++++++++++++++++++++------
 arch/arm/mach-shmobile/clock-sh7372.c   |    9 +++
 arch/arm/mach-shmobile/intc-sh7372.c    |    6 ++
 arch/arm/mach-shmobile/setup-sh7372.c   |   17 +++++-
 6 files changed, 194 insertions(+), 22 deletions(-)

-- 
1.7.2.5

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 1/7] ARM: sh7372: add missing "#interrupt-cells" properties

[Guennadi Liakhovetski]

Two of four interrupt controllers in sh7372.dtsi are missing the compulsory
"#interrupt-cells" properties. Fix this.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/boot/dts/sh7372.dtsi |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sh7372.dtsi b/arch/arm/boot/dts/sh7372.dtsi
index da03ee6..00f1645 100644
--- a/arch/arm/boot/dts/sh7372.dtsi
+++ b/arch/arm/boot/dts/sh7372.dtsi
@@ -943,6 +943,7 @@
 			interrupt-controller;
 			#address-cells = <1>;
 			#size-cells = <1>;
+			#interrupt-cells = <1>;
 			ranges;
 
 			reg = <0xe6900000 0x70>;
@@ -1052,6 +1053,7 @@
 			interrupt-controller;
 			#address-cells = <1>;
 			#size-cells = <1>;
+			#interrupt-cells = <1>;
 			ranges;
 
 			reg = <0xe6900008 0x70>;
-- 
1.7.2.5

Re: [PATCH 1/7] ARM: sh7372: add missing "#interrupt-cells" properties

[Simon Horman]

On Fri, Dec 14, 2012 at 05:45:25PM +0100, Guennadi Liakhovetski wrote:
> Two of four interrupt controllers in sh7372.dtsi are missing the compulsory
> "#interrupt-cells" properties. Fix this.

Thanks.

I think that is fixed in the latest INTC DT series that I posted just
a few moments ago "[RFC v7 00/10] ARM: shmobile: DT initialisation of INTC
and GIC".

Sorry for sitting on that code for so long. Could you check if the
problem you saw is resolved?

[PATCH 2/7] ARM: mackerel: include the correct .dtsi file

[Guennadi Liakhovetski]

Mackerel's .dts Device Tree description file should derive from the SoC's
.dtsi, not from skeleton.dtsi directly.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/boot/dts/sh7372-mackerel.dts |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/sh7372-mackerel.dts b/arch/arm/boot/dts/sh7372-mackerel.dts
index 286f0ca..2ede70d 100644
--- a/arch/arm/boot/dts/sh7372-mackerel.dts
+++ b/arch/arm/boot/dts/sh7372-mackerel.dts
@@ -9,7 +9,7 @@
  */
 
 /dts-v1/;
-/include/ "skeleton.dtsi"
+/include/ "sh7372.dtsi"
 
 / {
 	model = "Mackerel (AP4 EVM 2nd)";
-- 
1.7.2.5

Re: [PATCH 2/7] ARM: mackerel: include the correct .dtsi file

[Simon Horman]

On Fri, Dec 14, 2012 at 05:45:26PM +0100, Guennadi Liakhovetski wrote:
> Mackerel's .dts Device Tree description file should derive from the SoC's
> .dtsi, not from skeleton.dtsi directly.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Thanks applied.

I have applied this to a temporary 'boards4' branch which is based on 3.7-rc1.
I will rebase this on 3.8-rc1 once it is released and rename the branch
to the more sane 'boards'.

I have also merged this change into the next branch.

[PATCH 3/7] ARM: sh7372: support mixed DT and board code interrupt controller init

[Guennadi Liakhovetski]

Extend DT interrupt controller initialisation to automatically fall back to
platform data based configuration, if booting without DT. This simplifies
implementing boards, capable of booting in either mode with a single kernel.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/mach-shmobile/intc-sh7372.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/intc-sh7372.c b/arch/arm/mach-shmobile/intc-sh7372.c
index c923518..9c13ecc 100644
--- a/arch/arm/mach-shmobile/intc-sh7372.c
+++ b/arch/arm/mach-shmobile/intc-sh7372.c
@@ -23,6 +23,7 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/sh_intc.h>
+#include <mach/common.h>
 #include <mach/intc.h>
 #include <mach/irqs.h>
 #include <asm/mach-types.h>
@@ -629,6 +630,11 @@ static const struct of_device_id irq_of_match[] __initconst = {
 
 void __init sh7372_init_irq_of(void)
 {
+	if (!of_have_populated_dt()) {
+		sh7372_init_irq();
+		return;
+	}
+
 	of_irq_init(irq_of_match);
 
 	sh7372_init_intc(0xe6940000, 0xe6950000, 0xffd20000, 0xffd50000,
-- 
1.7.2.5

Re: [PATCH 3/7] ARM: sh7372: support mixed DT and board code interrupt controller init

[Simon Horman]

On Fri, Dec 14, 2012 at 05:45:27PM +0100, Guennadi Liakhovetski wrote:
> Extend DT interrupt controller initialisation to automatically fall back to
> platform data based configuration, if booting without DT. This simplifies
> implementing boards, capable of booting in either mode with a single kernel.

Hi Guennadi,

Do you have a case in mind where this will be used?
My thinking until now has been that sh7372_init_irq_of() should only be called
when a board is being initialised using DT.

> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  arch/arm/mach-shmobile/intc-sh7372.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/intc-sh7372.c b/arch/arm/mach-shmobile/intc-sh7372.c
> index c923518..9c13ecc 100644
> --- a/arch/arm/mach-shmobile/intc-sh7372.c
> +++ b/arch/arm/mach-shmobile/intc-sh7372.c
> @@ -23,6 +23,7 @@
>  #include <linux/irq.h>
>  #include <linux/io.h>
>  #include <linux/sh_intc.h>
> +#include <mach/common.h>
>  #include <mach/intc.h>
>  #include <mach/irqs.h>
>  #include <asm/mach-types.h>
> @@ -629,6 +630,11 @@ static const struct of_device_id irq_of_match[] __initconst = {
>  
>  void __init sh7372_init_irq_of(void)
>  {
> +	if (!of_have_populated_dt()) {
> +		sh7372_init_irq();
> +		return;
> +	}
> +
>  	of_irq_init(irq_of_match);
>  
>  	sh7372_init_intc(0xe6940000, 0xe6950000, 0xffd20000, 0xffd50000,
> -- 
> 1.7.2.5
> 

Re: [PATCH 3/7] ARM: sh7372: support mixed DT and board code interrupt controller init

[Guennadi Liakhovetski]

Hi Simon

On Sat, 15 Dec 2012, Simon Horman wrote:

> On Fri, Dec 14, 2012 at 05:45:27PM +0100, Guennadi Liakhovetski wrote:
> > Extend DT interrupt controller initialisation to automatically fall back to
> > platform data based configuration, if booting without DT. This simplifies
> > implementing boards, capable of booting in either mode with a single kernel.
> 
> Hi Guennadi,
> 
> Do you have a case in mind where this will be used?
> My thinking until now has been that sh7372_init_irq_of() should only be called
> when a board is being initialised using DT.

As discussed in follow-ups to another patch from this series, the idea was 
to only have one sh7372_init_irq(_of)() function exported, but it's not 
too important, I'll drop this from v2.

Thanks
Guennadi

> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  arch/arm/mach-shmobile/intc-sh7372.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-shmobile/intc-sh7372.c b/arch/arm/mach-shmobile/intc-sh7372.c
> > index c923518..9c13ecc 100644
> > --- a/arch/arm/mach-shmobile/intc-sh7372.c
> > +++ b/arch/arm/mach-shmobile/intc-sh7372.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/irq.h>
> >  #include <linux/io.h>
> >  #include <linux/sh_intc.h>
> > +#include <mach/common.h>
> >  #include <mach/intc.h>
> >  #include <mach/irqs.h>
> >  #include <asm/mach-types.h>
> > @@ -629,6 +630,11 @@ static const struct of_device_id irq_of_match[] __initconst = {
> >  
> >  void __init sh7372_init_irq_of(void)
> >  {
> > +	if (!of_have_populated_dt()) {
> > +		sh7372_init_irq();
> > +		return;
> > +	}
> > +
> >  	of_irq_init(irq_of_match);
> >  
> >  	sh7372_init_intc(0xe6940000, 0xe6950000, 0xffd20000, 0xffd50000,
> > -- 
> > 1.7.2.5
> > 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 4/7] ARM: sh7372: add clock lookup entries for DT-based devices

[Guennadi Liakhovetski]

When booting with DT, devices are named differently. To get their clocks
additional entries have to be added to the lookup table.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/mach-shmobile/clock-sh7372.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/clock-sh7372.c b/arch/arm/mach-shmobile/clock-sh7372.c
index 430a90f..2471f77 100644
--- a/arch/arm/mach-shmobile/clock-sh7372.c
+++ b/arch/arm/mach-shmobile/clock-sh7372.c
@@ -618,6 +618,7 @@ static struct clk_lookup lookups[] = {
 
 	/* MSTP32 clocks */
 	CLKDEV_DEV_ID("i2c-sh_mobile.2", &mstp_clks[MSTP001]), /* IIC2 */
+	CLKDEV_DEV_ID("fff30000.i2c", &mstp_clks[MSTP001]), /* IIC2 */
 	CLKDEV_DEV_ID("spi_sh_msiof.0", &mstp_clks[MSTP000]), /* MSIOF0 */
 	CLKDEV_DEV_ID("uio_pdrv_genirq.4", &mstp_clks[MSTP131]), /* VEU3 */
 	CLKDEV_DEV_ID("uio_pdrv_genirq.3", &mstp_clks[MSTP130]), /* VEU2 */
@@ -630,6 +631,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh-mipi-dsi.0", &mstp_clks[MSTP118]), /* DSITX0 */
 	CLKDEV_DEV_ID("sh_mobile_lcdc_fb.1", &mstp_clks[MSTP117]), /* LCDC1 */
 	CLKDEV_DEV_ID("i2c-sh_mobile.0", &mstp_clks[MSTP116]), /* IIC0 */
+	CLKDEV_DEV_ID("fff20000.i2c", &mstp_clks[MSTP116]), /* IIC0 */
 	CLKDEV_DEV_ID("sh_mobile_meram.0", &mstp_clks[MSTP113]), /* MERAM */
 	CLKDEV_DEV_ID("uio_pdrv_genirq.5", &mstp_clks[MSTP106]), /* JPU */
 	CLKDEV_DEV_ID("uio_pdrv_genirq.0", &mstp_clks[MSTP101]), /* VPU */
@@ -651,18 +653,25 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh-sci.4", &mstp_clks[MSTP200]), /* SCIFA4 */
 	CLKDEV_DEV_ID("sh_fsi2", &mstp_clks[MSTP328]), /* FSI2 */
 	CLKDEV_DEV_ID("i2c-sh_mobile.1", &mstp_clks[MSTP323]), /* IIC1 */
+	CLKDEV_DEV_ID("e6c20000.i2c", &mstp_clks[MSTP323]), /* IIC1 */
 	CLKDEV_DEV_ID("r8a66597_hcd.0", &mstp_clks[MSTP322]), /* USB0 */
 	CLKDEV_DEV_ID("r8a66597_udc.0", &mstp_clks[MSTP322]), /* USB0 */
 	CLKDEV_DEV_ID("renesas_usbhs.0", &mstp_clks[MSTP322]), /* USB0 */
 	CLKDEV_DEV_ID("sh_flctl.0", &mstp_clks[MSTP315]), /* FLCTL */
 	CLKDEV_DEV_ID("sh_mobile_sdhi.0", &mstp_clks[MSTP314]), /* SDHI0 */
+	CLKDEV_DEV_ID("e6850000.sdhi", &mstp_clks[MSTP314]), /* SDHI0 */
 	CLKDEV_DEV_ID("sh_mobile_sdhi.1", &mstp_clks[MSTP313]), /* SDHI1 */
+	CLKDEV_DEV_ID("e6860000.sdhi", &mstp_clks[MSTP313]), /* SDHI1 */
 	CLKDEV_DEV_ID("sh_mmcif.0", &mstp_clks[MSTP312]), /* MMC */
+	CLKDEV_DEV_ID("e6bd0000.mmcif", &mstp_clks[MSTP312]), /* MMC */
 	CLKDEV_DEV_ID("sh-mipi-dsi.1", &mstp_clks[MSTP423]), /* DSITX1 */
 	CLKDEV_DEV_ID("sh_mobile_sdhi.2", &mstp_clks[MSTP415]), /* SDHI2 */
+	CLKDEV_DEV_ID("e6870000.sdhi", &mstp_clks[MSTP415]), /* SDHI2 */
 	CLKDEV_DEV_ID("sh-mobile-hdmi", &mstp_clks[MSTP413]), /* HDMI */
 	CLKDEV_DEV_ID("i2c-sh_mobile.3", &mstp_clks[MSTP411]), /* IIC3 */
+	CLKDEV_DEV_ID("e6d20000.i2c", &mstp_clks[MSTP411]), /* IIC3 */
 	CLKDEV_DEV_ID("i2c-sh_mobile.4", &mstp_clks[MSTP410]), /* IIC4 */
+	CLKDEV_DEV_ID("e6d30000.i2c", &mstp_clks[MSTP410]), /* IIC4 */
 	CLKDEV_DEV_ID("sh-dma-engine.4", &mstp_clks[MSTP407]), /* USB-DMAC1 */
 	CLKDEV_DEV_ID("r8a66597_hcd.1", &mstp_clks[MSTP406]), /* USB1 */
 	CLKDEV_DEV_ID("r8a66597_udc.1", &mstp_clks[MSTP406]), /* USB1 */
-- 
1.7.2.5

Re: [PATCH 4/7] ARM: sh7372: add clock lookup entries for DT-based devices

[Grant Likely]

On Fri, 14 Dec 2012 17:45:28 +0100, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> When booting with DT, devices are named differently. To get their clocks
> additional entries have to be added to the lookup table.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Please use AUXDATA to manipulate the device name for consistency with
other platforms. When clock bindings get added to the device tree for
this platform then all the AUXDATA declarations can be removed.

g.

Re: [PATCH 4/7] ARM: sh7372: add clock lookup entries for DT-based devices

[Simon Horman]

On Fri, Dec 14, 2012 at 05:45:28PM +0100, Guennadi Liakhovetski wrote:
> When booting with DT, devices are named differently. To get their clocks
> additional entries have to be added to the lookup table.

Thanks applied.

I have applied this to a temporary 'soc5' branch which is based on 3.7-rc1.
I will rebase this on 3.8-rc1 once it is released and rename the branch
to the more sane 'soc'.

I have also merged this change into the next branch.

[PATCH 5/7] ARM: sh7372: allow boards supporting booting with or without DT

[Guennadi Liakhovetski]

For boards booting without DT no changes should be caused by this patch.
When booting with DT, devices, whose drivers support DT probing, will not
be registered.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/mach-shmobile/setup-sh7372.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-shmobile/setup-sh7372.c b/arch/arm/mach-shmobile/setup-sh7372.c
index bbe6e2a..3e6bf3d 100644
--- a/arch/arm/mach-shmobile/setup-sh7372.c
+++ b/arch/arm/mach-shmobile/setup-sh7372.c
@@ -981,9 +981,12 @@ static struct platform_device *sh7372_early_devices[] __initdata = {
 	&tmu01_device,
 };
 
-static struct platform_device *sh7372_late_devices[] __initdata = {
+static struct platform_device *sh7372_late_devices_dt[] __initdata = {
 	&iic0_device,
 	&iic1_device,
+};
+
+static struct platform_device *sh7372_late_devices[] __initdata = {
 	&dma0_device,
 	&dma1_device,
 	&dma2_device,
@@ -1012,13 +1015,11 @@ void __init sh7372_add_standard_devices(void)
 		{ "A3SP", &scif4_device, },
 		{ "A3SP", &scif5_device, },
 		{ "A3SP", &scif6_device, },
-		{ "A3SP", &iic1_device, },
 		{ "A3SP", &dma0_device, },
 		{ "A3SP", &dma1_device, },
 		{ "A3SP", &dma2_device, },
 		{ "A3SP", &usb_dma0_device, },
 		{ "A3SP", &usb_dma1_device, },
-		{ "A4R", &iic0_device, },
 		{ "A4R", &veu0_device, },
 		{ "A4R", &veu1_device, },
 		{ "A4R", &veu2_device, },
@@ -1027,6 +1028,10 @@ void __init sh7372_add_standard_devices(void)
 		{ "A4R", &tmu00_device, },
 		{ "A4R", &tmu01_device, },
 	};
+	struct pm_domain_device domain_devices_dt[] = {
+		{ "A3SP", &iic1_device, },
+		{ "A4R", &iic0_device, },
+	};
 
 	sh7372_init_pm_domains();
 
@@ -1035,9 +1040,15 @@ void __init sh7372_add_standard_devices(void)
 
 	platform_add_devices(sh7372_late_devices,
 			    ARRAY_SIZE(sh7372_late_devices));
+	if (!of_have_populated_dt())
+		platform_add_devices(sh7372_late_devices_dt,
+				     ARRAY_SIZE(sh7372_late_devices_dt));
 
 	rmobile_add_devices_to_domains(domain_devices,
 				       ARRAY_SIZE(domain_devices));
+	if (!of_have_populated_dt())
+		rmobile_add_devices_to_domains(domain_devices_dt,
+					       ARRAY_SIZE(domain_devices_dt));
 }
 
 static void __init sh7372_earlytimer_init(void)
-- 
1.7.2.5

Re: [PATCH 5/7] ARM: sh7372: allow boards supporting booting with or without DT

[Simon Horman]

On Fri, Dec 14, 2012 at 05:45:29PM +0100, Guennadi Liakhovetski wrote:
> For boards booting without DT no changes should be caused by this patch.
> When booting with DT, devices, whose drivers support DT probing, will not
> be registered.

This relates in part to my comment on "ARM: sh7372: support mixed DT and
board code interrupt controller init".

It is my understanding that sh7372_add_standard_devices_dt() already
exists in setup-sh7372.c and that it is appropriate to use for bring
up boards with DT.

> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  arch/arm/mach-shmobile/setup-sh7372.c |   17 ++++++++++++++---
>  1 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/setup-sh7372.c b/arch/arm/mach-shmobile/setup-sh7372.c
> index bbe6e2a..3e6bf3d 100644
> --- a/arch/arm/mach-shmobile/setup-sh7372.c
> +++ b/arch/arm/mach-shmobile/setup-sh7372.c
> @@ -981,9 +981,12 @@ static struct platform_device *sh7372_early_devices[] __initdata = {
>  	&tmu01_device,
>  };
>  
> -static struct platform_device *sh7372_late_devices[] __initdata = {
> +static struct platform_device *sh7372_late_devices_dt[] __initdata = {
>  	&iic0_device,
>  	&iic1_device,
> +};
> +
> +static struct platform_device *sh7372_late_devices[] __initdata = {
>  	&dma0_device,
>  	&dma1_device,
>  	&dma2_device,
> @@ -1012,13 +1015,11 @@ void __init sh7372_add_standard_devices(void)
>  		{ "A3SP", &scif4_device, },
>  		{ "A3SP", &scif5_device, },
>  		{ "A3SP", &scif6_device, },
> -		{ "A3SP", &iic1_device, },
>  		{ "A3SP", &dma0_device, },
>  		{ "A3SP", &dma1_device, },
>  		{ "A3SP", &dma2_device, },
>  		{ "A3SP", &usb_dma0_device, },
>  		{ "A3SP", &usb_dma1_device, },
> -		{ "A4R", &iic0_device, },
>  		{ "A4R", &veu0_device, },
>  		{ "A4R", &veu1_device, },
>  		{ "A4R", &veu2_device, },
> @@ -1027,6 +1028,10 @@ void __init sh7372_add_standard_devices(void)
>  		{ "A4R", &tmu00_device, },
>  		{ "A4R", &tmu01_device, },
>  	};
> +	struct pm_domain_device domain_devices_dt[] = {
> +		{ "A3SP", &iic1_device, },
> +		{ "A4R", &iic0_device, },
> +	};
>  
>  	sh7372_init_pm_domains();
>  
> @@ -1035,9 +1040,15 @@ void __init sh7372_add_standard_devices(void)
>  
>  	platform_add_devices(sh7372_late_devices,
>  			    ARRAY_SIZE(sh7372_late_devices));
> +	if (!of_have_populated_dt())
> +		platform_add_devices(sh7372_late_devices_dt,
> +				     ARRAY_SIZE(sh7372_late_devices_dt));
>  
>  	rmobile_add_devices_to_domains(domain_devices,
>  				       ARRAY_SIZE(domain_devices));
> +	if (!of_have_populated_dt())
> +		rmobile_add_devices_to_domains(domain_devices_dt,
> +					       ARRAY_SIZE(domain_devices_dt));
>  }
>  
>  static void __init sh7372_earlytimer_init(void)
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/7] ARM: sh7372: allow boards supporting booting with or without DT

[Guennadi Liakhovetski]

Hi Simon

On Sat, 15 Dec 2012, Simon Horman wrote:

> On Fri, Dec 14, 2012 at 05:45:29PM +0100, Guennadi Liakhovetski wrote:
> > For boards booting without DT no changes should be caused by this patch.
> > When booting with DT, devices, whose drivers support DT probing, will not
> > be registered.
> 
> This relates in part to my comment on "ARM: sh7372: support mixed DT and
> board code interrupt controller init".
> 
> It is my understanding that sh7372_add_standard_devices_dt() already
> exists in setup-sh7372.c and that it is appropriate to use for bring
> up boards with DT.

I think, we partially clarified this already in the discussion of patch 
6/7. This patch makes it possible to use standard functions, e.g. 
sh7372_add_standard_devices() from both DT and non-DT boots. Whereas when 
booting with DT, devices, for which sufficient DT support already exists, 
have to be initialised from DT.

Thanks
Guennadi

> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  arch/arm/mach-shmobile/setup-sh7372.c |   17 ++++++++++++++---
> >  1 files changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-shmobile/setup-sh7372.c b/arch/arm/mach-shmobile/setup-sh7372.c
> > index bbe6e2a..3e6bf3d 100644
> > --- a/arch/arm/mach-shmobile/setup-sh7372.c
> > +++ b/arch/arm/mach-shmobile/setup-sh7372.c
> > @@ -981,9 +981,12 @@ static struct platform_device *sh7372_early_devices[] __initdata = {
> >  	&tmu01_device,
> >  };
> >  
> > -static struct platform_device *sh7372_late_devices[] __initdata = {
> > +static struct platform_device *sh7372_late_devices_dt[] __initdata = {
> >  	&iic0_device,
> >  	&iic1_device,
> > +};
> > +
> > +static struct platform_device *sh7372_late_devices[] __initdata = {
> >  	&dma0_device,
> >  	&dma1_device,
> >  	&dma2_device,
> > @@ -1012,13 +1015,11 @@ void __init sh7372_add_standard_devices(void)
> >  		{ "A3SP", &scif4_device, },
> >  		{ "A3SP", &scif5_device, },
> >  		{ "A3SP", &scif6_device, },
> > -		{ "A3SP", &iic1_device, },
> >  		{ "A3SP", &dma0_device, },
> >  		{ "A3SP", &dma1_device, },
> >  		{ "A3SP", &dma2_device, },
> >  		{ "A3SP", &usb_dma0_device, },
> >  		{ "A3SP", &usb_dma1_device, },
> > -		{ "A4R", &iic0_device, },
> >  		{ "A4R", &veu0_device, },
> >  		{ "A4R", &veu1_device, },
> >  		{ "A4R", &veu2_device, },
> > @@ -1027,6 +1028,10 @@ void __init sh7372_add_standard_devices(void)
> >  		{ "A4R", &tmu00_device, },
> >  		{ "A4R", &tmu01_device, },
> >  	};
> > +	struct pm_domain_device domain_devices_dt[] = {
> > +		{ "A3SP", &iic1_device, },
> > +		{ "A4R", &iic0_device, },
> > +	};
> >  
> >  	sh7372_init_pm_domains();
> >  
> > @@ -1035,9 +1040,15 @@ void __init sh7372_add_standard_devices(void)
> >  
> >  	platform_add_devices(sh7372_late_devices,
> >  			    ARRAY_SIZE(sh7372_late_devices));
> > +	if (!of_have_populated_dt())
> > +		platform_add_devices(sh7372_late_devices_dt,
> > +				     ARRAY_SIZE(sh7372_late_devices_dt));
> >  
> >  	rmobile_add_devices_to_domains(domain_devices,
> >  				       ARRAY_SIZE(domain_devices));
> > +	if (!of_have_populated_dt())
> > +		rmobile_add_devices_to_domains(domain_devices_dt,
> > +					       ARRAY_SIZE(domain_devices_dt));
> >  }
> >  
> >  static void __init sh7372_earlytimer_init(void)
> > -- 
> > 1.7.2.5

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 6/7] ARM: mackerel: support booting with or without DT

[Guennadi Liakhovetski]

This patch adds dynamic switching to booting either with or without DT.
So far only a part of the board initialisation can be done via DT. Devices,
that still need platform data are kept that way. Devices, that can be
initialised from DT will not be supplied from the platform data, if a DT
image is detected.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/mach-shmobile/board-mackerel.c |   84 ++++++++++++++++++++++++-------
 1 files changed, 66 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 39b8f2e..a6358c9 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1326,7 +1326,6 @@ static struct platform_device mackerel_camera = {
 
 static struct platform_device *mackerel_devices[] __initdata = {
 	&nor_flash_device,
-	&smc911x_device,
 	&lcdc_device,
 	&usbhs0_device,
 	&usbhs1_device,
@@ -1335,17 +1334,21 @@ static struct platform_device *mackerel_devices[] __initdata = {
 	&fsi_ak4643_device,
 	&fsi_hdmi_device,
 	&nand_flash_device,
+	&ceu_device,
+	&mackerel_camera,
+	&hdmi_device,
+	&hdmi_lcdc_device,
+	&meram_device,
+};
+
+static struct platform_device *mackerel_devices_dt[] __initdata = {
+	&smc911x_device,
 	&sdhi0_device,
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 	&sdhi1_device,
 #endif
 	&sdhi2_device,
 	&sh_mmcif_device,
-	&ceu_device,
-	&mackerel_camera,
-	&hdmi_device,
-	&hdmi_lcdc_device,
-	&meram_device,
 };
 
 /* Keypad Initialization */
@@ -1404,6 +1407,24 @@ static struct i2c_board_info i2c1_devices[] = {
 	},
 };
 
+static int mackerel_i2c_bus_notify(struct notifier_block *nb,
+				   unsigned long action, void *data)
+{
+	struct device *dev = data;
+
+	if (action != BUS_NOTIFY_ADD_DEVICE ||
+	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
+		return NOTIFY_DONE;
+
+	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block mackerel_i2c_notifier = {
+	.notifier_call = mackerel_i2c_bus_notify,
+};
+
 #define GPIO_PORT9CR	IOMEM(0xE6051009)
 #define GPIO_PORT10CR	IOMEM(0xE605100A)
 #define GPIO_PORT167CR	IOMEM(0xE60520A7)
@@ -1420,22 +1441,26 @@ static void __init mackerel_init(void)
 		{ "A3SP", &usbhs0_device, },
 		{ "A3SP", &usbhs1_device, },
 		{ "A3SP", &nand_flash_device, },
+		{ "A4R", &ceu_device, },
+	};
+	struct pm_domain_device domain_devices_dt[] = {
 		{ "A3SP", &sh_mmcif_device, },
 		{ "A3SP", &sdhi0_device, },
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 		{ "A3SP", &sdhi1_device, },
 #endif
 		{ "A3SP", &sdhi2_device, },
-		{ "A4R", &ceu_device, },
 	};
 	u32 srcr4;
 	struct clk *clk;
 
-	regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
-				     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
-	regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
-				     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
-	regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
+	if (!of_have_populated_dt()) {
+		regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
+					     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
+		regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
+					     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
+		regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
+	}
 
 	/* External clock source */
 	clk_set_rate(&sh7372_dv_clki_clk, 27000000);
@@ -1633,22 +1658,35 @@ static void __init mackerel_init(void)
 	udelay(50);
 	__raw_writel(srcr4 & ~(1 << 13), SRCR4);
 
-	i2c_register_board_info(0, i2c0_devices,
-				ARRAY_SIZE(i2c0_devices));
-	i2c_register_board_info(1, i2c1_devices,
-				ARRAY_SIZE(i2c1_devices));
+	if (!of_have_populated_dt()) {
+		i2c_register_board_info(0, i2c0_devices,
+					ARRAY_SIZE(i2c0_devices));
+		i2c_register_board_info(1, i2c1_devices,
+					ARRAY_SIZE(i2c1_devices));
+	} else {
+		bus_register_notifier(&i2c_bus_type,
+				      &mackerel_i2c_notifier);
+	}
 
 	sh7372_add_standard_devices();
 
 	platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));
+	if (!of_have_populated_dt())
+		platform_add_devices(mackerel_devices_dt,
+				     ARRAY_SIZE(mackerel_devices_dt));
 
 	rmobile_add_devices_to_domains(domain_devices,
 				       ARRAY_SIZE(domain_devices));
+	if (!of_have_populated_dt())
+		rmobile_add_devices_to_domains(domain_devices_dt,
+					       ARRAY_SIZE(domain_devices_dt));
 
 	hdmi_init_pm_clock();
 	sh7372_pm_init();
 	pm_clk_add(&fsi_device.dev, "spu2");
 	pm_clk_add(&hdmi_lcdc_device.dev, "hdmi");
+
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 
 static const char *mackerel_boards_compat_dt[] __initdata = {
@@ -1659,10 +1697,20 @@ static const char *mackerel_boards_compat_dt[] __initdata = {
 DT_MACHINE_START(MACKEREL_DT, "mackerel")
 	.map_io		= sh7372_map_io,
 	.init_early	= sh7372_add_early_devices,
-	.init_irq	= sh7372_init_irq,
+	.init_irq	= sh7372_init_irq_of,
+	.handle_irq	= shmobile_handle_irq_intc,
+	.init_machine	= mackerel_init,
+	.init_late	= sh7372_pm_init_late,
+	.timer		= &shmobile_timer,
+	.dt_compat	= mackerel_boards_compat_dt,
+MACHINE_END
+
+MACHINE_START(MACKEREL, "mackerel")
+	.map_io		= sh7372_map_io,
+	.init_early	= sh7372_add_early_devices,
+	.init_irq	= sh7372_init_irq_of,
 	.handle_irq	= shmobile_handle_irq_intc,
 	.init_machine	= mackerel_init,
 	.init_late	= sh7372_pm_init_late,
 	.timer		= &shmobile_timer,
-	.dt_compat  = mackerel_boards_compat_dt,
 MACHINE_END
-- 
1.7.2.5

Re: [PATCH 6/7] ARM: mackerel: support booting with or without DT

[Simon Horman]

On Fri, Dec 14, 2012 at 05:45:30PM +0100, Guennadi Liakhovetski wrote:
> This patch adds dynamic switching to booting either with or without DT.
> So far only a part of the board initialisation can be done via DT. Devices,
> that still need platform data are kept that way. Devices, that can be
> initialised from DT will not be supplied from the platform data, if a DT
> image is detected.

Hi Guennadi,

I'm not sure that I'm entirely comfortable with the implications
for users here.

In the beginning there was no DT for Mackerel, all boots were non-DT,
and in general the board and its devices have been well supported.

At the present time Mackerel only boots using DT, though almost all
the initialisation is done in C. Thus currently a DT boot fairly full
support for the board and its devices.

If I understand things correctly, with this change, a DT boot becomes a
limited boot that basically only supports devices that can be initialised
using DT. And users are expected to go back to a non-DT boot if they want
fuller support.  This seems undesirable.

The approach that I took with the kzm9g was to provide an alternate dts and
board file, and compatibility string. Clearly that is not an entirely
elegant solution. But it does avoid the problem that I describe above.

> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  arch/arm/mach-shmobile/board-mackerel.c |   84 ++++++++++++++++++++++++-------
>  1 files changed, 66 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> index 39b8f2e..a6358c9 100644
> --- a/arch/arm/mach-shmobile/board-mackerel.c
> +++ b/arch/arm/mach-shmobile/board-mackerel.c
> @@ -1326,7 +1326,6 @@ static struct platform_device mackerel_camera = {
>  
>  static struct platform_device *mackerel_devices[] __initdata = {
>  	&nor_flash_device,
> -	&smc911x_device,
>  	&lcdc_device,
>  	&usbhs0_device,
>  	&usbhs1_device,
> @@ -1335,17 +1334,21 @@ static struct platform_device *mackerel_devices[] __initdata = {
>  	&fsi_ak4643_device,
>  	&fsi_hdmi_device,
>  	&nand_flash_device,
> +	&ceu_device,
> +	&mackerel_camera,
> +	&hdmi_device,
> +	&hdmi_lcdc_device,
> +	&meram_device,
> +};
> +
> +static struct platform_device *mackerel_devices_dt[] __initdata = {
> +	&smc911x_device,
>  	&sdhi0_device,
>  #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
>  	&sdhi1_device,
>  #endif
>  	&sdhi2_device,
>  	&sh_mmcif_device,
> -	&ceu_device,
> -	&mackerel_camera,
> -	&hdmi_device,
> -	&hdmi_lcdc_device,
> -	&meram_device,
>  };
>  
>  /* Keypad Initialization */
> @@ -1404,6 +1407,24 @@ static struct i2c_board_info i2c1_devices[] = {
>  	},
>  };
>  
> +static int mackerel_i2c_bus_notify(struct notifier_block *nb,
> +				   unsigned long action, void *data)
> +{
> +	struct device *dev = data;
> +
> +	if (action != BUS_NOTIFY_ADD_DEVICE ||
> +	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
> +		return NOTIFY_DONE;
> +
> +	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block mackerel_i2c_notifier = {
> +	.notifier_call = mackerel_i2c_bus_notify,
> +};
> +
>  #define GPIO_PORT9CR	IOMEM(0xE6051009)
>  #define GPIO_PORT10CR	IOMEM(0xE605100A)
>  #define GPIO_PORT167CR	IOMEM(0xE60520A7)
> @@ -1420,22 +1441,26 @@ static void __init mackerel_init(void)

I wonder if it would be cleaner to achieve this by providing
mackerel_init_dt().

>  		{ "A3SP", &usbhs0_device, },
>  		{ "A3SP", &usbhs1_device, },
>  		{ "A3SP", &nand_flash_device, },
> +		{ "A4R", &ceu_device, },
> +	};
> +	struct pm_domain_device domain_devices_dt[] = {
>  		{ "A3SP", &sh_mmcif_device, },
>  		{ "A3SP", &sdhi0_device, },
>  #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
>  		{ "A3SP", &sdhi1_device, },
>  #endif
>  		{ "A3SP", &sdhi2_device, },
> -		{ "A4R", &ceu_device, },
>  	};
>  	u32 srcr4;
>  	struct clk *clk;
>  
> -	regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
> -				     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
> -	regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
> -				     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
> -	regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
> +	if (!of_have_populated_dt()) {
> +		regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
> +					     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
> +		regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
> +					     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
> +		regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
> +	}
>  
>  	/* External clock source */
>  	clk_set_rate(&sh7372_dv_clki_clk, 27000000);
> @@ -1633,22 +1658,35 @@ static void __init mackerel_init(void)
>  	udelay(50);
>  	__raw_writel(srcr4 & ~(1 << 13), SRCR4);
>  
> -	i2c_register_board_info(0, i2c0_devices,
> -				ARRAY_SIZE(i2c0_devices));
> -	i2c_register_board_info(1, i2c1_devices,
> -				ARRAY_SIZE(i2c1_devices));
> +	if (!of_have_populated_dt()) {
> +		i2c_register_board_info(0, i2c0_devices,
> +					ARRAY_SIZE(i2c0_devices));
> +		i2c_register_board_info(1, i2c1_devices,
> +					ARRAY_SIZE(i2c1_devices));
> +	} else {
> +		bus_register_notifier(&i2c_bus_type,
> +				      &mackerel_i2c_notifier);
> +	}
>  
>  	sh7372_add_standard_devices();
>  
>  	platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));
> +	if (!of_have_populated_dt())
> +		platform_add_devices(mackerel_devices_dt,
> +				     ARRAY_SIZE(mackerel_devices_dt));
>  
>  	rmobile_add_devices_to_domains(domain_devices,
>  				       ARRAY_SIZE(domain_devices));
> +	if (!of_have_populated_dt())
> +		rmobile_add_devices_to_domains(domain_devices_dt,
> +					       ARRAY_SIZE(domain_devices_dt));
>  
>  	hdmi_init_pm_clock();
>  	sh7372_pm_init();
>  	pm_clk_add(&fsi_device.dev, "spu2");
>  	pm_clk_add(&hdmi_lcdc_device.dev, "hdmi");
> +
> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>  }
>  
>  static const char *mackerel_boards_compat_dt[] __initdata = {
> @@ -1659,10 +1697,20 @@ static const char *mackerel_boards_compat_dt[] __initdata = {
>  DT_MACHINE_START(MACKEREL_DT, "mackerel")
>  	.map_io		= sh7372_map_io,
>  	.init_early	= sh7372_add_early_devices,
> -	.init_irq	= sh7372_init_irq,
> +	.init_irq	= sh7372_init_irq_of,
> +	.handle_irq	= shmobile_handle_irq_intc,
> +	.init_machine	= mackerel_init,
> +	.init_late	= sh7372_pm_init_late,
> +	.timer		= &shmobile_timer,
> +	.dt_compat	= mackerel_boards_compat_dt,
> +MACHINE_END
> +
> +MACHINE_START(MACKEREL, "mackerel")
> +	.map_io		= sh7372_map_io,
> +	.init_early	= sh7372_add_early_devices,
> +	.init_irq	= sh7372_init_irq_of,

Could sh7372_init_irq be used here ?

>  	.handle_irq	= shmobile_handle_irq_intc,
>  	.init_machine	= mackerel_init,
>  	.init_late	= sh7372_pm_init_late,
>  	.timer		= &shmobile_timer,
> -	.dt_compat  = mackerel_boards_compat_dt,
>  MACHINE_END
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 6/7] ARM: mackerel: support booting with or without DT

[Guennadi Liakhovetski]

Hi Simon

Thanks for your comments. I'll reply to other your reviews later, I think, 
let me just briefly clarify this one.

On Sat, 15 Dec 2012, Simon Horman wrote:

> On Fri, Dec 14, 2012 at 05:45:30PM +0100, Guennadi Liakhovetski wrote:
> > This patch adds dynamic switching to booting either with or without DT.
> > So far only a part of the board initialisation can be done via DT. Devices,
> > that still need platform data are kept that way. Devices, that can be
> > initialised from DT will not be supplied from the platform data, if a DT
> > image is detected.
> 
> Hi Guennadi,
> 
> I'm not sure that I'm entirely comfortable with the implications
> for users here.
> 
> In the beginning there was no DT for Mackerel, all boots were non-DT,
> and in general the board and its devices have been well supported.
> 
> At the present time Mackerel only boots using DT, though almost all
> the initialisation is done in C. Thus currently a DT boot fairly full
> support for the board and its devices.
> 
> If I understand things correctly, with this change, a DT boot becomes a
> limited boot that basically only supports devices that can be initialised
> using DT. And users are expected to go back to a non-DT boot if they want
> fuller support.  This seems undesirable.

No, it's in a way the contrary. As you correctly point out after a recent 
change mackerel _must_ only be booted with DT, and, I think, this way an 
undesirable change. After this series of patches both becomes possible: 
booting with and without DT. And in both cases all devices are supported. 
If booting without DT, all devices are supported in the "legacy" way from 
the board file. If booting with DT _some_ devices, for which sufficient DT 
support already exist are initialised from the .dts file, others are still 
initialised from the .c. This way all devices are still supported. Note, 
however, that some devices don't have a complete DT support yet, e.g., you 
cannot specify card-detect GPIOs in .dts because of the lacking pincontrol 
/ GPIO DT support. As soon as that is added, .dts will have to be 
extended respectively. Similarly, as more devices get DT support they can 
be added to the .dts and excluded from the DT boot by putting them under 

+	if (!of_have_populated_dt()) {

clauses. If we ever come to a point, that no-DT booting will not have to 
be supported any more, these clauses and respective devices can be easily 
removed from board-mackerel.c. (Continued below)

> The approach that I took with the kzm9g was to provide an alternate dts and
> board file, and compatibility string. Clearly that is not an entirely
> elegant solution. But it does avoid the problem that I describe above.
> 
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  arch/arm/mach-shmobile/board-mackerel.c |   84 ++++++++++++++++++++++++-------
> >  1 files changed, 66 insertions(+), 18 deletions(-)
> > 
> > diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> > index 39b8f2e..a6358c9 100644
> > --- a/arch/arm/mach-shmobile/board-mackerel.c
> > +++ b/arch/arm/mach-shmobile/board-mackerel.c
> > @@ -1326,7 +1326,6 @@ static struct platform_device mackerel_camera = {
> >  
> >  static struct platform_device *mackerel_devices[] __initdata = {
> >  	&nor_flash_device,
> > -	&smc911x_device,
> >  	&lcdc_device,
> >  	&usbhs0_device,
> >  	&usbhs1_device,
> > @@ -1335,17 +1334,21 @@ static struct platform_device *mackerel_devices[] __initdata = {
> >  	&fsi_ak4643_device,
> >  	&fsi_hdmi_device,
> >  	&nand_flash_device,
> > +	&ceu_device,
> > +	&mackerel_camera,
> > +	&hdmi_device,
> > +	&hdmi_lcdc_device,
> > +	&meram_device,
> > +};
> > +
> > +static struct platform_device *mackerel_devices_dt[] __initdata = {
> > +	&smc911x_device,
> >  	&sdhi0_device,
> >  #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
> >  	&sdhi1_device,
> >  #endif
> >  	&sdhi2_device,
> >  	&sh_mmcif_device,
> > -	&ceu_device,
> > -	&mackerel_camera,
> > -	&hdmi_device,
> > -	&hdmi_lcdc_device,
> > -	&meram_device,
> >  };
> >  
> >  /* Keypad Initialization */
> > @@ -1404,6 +1407,24 @@ static struct i2c_board_info i2c1_devices[] = {
> >  	},
> >  };
> >  
> > +static int mackerel_i2c_bus_notify(struct notifier_block *nb,
> > +				   unsigned long action, void *data)
> > +{
> > +	struct device *dev = data;
> > +
> > +	if (action != BUS_NOTIFY_ADD_DEVICE ||
> > +	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
> > +		return NOTIFY_DONE;
> > +
> > +	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
> > +
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block mackerel_i2c_notifier = {
> > +	.notifier_call = mackerel_i2c_bus_notify,
> > +};
> > +
> >  #define GPIO_PORT9CR	IOMEM(0xE6051009)
> >  #define GPIO_PORT10CR	IOMEM(0xE605100A)
> >  #define GPIO_PORT167CR	IOMEM(0xE60520A7)
> > @@ -1420,22 +1441,26 @@ static void __init mackerel_init(void)
> 
> I wonder if it would be cleaner to achieve this by providing
> mackerel_init_dt().

I thought about this, but initialisation is interleaved and in some cases 
order is important, so, you would need something like "early_dt()," 
"mid_early_common()," "late_dt()" etc., which doesn't seem an improvement 
to me:-)

> >  		{ "A3SP", &usbhs0_device, },
> >  		{ "A3SP", &usbhs1_device, },
> >  		{ "A3SP", &nand_flash_device, },
> > +		{ "A4R", &ceu_device, },
> > +	};
> > +	struct pm_domain_device domain_devices_dt[] = {
> >  		{ "A3SP", &sh_mmcif_device, },
> >  		{ "A3SP", &sdhi0_device, },
> >  #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
> >  		{ "A3SP", &sdhi1_device, },
> >  #endif
> >  		{ "A3SP", &sdhi2_device, },
> > -		{ "A4R", &ceu_device, },
> >  	};
> >  	u32 srcr4;
> >  	struct clk *clk;
> >  
> > -	regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
> > -				     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
> > -	regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
> > -				     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
> > -	regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
> > +	if (!of_have_populated_dt()) {
> > +		regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
> > +					     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
> > +		regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
> > +					     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
> > +		regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
> > +	}
> >  
> >  	/* External clock source */
> >  	clk_set_rate(&sh7372_dv_clki_clk, 27000000);
> > @@ -1633,22 +1658,35 @@ static void __init mackerel_init(void)
> >  	udelay(50);
> >  	__raw_writel(srcr4 & ~(1 << 13), SRCR4);
> >  
> > -	i2c_register_board_info(0, i2c0_devices,
> > -				ARRAY_SIZE(i2c0_devices));
> > -	i2c_register_board_info(1, i2c1_devices,
> > -				ARRAY_SIZE(i2c1_devices));
> > +	if (!of_have_populated_dt()) {
> > +		i2c_register_board_info(0, i2c0_devices,
> > +					ARRAY_SIZE(i2c0_devices));
> > +		i2c_register_board_info(1, i2c1_devices,
> > +					ARRAY_SIZE(i2c1_devices));
> > +	} else {
> > +		bus_register_notifier(&i2c_bus_type,
> > +				      &mackerel_i2c_notifier);
> > +	}
> >  
> >  	sh7372_add_standard_devices();
> >  
> >  	platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));
> > +	if (!of_have_populated_dt())
> > +		platform_add_devices(mackerel_devices_dt,
> > +				     ARRAY_SIZE(mackerel_devices_dt));
> >  
> >  	rmobile_add_devices_to_domains(domain_devices,
> >  				       ARRAY_SIZE(domain_devices));
> > +	if (!of_have_populated_dt())
> > +		rmobile_add_devices_to_domains(domain_devices_dt,
> > +					       ARRAY_SIZE(domain_devices_dt));
> >  
> >  	hdmi_init_pm_clock();
> >  	sh7372_pm_init();
> >  	pm_clk_add(&fsi_device.dev, "spu2");
> >  	pm_clk_add(&hdmi_lcdc_device.dev, "hdmi");
> > +
> > +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> >  }
> >  
> >  static const char *mackerel_boards_compat_dt[] __initdata = {
> > @@ -1659,10 +1697,20 @@ static const char *mackerel_boards_compat_dt[] __initdata = {
> >  DT_MACHINE_START(MACKEREL_DT, "mackerel")
> >  	.map_io		= sh7372_map_io,
> >  	.init_early	= sh7372_add_early_devices,
> > -	.init_irq	= sh7372_init_irq,
> > +	.init_irq	= sh7372_init_irq_of,
> > +	.handle_irq	= shmobile_handle_irq_intc,
> > +	.init_machine	= mackerel_init,
> > +	.init_late	= sh7372_pm_init_late,
> > +	.timer		= &shmobile_timer,
> > +	.dt_compat	= mackerel_boards_compat_dt,
> > +MACHINE_END
> > +
> > +MACHINE_START(MACKEREL, "mackerel")
> > +	.map_io		= sh7372_map_io,
> > +	.init_early	= sh7372_add_early_devices,
> > +	.init_irq	= sh7372_init_irq_of,
> 
> Could sh7372_init_irq be used here ?

Yes, it could. I'll reply to your other review separately, briefly, by 
using the conditional, that I proposed in my patch 3/7 we could make the 
non-dt version static and let everyone just use sh7372_init_irq_of. But 
this is just an idea, we can keep sh7372_init_irq() too if this is 
prefered.

Thanks
Guennadi

> >  	.handle_irq	= shmobile_handle_irq_intc,
> >  	.init_machine	= mackerel_init,
> >  	.init_late	= sh7372_pm_init_late,
> >  	.timer		= &shmobile_timer,
> > -	.dt_compat  = mackerel_boards_compat_dt,
> >  MACHINE_END
> > -- 
> > 1.7.2.5

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 6/7] ARM: mackerel: support booting with or without DT

[Simon Horman]

On Sat, Dec 15, 2012 at 08:02:39PM +0100, Guennadi Liakhovetski wrote:
> Hi Simon
> 
> Thanks for your comments. I'll reply to other your reviews later, I think, 
> let me just briefly clarify this one.
> 
> On Sat, 15 Dec 2012, Simon Horman wrote:
> 
> > On Fri, Dec 14, 2012 at 05:45:30PM +0100, Guennadi Liakhovetski wrote:
> > > This patch adds dynamic switching to booting either with or without DT.
> > > So far only a part of the board initialisation can be done via DT. Devices,
> > > that still need platform data are kept that way. Devices, that can be
> > > initialised from DT will not be supplied from the platform data, if a DT
> > > image is detected.
> > 
> > Hi Guennadi,
> > 
> > I'm not sure that I'm entirely comfortable with the implications
> > for users here.
> > 
> > In the beginning there was no DT for Mackerel, all boots were non-DT,
> > and in general the board and its devices have been well supported.
> > 
> > At the present time Mackerel only boots using DT, though almost all
> > the initialisation is done in C. Thus currently a DT boot fairly full
> > support for the board and its devices.
> > 
> > If I understand things correctly, with this change, a DT boot becomes a
> > limited boot that basically only supports devices that can be initialised
> > using DT. And users are expected to go back to a non-DT boot if they want
> > fuller support.  This seems undesirable.
> 
> No, it's in a way the contrary. As you correctly point out after a recent 
> change mackerel _must_ only be booted with DT, and, I think, this way an 
> undesirable change. After this series of patches both becomes possible: 
> booting with and without DT. And in both cases all devices are supported. 
> If booting without DT, all devices are supported in the "legacy" way from 
> the board file. If booting with DT _some_ devices, for which sufficient DT 
> support already exist are initialised from the .dts file, others are still 
> initialised from the .c. This way all devices are still supported. Note, 
> however, that some devices don't have a complete DT support yet, e.g., you 
> cannot specify card-detect GPIOs in .dts because of the lacking pincontrol 
> / GPIO DT support. As soon as that is added, .dts will have to be 
> extended respectively. Similarly, as more devices get DT support they can 
> be added to the .dts and excluded from the DT boot by putting them under 
> 
> +	if (!of_have_populated_dt()) {
> 
> clauses. If we ever come to a point, that no-DT booting will not have to 
> be supported any more, these clauses and respective devices can be easily 
> removed from board-mackerel.c. (Continued below)

Thanks, sorry for misunderstanding things. In that case I think I am
comfortable with the direction of these changes.

> > The approach that I took with the kzm9g was to provide an alternate dts and
> > board file, and compatibility string. Clearly that is not an entirely
> > elegant solution. But it does avoid the problem that I describe above.
> > 
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > >  arch/arm/mach-shmobile/board-mackerel.c |   84 ++++++++++++++++++++++++-------
> > >  1 files changed, 66 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> > > index 39b8f2e..a6358c9 100644
> > > --- a/arch/arm/mach-shmobile/board-mackerel.c
> > > +++ b/arch/arm/mach-shmobile/board-mackerel.c
> > > @@ -1326,7 +1326,6 @@ static struct platform_device mackerel_camera = {
> > >  
> > >  static struct platform_device *mackerel_devices[] __initdata = {
> > >  	&nor_flash_device,
> > > -	&smc911x_device,
> > >  	&lcdc_device,
> > >  	&usbhs0_device,
> > >  	&usbhs1_device,
> > > @@ -1335,17 +1334,21 @@ static struct platform_device *mackerel_devices[] __initdata = {
> > >  	&fsi_ak4643_device,
> > >  	&fsi_hdmi_device,
> > >  	&nand_flash_device,
> > > +	&ceu_device,
> > > +	&mackerel_camera,
> > > +	&hdmi_device,
> > > +	&hdmi_lcdc_device,
> > > +	&meram_device,
> > > +};
> > > +
> > > +static struct platform_device *mackerel_devices_dt[] __initdata = {
> > > +	&smc911x_device,
> > >  	&sdhi0_device,
> > >  #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
> > >  	&sdhi1_device,
> > >  #endif
> > >  	&sdhi2_device,
> > >  	&sh_mmcif_device,
> > > -	&ceu_device,
> > > -	&mackerel_camera,
> > > -	&hdmi_device,
> > > -	&hdmi_lcdc_device,
> > > -	&meram_device,
> > >  };
> > >  
> > >  /* Keypad Initialization */
> > > @@ -1404,6 +1407,24 @@ static struct i2c_board_info i2c1_devices[] = {
> > >  	},
> > >  };
> > >  
> > > +static int mackerel_i2c_bus_notify(struct notifier_block *nb,
> > > +				   unsigned long action, void *data)
> > > +{
> > > +	struct device *dev = data;
> > > +
> > > +	if (action != BUS_NOTIFY_ADD_DEVICE ||
> > > +	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
> > > +		return NOTIFY_DONE;
> > > +
> > > +	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
> > > +
> > > +	return NOTIFY_OK;
> > > +}
> > > +
> > > +static struct notifier_block mackerel_i2c_notifier = {
> > > +	.notifier_call = mackerel_i2c_bus_notify,
> > > +};
> > > +
> > >  #define GPIO_PORT9CR	IOMEM(0xE6051009)
> > >  #define GPIO_PORT10CR	IOMEM(0xE605100A)
> > >  #define GPIO_PORT167CR	IOMEM(0xE60520A7)
> > > @@ -1420,22 +1441,26 @@ static void __init mackerel_init(void)
> > 
> > I wonder if it would be cleaner to achieve this by providing
> > mackerel_init_dt().
> 
> I thought about this, but initialisation is interleaved and in some cases 
> order is important, so, you would need something like "early_dt()," 
> "mid_early_common()," "late_dt()" etc., which doesn't seem an improvement 
> to me:-)

Understood. I guess it is case by case, and in this case the approach
you have taken does seem to make sense. Magnus, do you have any
thoughts on this?

> > >  		{ "A3SP", &usbhs0_device, },
> > >  		{ "A3SP", &usbhs1_device, },
> > >  		{ "A3SP", &nand_flash_device, },
> > > +		{ "A4R", &ceu_device, },
> > > +	};
> > > +	struct pm_domain_device domain_devices_dt[] = {
> > >  		{ "A3SP", &sh_mmcif_device, },
> > >  		{ "A3SP", &sdhi0_device, },
> > >  #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
> > >  		{ "A3SP", &sdhi1_device, },
> > >  #endif
> > >  		{ "A3SP", &sdhi2_device, },
> > > -		{ "A4R", &ceu_device, },
> > >  	};
> > >  	u32 srcr4;
> > >  	struct clk *clk;
> > >  
> > > -	regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
> > > -				     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
> > > -	regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
> > > -				     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
> > > -	regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
> > > +	if (!of_have_populated_dt()) {
> > > +		regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
> > > +					     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
> > > +		regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
> > > +					     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
> > > +		regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
> > > +	}
> > >  
> > >  	/* External clock source */
> > >  	clk_set_rate(&sh7372_dv_clki_clk, 27000000);
> > > @@ -1633,22 +1658,35 @@ static void __init mackerel_init(void)
> > >  	udelay(50);
> > >  	__raw_writel(srcr4 & ~(1 << 13), SRCR4);
> > >  
> > > -	i2c_register_board_info(0, i2c0_devices,
> > > -				ARRAY_SIZE(i2c0_devices));
> > > -	i2c_register_board_info(1, i2c1_devices,
> > > -				ARRAY_SIZE(i2c1_devices));
> > > +	if (!of_have_populated_dt()) {
> > > +		i2c_register_board_info(0, i2c0_devices,
> > > +					ARRAY_SIZE(i2c0_devices));
> > > +		i2c_register_board_info(1, i2c1_devices,
> > > +					ARRAY_SIZE(i2c1_devices));
> > > +	} else {
> > > +		bus_register_notifier(&i2c_bus_type,
> > > +				      &mackerel_i2c_notifier);
> > > +	}
> > >  
> > >  	sh7372_add_standard_devices();
> > >  
> > >  	platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));
> > > +	if (!of_have_populated_dt())
> > > +		platform_add_devices(mackerel_devices_dt,
> > > +				     ARRAY_SIZE(mackerel_devices_dt));
> > >  
> > >  	rmobile_add_devices_to_domains(domain_devices,
> > >  				       ARRAY_SIZE(domain_devices));
> > > +	if (!of_have_populated_dt())
> > > +		rmobile_add_devices_to_domains(domain_devices_dt,
> > > +					       ARRAY_SIZE(domain_devices_dt));
> > >  
> > >  	hdmi_init_pm_clock();
> > >  	sh7372_pm_init();
> > >  	pm_clk_add(&fsi_device.dev, "spu2");
> > >  	pm_clk_add(&hdmi_lcdc_device.dev, "hdmi");
> > > +
> > > +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > >  }
> > >  
> > >  static const char *mackerel_boards_compat_dt[] __initdata = {
> > > @@ -1659,10 +1697,20 @@ static const char *mackerel_boards_compat_dt[] __initdata = {
> > >  DT_MACHINE_START(MACKEREL_DT, "mackerel")
> > >  	.map_io		= sh7372_map_io,
> > >  	.init_early	= sh7372_add_early_devices,
> > > -	.init_irq	= sh7372_init_irq,
> > > +	.init_irq	= sh7372_init_irq_of,
> > > +	.handle_irq	= shmobile_handle_irq_intc,
> > > +	.init_machine	= mackerel_init,
> > > +	.init_late	= sh7372_pm_init_late,
> > > +	.timer		= &shmobile_timer,
> > > +	.dt_compat	= mackerel_boards_compat_dt,
> > > +MACHINE_END
> > > +
> > > +MACHINE_START(MACKEREL, "mackerel")
> > > +	.map_io		= sh7372_map_io,
> > > +	.init_early	= sh7372_add_early_devices,
> > > +	.init_irq	= sh7372_init_irq_of,
> > 
> > Could sh7372_init_irq be used here ?
> 
> Yes, it could. I'll reply to your other review separately, briefly, by 
> using the conditional, that I proposed in my patch 3/7 we could make the 
> non-dt version static and let everyone just use sh7372_init_irq_of. But 
> this is just an idea, we can keep sh7372_init_irq() too if this is 
> prefered.

That is my preference at this point.

> Thanks
> Guennadi
> 
> > >  	.handle_irq	= shmobile_handle_irq_intc,
> > >  	.init_machine	= mackerel_init,
> > >  	.init_late	= sh7372_pm_init_late,
> > >  	.timer		= &shmobile_timer,
> > > -	.dt_compat  = mackerel_boards_compat_dt,
> > >  MACHINE_END
> > > -- 
> > > 1.7.2.5
> 
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

Re: [PATCH 6/7] ARM: mackerel: support booting with or without DT

[Grant Likely]

On Fri, 14 Dec 2012 17:45:30 +0100, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> This patch adds dynamic switching to booting either with or without DT.
> So far only a part of the board initialisation can be done via DT. Devices,
> that still need platform data are kept that way. Devices, that can be
> initialised from DT will not be supplied from the platform data, if a DT
> image is detected.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  
> +static int mackerel_i2c_bus_notify(struct notifier_block *nb,
> +				   unsigned long action, void *data)
> +{
> +	struct device *dev = data;
> +
> +	if (action != BUS_NOTIFY_ADD_DEVICE ||
> +	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
> +		return NOTIFY_DONE;
> +
> +	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block mackerel_i2c_notifier = {
> +	.notifier_call = mackerel_i2c_bus_notify,
> +};

This looks dodgy. It appears that the purpose of this hook is to create
the tca6408-keys device because it has some platform data. However,
this hook will try to create the device every time BUS_NOTIFY_ADD_DEVICE
happens on the fff20000.i2c bus *including* when the tca6408-keys device
gets added.

The correct approach when you need to add specific platform data is to
still put the device into the device tree and make the notifier look for
that specific device. Then the platform data pointer can be attached at
BUS_NOTIFY_ADD_DEVICE time.

However, it doesn't look like you need a notifier at all. From what I
can see the tca6408-keys device does have platform data, but it is all
simple (no callback pointers). You should instead encode the platform
data into device tree properties and extract them from the driver.

g.

Re: [PATCH 6/7] ARM: mackerel: support booting with or without DT

[Guennadi Liakhovetski]

Hi Grant

On Sun, 16 Dec 2012, Grant Likely wrote:

> On Fri, 14 Dec 2012 17:45:30 +0100, Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org> wrote:
> > This patch adds dynamic switching to booting either with or without DT.
> > So far only a part of the board initialisation can be done via DT. Devices,
> > that still need platform data are kept that way. Devices, that can be
> > initialised from DT will not be supplied from the platform data, if a DT
> > image is detected.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
> > ---
> >  
> > +static int mackerel_i2c_bus_notify(struct notifier_block *nb,
> > +				   unsigned long action, void *data)
> > +{
> > +	struct device *dev = data;
> > +
> > +	if (action != BUS_NOTIFY_ADD_DEVICE ||
> > +	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
> > +		return NOTIFY_DONE;
> > +
> > +	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
> > +
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block mackerel_i2c_notifier = {
> > +	.notifier_call = mackerel_i2c_bus_notify,
> > +};
> 
> This looks dodgy. It appears that the purpose of this hook is to create
> the tca6408-keys device because it has some platform data. However,
> this hook will try to create the device every time BUS_NOTIFY_ADD_DEVICE
> happens on the fff20000.i2c bus *including* when the tca6408-keys device
> gets added.

I think, this is only called once, when the i2c adapter device is added in 
i2c_register_adapter(). I2C client devices have these adapter devices as 
their parents, and those devices have "i2c-%d" as their names. Since all 
client devices get destroyed when the adapter is unbound, the above should 
be safe.

> The correct approach when you need to add specific platform data is to
> still put the device into the device tree and make the notifier look for
> that specific device. Then the platform data pointer can be attached at
> BUS_NOTIFY_ADD_DEVICE time.
> 
> However, it doesn't look like you need a notifier at all. From what I
> can see the tca6408-keys device does have platform data, but it is all
> simple (no callback pointers). You should instead encode the platform
> data into device tree properties and extract them from the driver.

Yes, this is the ultimate goal. But the purpose of this patch series is to 
move whatever devices are possible right _now_ to DT. Ultimately all of 
them should be migrated. So, yes, we could try to begin with tca6408, 
because the above hack is certainly the ugliest one in this series, but in 
principle this is just one of several devices, that we have to keep in 
platform data for now and aim at removing these temporary hacks as soon as 
respective drivers implement sufficient DT support.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 6/7] ARM: mackerel: support booting with or without DT

[Grant Likely]

On Sun, 16 Dec 2012 22:36:56 +0100 (CET), Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> Hi Grant
> 
> On Sun, 16 Dec 2012, Grant Likely wrote:
> 
> > On Fri, 14 Dec 2012 17:45:30 +0100, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > > This patch adds dynamic switching to booting either with or without DT.
> > > So far only a part of the board initialisation can be done via DT. Devices,
> > > that still need platform data are kept that way. Devices, that can be
> > > initialised from DT will not be supplied from the platform data, if a DT
> > > image is detected.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > >  
> > > +static int mackerel_i2c_bus_notify(struct notifier_block *nb,
> > > +				   unsigned long action, void *data)
> > > +{
> > > +	struct device *dev = data;
> > > +
> > > +	if (action != BUS_NOTIFY_ADD_DEVICE ||
> > > +	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
> > > +		return NOTIFY_DONE;
> > > +
> > > +	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
> > > +
> > > +	return NOTIFY_OK;
> > > +}
> > > +
> > > +static struct notifier_block mackerel_i2c_notifier = {
> > > +	.notifier_call = mackerel_i2c_bus_notify,
> > > +};
> > 
> > This looks dodgy. It appears that the purpose of this hook is to create
> > the tca6408-keys device because it has some platform data. However,
> > this hook will try to create the device every time BUS_NOTIFY_ADD_DEVICE
> > happens on the fff20000.i2c bus *including* when the tca6408-keys device
> > gets added.
> 
> I think, this is only called once, when the i2c adapter device is added in 
> i2c_register_adapter(). I2C client devices have these adapter devices as 
> their parents, and those devices have "i2c-%d" as their names. Since all 
> client devices get destroyed when the adapter is unbound, the above should 
> be safe.

Take another look. The way the bus notifiers work is they get called once for every device registered with the given bus type. That means i2c_client objects, not i3c
> 
> > The correct approach when you need to add specific platform data is to
> > still put the device into the device tree and make the notifier look for
> > that specific device. Then the platform data pointer can be attached at
> > BUS_NOTIFY_ADD_DEVICE time.
> > 
> > However, it doesn't look like you need a notifier at all. From what I
> > can see the tca6408-keys device does have platform data, but it is all
> > simple (no callback pointers). You should instead encode the platform
> > data into device tree properties and extract them from the driver.
> 
> Yes, this is the ultimate goal. But the purpose of this patch series is to 
> move whatever devices are possible right _now_ to DT. Ultimately all of 
> them should be migrated. So, yes, we could try to begin with tca6408, 
> because the above hack is certainly the ugliest one in this series, but in 
> principle this is just one of several devices, that we have to keep in 
> platform data for now and aim at removing these temporary hacks as soon as 
> respective drivers implement sufficient DT support.
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH 6/7] ARM: mackerel: support booting with or without DT

[Guennadi Liakhovetski]

Hi Grant

On Mon, 17 Dec 2012, Grant Likely wrote:

> On Sun, 16 Dec 2012 22:36:56 +0100 (CET), Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > Hi Grant
> > 
> > On Sun, 16 Dec 2012, Grant Likely wrote:
> > 
> > > On Fri, 14 Dec 2012 17:45:30 +0100, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > > > This patch adds dynamic switching to booting either with or without DT.
> > > > So far only a part of the board initialisation can be done via DT. Devices,
> > > > that still need platform data are kept that way. Devices, that can be
> > > > initialised from DT will not be supplied from the platform data, if a DT
> > > > image is detected.
> > > > 
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > > ---
> > > >  
> > > > +static int mackerel_i2c_bus_notify(struct notifier_block *nb,
> > > > +				   unsigned long action, void *data)
> > > > +{
> > > > +	struct device *dev = data;
> > > > +
> > > > +	if (action != BUS_NOTIFY_ADD_DEVICE ||
> > > > +	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
> > > > +		return NOTIFY_DONE;
> > > > +
> > > > +	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
> > > > +
> > > > +	return NOTIFY_OK;
> > > > +}
> > > > +
> > > > +static struct notifier_block mackerel_i2c_notifier = {
> > > > +	.notifier_call = mackerel_i2c_bus_notify,
> > > > +};
> > > 
> > > This looks dodgy. It appears that the purpose of this hook is to create
> > > the tca6408-keys device because it has some platform data. However,
> > > this hook will try to create the device every time BUS_NOTIFY_ADD_DEVICE
> > > happens on the fff20000.i2c bus *including* when the tca6408-keys device
> > > gets added.
> > 
> > I think, this is only called once, when the i2c adapter device is added in 
> > i2c_register_adapter(). I2C client devices have these adapter devices as 
> > their parents, and those devices have "i2c-%d" as their names. Since all 
> > client devices get destroyed when the adapter is unbound, the above should 
> > be safe.
> 
> Take another look. The way the bus notifiers work is they get called 
> once for every device registered with the given bus type. That means 
> i2c_client objects, not i3c

I did, and I also tested. Both I2C clients and I2C adapters are placed on 
the i2c_bus_type. So, yes, you're right, the above notifier will be called 
upon registration of each adapter and client. But, the string comparison 
in the "if" above will only match for the I2C adapter, so, no recursion.

Thanks
Guennadi

> > > The correct approach when you need to add specific platform data is to
> > > still put the device into the device tree and make the notifier look for
> > > that specific device. Then the platform data pointer can be attached at
> > > BUS_NOTIFY_ADD_DEVICE time.
> > > 
> > > However, it doesn't look like you need a notifier at all. From what I
> > > can see the tca6408-keys device does have platform data, but it is all
> > > simple (no callback pointers). You should instead encode the platform
> > > data into device tree properties and extract them from the driver.
> > 
> > Yes, this is the ultimate goal. But the purpose of this patch series is to 
> > move whatever devices are possible right _now_ to DT. Ultimately all of 
> > them should be migrated. So, yes, we could try to begin with tca6408, 
> > because the above hack is certainly the ugliest one in this series, but in 
> > principle this is just one of several devices, that we have to keep in 
> > platform data for now and aim at removing these temporary hacks as soon as 
> > respective drivers implement sufficient DT support.
> > 
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> 
> -- 
> Grant Likely, B.Sc, P.Eng.
> Secret Lab Technologies, Ltd.
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 6/7] ARM: mackerel: support booting with or without DT

[Grant Likely]

On Sun, 16 Dec 2012 22:36:56 +0100 (CET), Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> Hi Grant
> 
> On Sun, 16 Dec 2012, Grant Likely wrote:
> 
> > On Fri, 14 Dec 2012 17:45:30 +0100, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > > This patch adds dynamic switching to booting either with or without DT.
> > > So far only a part of the board initialisation can be done via DT. Devices,
> > > that still need platform data are kept that way. Devices, that can be
> > > initialised from DT will not be supplied from the platform data, if a DT
> > > image is detected.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > >  
> > > +static int mackerel_i2c_bus_notify(struct notifier_block *nb,
> > > +				   unsigned long action, void *data)
> > > +{
> > > +	struct device *dev = data;
> > > +
> > > +	if (action != BUS_NOTIFY_ADD_DEVICE ||
> > > +	    strcmp(dev_name(dev->parent), "fff20000.i2c"))
> > > +		return NOTIFY_DONE;
> > > +
> > > +	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
> > > +
> > > +	return NOTIFY_OK;
> > > +}
> > > +
> > > +static struct notifier_block mackerel_i2c_notifier = {
> > > +	.notifier_call = mackerel_i2c_bus_notify,
> > > +};
> > 
> > This looks dodgy. It appears that the purpose of this hook is to create
> > the tca6408-keys device because it has some platform data. However,
> > this hook will try to create the device every time BUS_NOTIFY_ADD_DEVICE
> > happens on the fff20000.i2c bus *including* when the tca6408-keys device
> > gets added.
> 
> I think, this is only called once, when the i2c adapter device is added in 
> i2c_register_adapter(). I2C client devices have these adapter devices as 
> their parents, and those devices have "i2c-%d" as their names. Since all 
> client devices get destroyed when the adapter is unbound, the above should 
> be safe.

Take another look. The way the bus notifiers work is they get called
once for every device registered that has the given bus type. In this
case, that means i2c_bus_type, which also means every i2c_client object
registration. Also, the point of i2c_new_device() is that it creates a
new i2c_client object and registers it.... trigger a new call to this
notifier... which calls i2c_new_device again!

> 
> > The correct approach when you need to add specific platform data is to
> > still put the device into the device tree and make the notifier look for
> > that specific device. Then the platform data pointer can be attached at
> > BUS_NOTIFY_ADD_DEVICE time.
> > 
> > However, it doesn't look like you need a notifier at all. From what I
> > can see the tca6408-keys device does have platform data, but it is all
> > simple (no callback pointers). You should instead encode the platform
> > data into device tree properties and extract them from the driver.
> 
> Yes, this is the ultimate goal. But the purpose of this patch series is to 
> move whatever devices are possible right _now_ to DT. Ultimately all of 
> them should be migrated. So, yes, we could try to begin with tca6408, 
> because the above hack is certainly the ugliest one in this series, but in 
> principle this is just one of several devices, that we have to keep in 
> platform data for now and aim at removing these temporary hacks as soon as 
> respective drivers implement sufficient DT support.

Understood. However I think you're going about it the long way around.
Instead of cherry picking some devices to put into the device tree, you
should put all of them into the DT, even if the driver doesn't have
bindings for them yet. If you really prefer the temporary hack approach
then you should do two things:

1) for platform devices, use AUXDATA to hook up platform_data pointers
when the devices are instantiated from the device tree.
2) for everything else, use 1 notifier per bus_type to hook up
platform_data on the BUS_NOTIFY_ADD_DEVICE event, but instead of the
dodgy code used above, use the dev->of_node->full_name property to
figure out which device has gotten registered.

It would be convenient to have AUXDATA for non-platform device
registrations, but nobody has hooked that up just yet, and it is really
a temporary measure until clock bindings are fully in place.

g.

[PATCH v2 6/7] ARM: mackerel: support booting with or without DT

[Guennadi Liakhovetski]

This patch adds dynamic switching to booting either with or without DT.
So far only a part of the board initialisation can be done via DT. Devices,
that still need platform data are kept that way. Devices, that can be
initialised from DT will not be supplied from the platform data, if a DT
image is detected.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

Hi Simon

As suggested by you we drop patch #3/7 "ARM: sh7372: support mixed DT and 
board code interrupt controller init" and as suggested by Grant we drop 
patch #4/7 "ARM: sh7372: add clock lookup entries for DT-based devices" 
(you can drop it from your soc5 branch too now.) As a result we update 
this patch as follows:

v2:

1. use a lookup table to still be able to use non-DT clock names (thanks 
to Grant for pointing out)

2. use sh7372_init_irq in the non-DT case (thanks to Simon)

Patches 1, 2, 5 and 7 don't change.

Thanks
Guennadi

 arch/arm/mach-shmobile/board-mackerel.c |  100 +++++++++++++++++++++++++-----
 1 files changed, 83 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 39b8f2e..6d13b3a 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1326,7 +1326,6 @@ static struct platform_device mackerel_camera = {
 
 static struct platform_device *mackerel_devices[] __initdata = {
 	&nor_flash_device,
-	&smc911x_device,
 	&lcdc_device,
 	&usbhs0_device,
 	&usbhs1_device,
@@ -1335,17 +1334,21 @@ static struct platform_device *mackerel_devices[] __initdata = {
 	&fsi_ak4643_device,
 	&fsi_hdmi_device,
 	&nand_flash_device,
+	&ceu_device,
+	&mackerel_camera,
+	&hdmi_device,
+	&hdmi_lcdc_device,
+	&meram_device,
+};
+
+static struct platform_device *mackerel_devices_dt[] __initdata = {
+	&smc911x_device,
 	&sdhi0_device,
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 	&sdhi1_device,
 #endif
 	&sdhi2_device,
 	&sh_mmcif_device,
-	&ceu_device,
-	&mackerel_camera,
-	&hdmi_device,
-	&hdmi_lcdc_device,
-	&meram_device,
 };
 
 /* Keypad Initialization */
@@ -1404,6 +1407,41 @@ static struct i2c_board_info i2c1_devices[] = {
 	},
 };
 
+static int mackerel_i2c_bus_notify(struct notifier_block *nb,
+				   unsigned long action, void *data)
+{
+	struct device *dev = data;
+
+	/* We're only interested in 1 event: when the adapter is added */
+	if (action != BUS_NOTIFY_ADD_DEVICE ||
+	    strcmp(dev_name(dev->parent), "i2c-sh_mobile.0"))
+		return NOTIFY_DONE;
+
+	i2c_new_device(to_i2c_adapter(dev), &i2c0_devices[1]);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block mackerel_i2c_notifier = {
+	.notifier_call = mackerel_i2c_bus_notify,
+};
+
+/*
+ * Auxdata required until real OF clocks are provided
+ */
+struct of_dev_auxdata mackerel_auxdata_lookup[] __initdata = {
+	OF_DEV_AUXDATA("renesas,rmobile-iic", 0xfff20000, "i2c-sh_mobile.0", NULL),
+	OF_DEV_AUXDATA("renesas,rmobile-iic", 0xe6c20000, "i2c-sh_mobile.1", NULL),
+	OF_DEV_AUXDATA("renesas,rmobile-iic", 0xfff30000, "i2c-sh_mobile.2", NULL),
+	OF_DEV_AUXDATA("renesas,rmobile-iic", 0xe6d20000, "i2c-sh_mobile.3", NULL),
+	OF_DEV_AUXDATA("renesas,rmobile-iic", 0xe6d30000, "i2c-sh_mobile.4", NULL),
+	OF_DEV_AUXDATA("renesas,shmobile-sdhi", 0xe6850000, "sh_mobile_sdhi.0", NULL),
+	OF_DEV_AUXDATA("renesas,shmobile-sdhi", 0xe6860000, "sh_mobile_sdhi.1", NULL),
+	OF_DEV_AUXDATA("renesas,shmobile-sdhi", 0xe6870000, "sh_mobile_sdhi.2", NULL),
+	OF_DEV_AUXDATA("renesas,sh-mmcif", 0xe6bd0000, "sh_mmcif.0", NULL),
+	{},
+};
+
 #define GPIO_PORT9CR	IOMEM(0xE6051009)
 #define GPIO_PORT10CR	IOMEM(0xE605100A)
 #define GPIO_PORT167CR	IOMEM(0xE60520A7)
@@ -1420,22 +1458,26 @@ static void __init mackerel_init(void)
 		{ "A3SP", &usbhs0_device, },
 		{ "A3SP", &usbhs1_device, },
 		{ "A3SP", &nand_flash_device, },
+		{ "A4R", &ceu_device, },
+	};
+	struct pm_domain_device domain_devices_dt[] = {
 		{ "A3SP", &sh_mmcif_device, },
 		{ "A3SP", &sdhi0_device, },
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 		{ "A3SP", &sdhi1_device, },
 #endif
 		{ "A3SP", &sdhi2_device, },
-		{ "A4R", &ceu_device, },
 	};
 	u32 srcr4;
 	struct clk *clk;
 
-	regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
-				     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
-	regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
-				     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
-	regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
+	if (!of_have_populated_dt()) {
+		regulator_register_always_on(0, "fixed-1.8V", fixed1v8_power_consumers,
+					     ARRAY_SIZE(fixed1v8_power_consumers), 1800000);
+		regulator_register_always_on(1, "fixed-3.3V", fixed3v3_power_consumers,
+					     ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
+		regulator_register_fixed(2, dummy_supplies, ARRAY_SIZE(dummy_supplies));
+	}
 
 	/* External clock source */
 	clk_set_rate(&sh7372_dv_clki_clk, 27000000);
@@ -1633,22 +1675,36 @@ static void __init mackerel_init(void)
 	udelay(50);
 	__raw_writel(srcr4 & ~(1 << 13), SRCR4);
 
-	i2c_register_board_info(0, i2c0_devices,
-				ARRAY_SIZE(i2c0_devices));
-	i2c_register_board_info(1, i2c1_devices,
-				ARRAY_SIZE(i2c1_devices));
+	if (!of_have_populated_dt()) {
+		i2c_register_board_info(0, i2c0_devices,
+					ARRAY_SIZE(i2c0_devices));
+		i2c_register_board_info(1, i2c1_devices,
+					ARRAY_SIZE(i2c1_devices));
+	} else {
+		bus_register_notifier(&i2c_bus_type,
+				      &mackerel_i2c_notifier);
+	}
 
 	sh7372_add_standard_devices();
 
 	platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));
+	if (!of_have_populated_dt())
+		platform_add_devices(mackerel_devices_dt,
+				     ARRAY_SIZE(mackerel_devices_dt));
 
 	rmobile_add_devices_to_domains(domain_devices,
 				       ARRAY_SIZE(domain_devices));
+	if (!of_have_populated_dt())
+		rmobile_add_devices_to_domains(domain_devices_dt,
+					       ARRAY_SIZE(domain_devices_dt));
 
 	hdmi_init_pm_clock();
 	sh7372_pm_init();
 	pm_clk_add(&fsi_device.dev, "spu2");
 	pm_clk_add(&hdmi_lcdc_device.dev, "hdmi");
+
+	of_platform_populate(NULL, of_default_bus_match_table,
+			     mackerel_auxdata_lookup, NULL);
 }
 
 static const char *mackerel_boards_compat_dt[] __initdata = {
@@ -1659,10 +1715,20 @@ static const char *mackerel_boards_compat_dt[] __initdata = {
 DT_MACHINE_START(MACKEREL_DT, "mackerel")
 	.map_io		= sh7372_map_io,
 	.init_early	= sh7372_add_early_devices,
+	.init_irq	= sh7372_init_irq_of,
+	.handle_irq	= shmobile_handle_irq_intc,
+	.init_machine	= mackerel_init,
+	.init_late	= sh7372_pm_init_late,
+	.timer		= &shmobile_timer,
+	.dt_compat	= mackerel_boards_compat_dt,
+MACHINE_END
+
+MACHINE_START(MACKEREL, "mackerel")
+	.map_io		= sh7372_map_io,
+	.init_early	= sh7372_add_early_devices,
 	.init_irq	= sh7372_init_irq,
 	.handle_irq	= shmobile_handle_irq_intc,
 	.init_machine	= mackerel_init,
 	.init_late	= sh7372_pm_init_late,
 	.timer		= &shmobile_timer,
-	.dt_compat  = mackerel_boards_compat_dt,
 MACHINE_END
-- 
1.7.2.5

Re: [PATCH v2 6/7] ARM: mackerel: support booting with or without DT

[Grant Likely]

On Mon, 17 Dec 2012 13:40:45 +0100 (CET), Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> This patch adds dynamic switching to booting either with or without DT.
> So far only a part of the board initialisation can be done via DT. Devices,
> that still need platform data are kept that way. Devices, that can be
> initialised from DT will not be supplied from the platform data, if a DT
> image is detected.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  
>  static const char *mackerel_boards_compat_dt[] __initdata = {
> @@ -1659,10 +1715,20 @@ static const char *mackerel_boards_compat_dt[] __initdata = {
>  DT_MACHINE_START(MACKEREL_DT, "mackerel")
>  	.map_io		= sh7372_map_io,
>  	.init_early	= sh7372_add_early_devices,
> +	.init_irq	= sh7372_init_irq_of,
> +	.handle_irq	= shmobile_handle_irq_intc,
> +	.init_machine	= mackerel_init,
> +	.init_late	= sh7372_pm_init_late,
> +	.timer		= &shmobile_timer,
> +	.dt_compat	= mackerel_boards_compat_dt,
> +MACHINE_END
> +
> +MACHINE_START(MACKEREL, "mackerel")
> +	.map_io		= sh7372_map_io,
> +	.init_early	= sh7372_add_early_devices,
>  	.init_irq	= sh7372_init_irq,
>  	.handle_irq	= shmobile_handle_irq_intc,
>  	.init_machine	= mackerel_init,
>  	.init_late	= sh7372_pm_init_late,
>  	.timer		= &shmobile_timer,
> -	.dt_compat  = mackerel_boards_compat_dt,
>  MACHINE_END

MACHINE_START() handle's the DT case just fine. You shouldn't need
separate MACHINE_START() and DT_MACHINE_START() stanzas. Please merge. I
see that there was some discussion around sh7372_init_irq_of(), but I
it is better to have the single function handle the OF/non-OF case
gracefully rather than duplicating MACHINE definitions.

g.

[PATCH 7/7] ARM: mackerel: add more devices to DT

[Guennadi Liakhovetski]

This patch adds devices, whose initialisation from DT is already supported,
into the board .dts file.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/boot/dts/sh7372-mackerel.dts |   96 +++++++++++++++++++++++++++++++++
 1 files changed, 96 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sh7372-mackerel.dts b/arch/arm/boot/dts/sh7372-mackerel.dts
index 2ede70d..8bc95c0 100644
--- a/arch/arm/boot/dts/sh7372-mackerel.dts
+++ b/arch/arm/boot/dts/sh7372-mackerel.dts
@@ -19,4 +19,100 @@
 		device_type = "memory";
 		reg = <0x40000000 0x10000000>;
 	};
+
+	reg_1p8v: regulator@0 {
+		compatible = "regulator-fixed";
+		regulator-name = "1P8V";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	reg_3p3v: regulator@1 {
+		compatible = "regulator-fixed";
+		regulator-name = "3P3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	lan9220@14000000 {
+		compatible = "smsc,lan9220", "smsc,lan9115";
+		reg = <0x14000000 0x2000000>;
+		phy-mode = "mii";
+		interrupt-parent = <&intca_irq_pins_lo>;
+		interrupts = <0x2c0>;
+		reg-io-width = <4>;
+		smsc,irq-push-pull;
+		smsc,save-mac-address;
+		vddvario-supply = <&reg_1p8v>;
+		vdd33a-supply = <&reg_3p3v>;
+	};
+
+	i2c0: i2c@fff20000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "renesas,rmobile-iic";
+		reg = <0xfff20000 0x1000>;
+		interrupt-parent = <&intcs>;
+		interrupts = <0x4200 0x4220 0x4240 0x4260>;
+
+		clock-frequency = <100000>;
+
+		touchscreen@55 {
+			compatible = "sitronix,st1232-ts";
+			reg = <0x55>;
+			interrupt-parent = <&intca_irq_pins_lo>;
+			interrupts = <0x02e0>;
+		};
+
+		codec@13 {
+			compatible = "asahi-kasei,ak4642-codec";
+			reg = <0x13>;
+		};
+	};
+
+	i2c1: i2c@e6c20000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "renesas,rmobile-iic";
+		reg = <0xe6c20000 0x1000>;
+		interrupt-parent = <&intca>;
+		interrupts = <0x780 0x7a0 0x7c0 0x7e0>;
+
+		clock-frequency = <100000>;
+
+		accelerometer@53 {
+			compatible = "analog-devices,adxl34x";
+			reg = <0x53>;
+			interrupt-parent = <&intca_irq_pins_hi>;
+			interrupts = <0x32a0>;
+		};
+	};
+
+	mmcif0: mmcif@0xe6bd0000 {
+		compatible = "renesas,sh-mmcif", "renesas,sh7372-mmcif";
+		reg = <0xe6bd0000 0x100>;
+		interrupt-parent = <&intca>;
+		interrupts = <0x1ac0 0x1ae0>;
+		vmmc-supply = <&reg_1p8v>;
+	};
+
+	sdhi0: sdhi@0xe6850000 {
+		compatible = "renesas,shmobile-sdhi";
+		reg = <0xe6850000 0x100>;
+		interrupt-parent = <&intca>;
+		interrupts = <0x0e00 0x0e20 0x0e40>;
+		vmmc-supply = <&reg_3p3v>;
+	};
+
+	sdhi2: sdhi@0xe6870000 {
+		compatible = "renesas,shmobile-sdhi";
+		reg = <0xe6870000 0x100>;
+		interrupt-parent = <&intca>;
+		interrupts = <0x1200 0x1220 0x1240>;
+		vmmc-supply = <&reg_3p3v>;
+	};
 };
-- 
1.7.2.5