diff mbox

[4/6] Add I2C support for SH7724

Message ID ubpqy3adp.wl%morimoto.kuninori@renesas.com (mailing list archive)
State Superseded
Headers show

Commit Message

Kuninori Morimoto April 15, 2009, 2:42 a.m. UTC 
Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 arch/sh/kernel/cpu/sh4a/setup-sh7724.c |   45 ++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

Comments

Magnus Damm April 15, 2009, 11:36 a.m. UTC | #1 
Hi Morimoto-san,

Here comes a few nitpicks!

On Wed, Apr 15, 2009 at 11:42 AM, Kuninori Morimoto
<morimoto.kuninori@renesas.com> wrote:
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
> ---
>  arch/sh/kernel/cpu/sh4a/setup-sh7724.c |   45 ++++++++++++++++++++++++++++++++
>  1 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
> index cac953b..ca625c2 100644
> --- a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
> +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
> @@ -125,10 +125,55 @@ static struct platform_device usb0_device = {
>        .resource       = usb0_host_resources,
>  };
>
> +/* I2C */
> +static struct resource iic0_resources[] = {
> +       [0] = {
> +               .name   = "IIC0",
> +               .start  = 0x04470000,
> +               .end    = 0x04470016 - 1,

Each SuperH Mobile I2C register is accessed using 8-bit read/write
operation, but all registers are 32-bit aligned. sh7724 does not seem
to be any exception. Please use ...0017 as end.

> +static struct resource iic1_resources[] = {
> +       [0] = {
> +               .name   = "IIC1",
> +               .start  = 0x04750000,
> +               .end    = 0x04750016 - 1,

Same here.

> +static struct platform_device iic0_device = {
> +       .name           = "i2c-sh_mobile",
> +       .id             = 0, /* "i2c0" clock */
> +       .num_resources  = ARRAY_SIZE(iic0_resources),
> +       .resource       = iic0_resources,
> +};
> +
> +static struct platform_device iic1_device = {
> +       .name           = "i2c-sh_mobile",
> +       .id             = 1, /* "i2c1" clock */
> +       .num_resources  = ARRAY_SIZE(iic1_resources),
> +       .resource       = iic1_resources,
> +};

Why do you keep these structures together? Please put iic0_device
after iic0_resources.

Please fix and resend.

Thanks,

/ magnus
--
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
Kuninori Morimoto April 16, 2009, midnight UTC | #2 
Dear Magnus

Thank you for your comment

> > +/* I2C */
> > +static struct resource iic0_resources[] = {
> > +       [0] = {
> > +               .name   = "IIC0",
> > +               .start  = 0x04470000,
> > +               .end    = 0x04470016 - 1,
> 
> Each SuperH Mobile I2C register is accessed using 8-bit read/write
> operation, but all registers are 32-bit aligned. sh7724 does not seem
> to be any exception. Please use ...0017 as end.

I could understand thank you.

> > +static struct platform_device iic0_device = {
> > +       .name           = "i2c-sh_mobile",
> > +       .id             = 0, /* "i2c0" clock */
> > +       .num_resources  = ARRAY_SIZE(iic0_resources),
> > +       .resource       = iic0_resources,
> > +};
> > +
> > +static struct platform_device iic1_device = {
> > +       .name           = "i2c-sh_mobile",
> > +       .id             = 1, /* "i2c1" clock */
> > +       .num_resources  = ARRAY_SIZE(iic1_resources),
> > +       .resource       = iic1_resources,
> > +};
> 
> Why do you keep these structures together? Please put iic0_device
> after iic0_resources.

sorry. I will fix it.
Thank you

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
diff mbox

Patch

diff --git a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
index cac953b..ca625c2 100644
--- a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
+++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c
@@ -125,10 +125,55 @@  static struct platform_device usb0_device = {
 	.resource	= usb0_host_resources,
 };
 
+/* I2C */
+static struct resource iic0_resources[] = {
+	[0] = {
+		.name	= "IIC0",
+		.start  = 0x04470000,
+		.end    = 0x04470016 - 1,
+		.flags  = IORESOURCE_MEM,
+	},
+	[1] = {
+		.start  = 96,
+		.end    = 99,
+		.flags  = IORESOURCE_IRQ,
+	},
+};
+
+static struct resource iic1_resources[] = {
+	[0] = {
+		.name	= "IIC1",
+		.start  = 0x04750000,
+		.end    = 0x04750016 - 1,
+		.flags  = IORESOURCE_MEM,
+	},
+	[1] = {
+		.start  = 92,
+		.end    = 95,
+		.flags  = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device iic0_device = {
+	.name           = "i2c-sh_mobile",
+	.id             = 0, /* "i2c0" clock */
+	.num_resources  = ARRAY_SIZE(iic0_resources),
+	.resource       = iic0_resources,
+};
+
+static struct platform_device iic1_device = {
+	.name           = "i2c-sh_mobile",
+	.id             = 1, /* "i2c1" clock */
+	.num_resources  = ARRAY_SIZE(iic1_resources),
+	.resource       = iic1_resources,
+};
+
 static struct platform_device *sh7724_devices[] __initdata = {
 	&sci_device,
 	&rtc_device,
 	&usb0_device,
+	&iic0_device,
+	&iic1_device,
 };
 
 #define UPONCR0		0xa40501d4