diff mbox

[3/4] ARM: shmobile: r8a7790: add I2C support

Message ID 87li0752rz.wl%kuninori.morimoto.gx@renesas.com (mailing list archive)
State Accepted
Commit b448c904f5058b6cd35bf1a43ca219dcfeca4da6
Headers show

Commit Message

Kuninori Morimoto Nov. 29, 2013, 3:02 a.m. UTC 
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/mach-shmobile/clock-r8a7790.c |    4 ++++
 arch/arm/mach-shmobile/setup-r8a7790.c |   25 +++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

Comments

Laurent Pinchart Nov. 29, 2013, 5:32 p.m. UTC | #1 
Hi Morimoto-san,

Thank you for the patch.

On Thursday 28 November 2013 19:02:12 Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  arch/arm/mach-shmobile/clock-r8a7790.c |    4 ++++
>  arch/arm/mach-shmobile/setup-r8a7790.c |   25 +++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c
> b/arch/arm/mach-shmobile/clock-r8a7790.c index 7960dc0..cf6dd7f 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> @@ -277,9 +277,13 @@ static struct clk_lookup lookups[] = {
>  	CLKDEV_DEV_ID("sh-sci.8", &mstp_clks[MSTP717]),
>  	CLKDEV_DEV_ID("sh-sci.9", &mstp_clks[MSTP716]),
>  	CLKDEV_DEV_ID("e6508000.i2c", &mstp_clks[MSTP931]),
> +	CLKDEV_DEV_ID("i2c-rcar.0", &mstp_clks[MSTP931]),
>  	CLKDEV_DEV_ID("e6518000.i2c", &mstp_clks[MSTP930]),
> +	CLKDEV_DEV_ID("i2c-rcar.1", &mstp_clks[MSTP930]),
>  	CLKDEV_DEV_ID("e6530000.i2c", &mstp_clks[MSTP929]),
> +	CLKDEV_DEV_ID("i2c-rcar.2", &mstp_clks[MSTP929]),
>  	CLKDEV_DEV_ID("e6540000.i2c", &mstp_clks[MSTP928]),
> +	CLKDEV_DEV_ID("i2c-rcar.3", &mstp_clks[MSTP928]),
>  	CLKDEV_DEV_ID("r8a7790-ether", &mstp_clks[MSTP813]),
>  	CLKDEV_DEV_ID("e61f0000.thermal", &mstp_clks[MSTP522]),
>  	CLKDEV_DEV_ID("rcar_thermal", &mstp_clks[MSTP522]),
> diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c
> b/arch/arm/mach-shmobile/setup-r8a7790.c index 3543c3b..8474818 100644
> --- a/arch/arm/mach-shmobile/setup-r8a7790.c
> +++ b/arch/arm/mach-shmobile/setup-r8a7790.c
> @@ -67,6 +67,27 @@ R8A7790_GPIO(5);
>  		&r8a7790_gpio##idx##_platform_data,			\
>  		sizeof(r8a7790_gpio##idx##_platform_data))
> 
> +static struct resource i2c_resources[] __initdata = {
> +	/* I2C0 */
> +	DEFINE_RES_MEM(0xE6508000, 0x40),
> +	DEFINE_RES_IRQ(gic_spi(287)),
> +	/* I2C1 */
> +	DEFINE_RES_MEM(0xE6518000, 0x40),
> +	DEFINE_RES_IRQ(gic_spi(288)),
> +	/* I2C2 */
> +	DEFINE_RES_MEM(0xE6530000, 0x40),
> +	DEFINE_RES_IRQ(gic_spi(286)),
> +	/* I2C3 */
> +	DEFINE_RES_MEM(0xE6540000, 0x40),
> +	DEFINE_RES_IRQ(gic_spi(290)),
> +
> +};
> +
> +#define r8a7790_register_i2c(idx)		\
> +	platform_device_register_simple(	\
> +		"i2c-rcar", idx,		\
> +		i2c_resources + (2 * idx), 2);	\
> +
>  void __init r8a7790_pinmux_init(void)
>  {
>  	r8a7790_register_pfc();
> @@ -76,6 +97,10 @@ void __init r8a7790_pinmux_init(void)
>  	r8a7790_register_gpio(3);
>  	r8a7790_register_gpio(4);
>  	r8a7790_register_gpio(5);
> +	r8a7790_register_i2c(0);
> +	r8a7790_register_i2c(1);
> +	r8a7790_register_i2c(2);
> +	r8a7790_register_i2c(3);
>  }

I might be wrong, but I believe we have been requested to add new devices 
directly to the board-*.c files, not to the setup-*.c files.

> 
>  #define SCIF_COMMON(scif_type, baseaddr, irq)			\
Kuninori Morimoto Dec. 2, 2013, 1:32 a.m. UTC | #2 
Hi Laurent

> > @@ -76,6 +97,10 @@ void __init r8a7790_pinmux_init(void)
> >  	r8a7790_register_gpio(3);
> >  	r8a7790_register_gpio(4);
> >  	r8a7790_register_gpio(5);
> > +	r8a7790_register_i2c(0);
> > +	r8a7790_register_i2c(1);
> > +	r8a7790_register_i2c(2);
> > +	r8a7790_register_i2c(3);
> >  }
> 
> I might be wrong, but I believe we have been requested to add new devices 
> directly to the board-*.c files, not to the setup-*.c files.

It depends on device.
I2C host doesn't request board/platform information,
so, setup-xxxx.c is fine.
Please check other setup-xxx.c


Best regards
---
Kuninori Morimoto
--
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
Laurent Pinchart Dec. 2, 2013, 11:38 a.m. UTC | #3 
Hi Morimoto-san,

On Sunday 01 December 2013 17:32:37 Kuninori Morimoto wrote:
> Hi Laurent
> 
> > > @@ -76,6 +97,10 @@ void __init r8a7790_pinmux_init(void)
> > > 
> > >  	r8a7790_register_gpio(3);
> > >  	r8a7790_register_gpio(4);
> > >  	r8a7790_register_gpio(5);
> > > 
> > > +	r8a7790_register_i2c(0);
> > > +	r8a7790_register_i2c(1);
> > > +	r8a7790_register_i2c(2);
> > > +	r8a7790_register_i2c(3);
> > > 
> > >  }
> > 
> > I might be wrong, but I believe we have been requested to add new devices
> > directly to the board-*.c files, not to the setup-*.c files.
> 
> It depends on device.
> I2C host doesn't request board/platform information,
> so, setup-xxxx.c is fine.
> Please check other setup-xxx.c

