Message ID | 87li0752rz.wl%kuninori.morimoto.gx@renesas.com (mailing list archive) |
---|---|
State | Accepted |
Commit | b448c904f5058b6cd35bf1a43ca219dcfeca4da6 |
Headers | show |
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) \
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
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 ?
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
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.
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 --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) \
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(+)