Sure, that's how it's currently done for other SoCs, but I thought we were 
asked to change that for new patches. I might be wrong, Magnus, Simon, could 
you please comment on this ?
Simon Horman Dec. 5, 2013, 3:12 a.m. UTC | #4 
On Mon, Dec 02, 2013 at 12:38:54PM +0100, Laurent Pinchart wrote:
> Hi Morimoto-san,
> 
> On Sunday 01 December 2013 17:32:37 Kuninori Morimoto wrote:
> > Hi Laurent
> > 
> > > > @@ -76,6 +97,10 @@ void __init r8a7790_pinmux_init(void)
> > > > 
> > > >  	r8a7790_register_gpio(3);
> > > >  	r8a7790_register_gpio(4);
> > > >  	r8a7790_register_gpio(5);
> > > > 
> > > > +	r8a7790_register_i2c(0);
> > > > +	r8a7790_register_i2c(1);
> > > > +	r8a7790_register_i2c(2);
> > > > +	r8a7790_register_i2c(3);
> > > > 
> > > >  }
> > > 
> > > I might be wrong, but I believe we have been requested to add new devices
> > > directly to the board-*.c files, not to the setup-*.c files.
> > 
> > It depends on device.
> > I2C host doesn't request board/platform information,
> > so, setup-xxxx.c is fine.
> > Please check other setup-xxx.c
> 
> Sure, that's how it's currently done for other SoCs, but I thought we were 
> asked to change that for new patches. I might be wrong, Magnus, Simon, could 
> you please comment on this ?

I think that if the device can be initialised entirely
in the setup- file then its clean enough to put it there.
However, if any part of the initialisation should
go in the board- code then all of it should go there.

This is different to how we previously handled things
where board- code may call some setup- code.
That made a lot of sense in terms of the hardware.
But it added some complexity.

As we would like to ultimately move to DT it seems that there
is little point in adding such complexity now to legacy code. So the
idea is just to completely initialise the devices in one file
or another.

At least, that is my understanding :)
--
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
Laurent Pinchart Dec. 5, 2013, 5:49 a.m. UTC | #5 
Hi Simon,

On Thursday 05 December 2013 12:12:43 Simon Horman wrote:
> On Mon, Dec 02, 2013 at 12:38:54PM +0100, Laurent Pinchart wrote:
> > On Sunday 01 December 2013 17:32:37 Kuninori Morimoto wrote:
> > > Hi Laurent
> > > 
> > > > > @@ -76,6 +97,10 @@ void __init r8a7790_pinmux_init(void)
> > > > > 
> > > > >  	r8a7790_register_gpio(3);
> > > > >  	r8a7790_register_gpio(4);
> > > > >  	r8a7790_register_gpio(5);
> > > > > 
> > > > > +	r8a7790_register_i2c(0);
> > > > > +	r8a7790_register_i2c(1);
> > > > > +	r8a7790_register_i2c(2);
> > > > > +	r8a7790_register_i2c(3);
> > > > > 
> > > > >  }
> > > > 
> > > > I might be wrong, but I believe we have been requested to add new
> > > > devices directly to the board-*.c files, not to the setup-*.c files.
> > > 
> > > It depends on device.
> > > I2C host doesn't request board/platform information,
> > > so, setup-xxxx.c is fine.
> > > Please check other setup-xxx.c
> > 
> > Sure, that's how it's currently done for other SoCs, but I thought we were
> > asked to change that for new patches. I might be wrong, Magnus, Simon,
> > could you please comment on this ?
> 
> I think that if the device can be initialised entirely
> in the setup- file then its clean enough to put it there.
> However, if any part of the initialisation should
> go in the board- code then all of it should go there.
> 
> This is different to how we previously handled things
> where board- code may call some setup- code.
> That made a lot of sense in terms of the hardware.
> But it added some complexity.
> 
> As we would like to ultimately move to DT it seems that there
> is little point in adding such complexity now to legacy code. So the
> idea is just to completely initialise the devices in one file
> or another.
> 
> At least, that is my understanding :)

Thank you for the clarification.
Simon Horman Dec. 12, 2013, 11:53 a.m. UTC | #6 
On Thu, Nov 28, 2013 at 07:02:12PM -0800, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  arch/arm/mach-shmobile/clock-r8a7790.c |    4 ++++
>  arch/arm/mach-shmobile/setup-r8a7790.c |   25 +++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)

Thanks, I will queue this up.

> 
> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> index 7960dc0..cf6dd7f 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> @@ -277,9 +277,13 @@ static struct clk_lookup lookups[] = {
>  	CLKDEV_DEV_ID("sh-sci.8", &mstp_clks[MSTP717]),
>  	CLKDEV_DEV_ID("sh-sci.9", &mstp_clks[MSTP716]),
>  	CLKDEV_DEV_ID("e6508000.i2c", &mstp_clks[MSTP931]),
> +	CLKDEV_DEV_ID("i2c-rcar.0", &mstp_clks[MSTP931]),
>  	CLKDEV_DEV_ID("e6518000.i2c", &mstp_clks[MSTP930]),
> +	CLKDEV_DEV_ID("i2c-rcar.1", &mstp_clks[MSTP930]),
>  	CLKDEV_DEV_ID("e6530000.i2c", &mstp_clks[MSTP929]),
> +	CLKDEV_DEV_ID("i2c-rcar.2", &mstp_clks[MSTP929]),
>  	CLKDEV_DEV_ID("e6540000.i2c", &mstp_clks[MSTP928]),
> +	CLKDEV_DEV_ID("i2c-rcar.3", &mstp_clks[MSTP928]),
>  	CLKDEV_DEV_ID("r8a7790-ether", &mstp_clks[MSTP813]),
>  	CLKDEV_DEV_ID("e61f0000.thermal", &mstp_clks[MSTP522]),
>  	CLKDEV_DEV_ID("rcar_thermal", &mstp_clks[MSTP522]),
> diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
> index 3543c3b..8474818 100644
> --- a/arch/arm/mach-shmobile/setup-r8a7790.c
> +++ b/arch/arm/mach-shmobile/setup-r8a7790.c
> @@ -67,6 +67,27 @@ R8A7790_GPIO(5);
>  		&r8a7790_gpio##idx##_platform_data,			\
>  		sizeof(r8a7790_gpio##idx##_platform_data))
>  
> +static struct resource i2c_resources[] __initdata = {
> +	/* I2C0 */
> +	DEFINE_RES_MEM(0xE6508000, 0x40),
> +	DEFINE_RES_IRQ(gic_spi(287)),
> +	/* I2C1 */
> +	DEFINE_RES_MEM(0xE6518000, 0x40),
> +	DEFINE_RES_IRQ(gic_spi(288)),
> +	/* I2C2 */
> +	DEFINE_RES_MEM(0xE6530000, 0x40),
> +	DEFINE_RES_IRQ(gic_spi(286)),
> +	/* I2C3 */
> +	DEFINE_RES_MEM(0xE6540000, 0x40),
> +	DEFINE_RES_IRQ(gic_spi(290)),
> +
> +};
> +
> +#define r8a7790_register_i2c(idx)		\
> +	platform_device_register_simple(	\
> +		"i2c-rcar", idx,		\
> +		i2c_resources + (2 * idx), 2);	\
> +
>  void __init r8a7790_pinmux_init(void)
>  {
>  	r8a7790_register_pfc();
> @@ -76,6 +97,10 @@ void __init r8a7790_pinmux_init(void)
>  	r8a7790_register_gpio(3);
>  	r8a7790_register_gpio(4);
>  	r8a7790_register_gpio(5);
> +	r8a7790_register_i2c(0);
> +	r8a7790_register_i2c(1);
> +	r8a7790_register_i2c(2);
> +	r8a7790_register_i2c(3);
>  }
>  
>  #define SCIF_COMMON(scif_type, baseaddr, irq)			\
> -- 
> 1.7.9.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
> 
--
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
diff mbox

Patch

diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
index 7960dc0..cf6dd7f 100644
--- a/arch/arm/mach-shmobile/clock-r8a7790.c
+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
@@ -277,9 +277,13 @@  static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh-sci.8", &mstp_clks[MSTP717]),
 	CLKDEV_DEV_ID("sh-sci.9", &mstp_clks[MSTP716]),
 	CLKDEV_DEV_ID("e6508000.i2c", &mstp_clks[MSTP931]),
+	CLKDEV_DEV_ID("i2c-rcar.0", &mstp_clks[MSTP931]),
 	CLKDEV_DEV_ID("e6518000.i2c", &mstp_clks[MSTP930]),
+	CLKDEV_DEV_ID("i2c-rcar.1", &mstp_clks[MSTP930]),
 	CLKDEV_DEV_ID("e6530000.i2c", &mstp_clks[MSTP929]),
+	CLKDEV_DEV_ID("i2c-rcar.2", &mstp_clks[MSTP929]),
 	CLKDEV_DEV_ID("e6540000.i2c", &mstp_clks[MSTP928]),
+	CLKDEV_DEV_ID("i2c-rcar.3", &mstp_clks[MSTP928]),
 	CLKDEV_DEV_ID("r8a7790-ether", &mstp_clks[MSTP813]),
 	CLKDEV_DEV_ID("e61f0000.thermal", &mstp_clks[MSTP522]),
 	CLKDEV_DEV_ID("rcar_thermal", &mstp_clks[MSTP522]),
diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
index 3543c3b..8474818 100644
--- a/arch/arm/mach-shmobile/setup-r8a7790.c
+++ b/arch/arm/mach-shmobile/setup-r8a7790.c
@@ -67,6 +67,27 @@  R8A7790_GPIO(5);
 		&r8a7790_gpio##idx##_platform_data,			\
 		sizeof(r8a7790_gpio##idx##_platform_data))
 
+static struct resource i2c_resources[] __initdata = {
+	/* I2C0 */
+	DEFINE_RES_MEM(0xE6508000, 0x40),
+	DEFINE_RES_IRQ(gic_spi(287)),
+	/* I2C1 */
+	DEFINE_RES_MEM(0xE6518000, 0x40),
+	DEFINE_RES_IRQ(gic_spi(288)),
+	/* I2C2 */
+	DEFINE_RES_MEM(0xE6530000, 0x40),
+	DEFINE_RES_IRQ(gic_spi(286)),
+	/* I2C3 */
+	DEFINE_RES_MEM(0xE6540000, 0x40),
+	DEFINE_RES_IRQ(gic_spi(290)),
+
+};
+
+#define r8a7790_register_i2c(idx)		\
+	platform_device_register_simple(	\
+		"i2c-rcar", idx,		\
+		i2c_resources + (2 * idx), 2);	\
+
 void __init r8a7790_pinmux_init(void)
 {
 	r8a7790_register_pfc();
@@ -76,6 +97,10 @@  void __init r8a7790_pinmux_init(void)
 	r8a7790_register_gpio(3);
 	r8a7790_register_gpio(4);
 	r8a7790_register_gpio(5);
+	r8a7790_register_i2c(0);
+	r8a7790_register_i2c(1);
+	r8a7790_register_i2c(2);
+	r8a7790_register_i2c(3);
 }
 
 #define SCIF_COMMON(scif_type, baseaddr, irq)			\