| Message ID | 1371024438-16631-5-git-send-email-maxime.ripard@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Maxime, On Wednesday 12 of June 2013 10:07:13 Maxime Ripard wrote: > Add the compatible string for the Allwinner A10 i2c controller and the > associated register layout. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > drivers/i2c/busses/Kconfig | 3 ++- > drivers/i2c/busses/i2c-mv64xxx.c | 1 + > include/linux/mv643xx_i2c.h | 10 ++++++++++ > 3 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 631736e..5dc4148 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -507,10 +507,11 @@ config I2C_MPC > > config I2C_MV64XXX > tristate "Marvell mv64xxx I2C Controller" > - depends on (MV64X60 || PLAT_ORION) > + depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI) > help > If you say yes to this option, support will be included for the > built-in I2C interface on the Marvell 64xxx line of host bridges. > + This driver is also used for Allwinner SoCs I2C controllers. > > This driver can also be built as a module. If so, the module > will be called i2c-mv64xxx. > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c > b/drivers/i2c/busses/i2c-mv64xxx.c index f9e076e..febf5ba 100644 > --- a/drivers/i2c/busses/i2c-mv64xxx.c > +++ b/drivers/i2c/busses/i2c-mv64xxx.c > @@ -490,6 +490,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = > { */ > #ifdef CONFIG_OF > static const struct of_device_id mv64xxx_i2c_of_match_table[] = { > + { .compatible = "allwinner,sun4i-i2c", .data = > &mv64xxx_i2c_regs_sun4i}, { .compatible = "marvell,mv64xxx-i2c", .data > = &mv64xxx_i2c_regs_mv64xxx}, {} Ah, OK, here you use compatible matching to get variant-specific data, fine. Still I think the same method (based on platform device name) should be used for non-DT variant. > }; > diff --git a/include/linux/mv643xx_i2c.h b/include/linux/mv643xx_i2c.h > index 9304c94..737108e 100644 > --- a/include/linux/mv643xx_i2c.h > +++ b/include/linux/mv643xx_i2c.h > @@ -32,6 +32,16 @@ struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = { > .soft_reset = 0x1c, > }; > > +struct mv64xxx_i2c_regs mv64xxx_i2c_regs_sun4i = { > + .addr = 0x00, > + .ext_addr = 0x04, > + .data = 0x08, > + .control = 0x0c, > + .status = 0x10, > + .clock = 0x14, > + .soft_reset = 0x18, > +}; > + Hmm, header doesn't look like a correct place for a structure definition, especially when the structure isn't even static. Best regards, Tomasz > /* i2c Platform Device, Driver Data */ > struct mv64xxx_i2c_pdata { > u32 freq_m; -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 12, 2013 at 10:07:13AM +0200, Maxime Ripard wrote: > Add the compatible string for the Allwinner A10 i2c controller and the > associated register layout. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > drivers/i2c/busses/Kconfig | 3 ++- > drivers/i2c/busses/i2c-mv64xxx.c | 1 + > include/linux/mv643xx_i2c.h | 10 ++++++++++ > 3 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 631736e..5dc4148 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -507,10 +507,11 @@ config I2C_MPC > > config I2C_MV64XXX > tristate "Marvell mv64xxx I2C Controller" > - depends on (MV64X60 || PLAT_ORION) > + depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI) Hi Maxime. What about MV64X60? It also needs to pass the registers in its platform data. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tomasz, On Wed, Jun 12, 2013 at 10:42:31AM +0200, Tomasz Figa wrote: > Hi Maxime, > > On Wednesday 12 of June 2013 10:07:13 Maxime Ripard wrote: > > Add the compatible string for the Allwinner A10 i2c controller and the > > associated register layout. > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > > drivers/i2c/busses/Kconfig | 3 ++- > > drivers/i2c/busses/i2c-mv64xxx.c | 1 + > > include/linux/mv643xx_i2c.h | 10 ++++++++++ > > 3 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > index 631736e..5dc4148 100644 > > --- a/drivers/i2c/busses/Kconfig > > +++ b/drivers/i2c/busses/Kconfig > > @@ -507,10 +507,11 @@ config I2C_MPC > > > > config I2C_MV64XXX > > tristate "Marvell mv64xxx I2C Controller" > > - depends on (MV64X60 || PLAT_ORION) > > + depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI) > > help > > If you say yes to this option, support will be included for the > > built-in I2C interface on the Marvell 64xxx line of host > bridges. > > + This driver is also used for Allwinner SoCs I2C controllers. > > > > This driver can also be built as a module. If so, the module > > will be called i2c-mv64xxx. > > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c > > b/drivers/i2c/busses/i2c-mv64xxx.c index f9e076e..febf5ba 100644 > > --- a/drivers/i2c/busses/i2c-mv64xxx.c > > +++ b/drivers/i2c/busses/i2c-mv64xxx.c > > @@ -490,6 +490,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = > > { */ > > #ifdef CONFIG_OF > > static const struct of_device_id mv64xxx_i2c_of_match_table[] = { > > + { .compatible = "allwinner,sun4i-i2c", .data = > > &mv64xxx_i2c_regs_sun4i}, { .compatible = "marvell,mv64xxx-i2c", .data > > = &mv64xxx_i2c_regs_mv64xxx}, {} > > Ah, OK, here you use compatible matching to get variant-specific data, > fine. Still I think the same method (based on platform device name) should > be used for non-DT variant. Yes, you're right. I wasn't aware that you could do something similar with the platform device name, hence why I used the platform data. I'll fix this in the next iteration. > > }; > > diff --git a/include/linux/mv643xx_i2c.h b/include/linux/mv643xx_i2c.h > > index 9304c94..737108e 100644 > > --- a/include/linux/mv643xx_i2c.h > > +++ b/include/linux/mv643xx_i2c.h > > @@ -32,6 +32,16 @@ struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = { > > .soft_reset = 0x1c, > > }; > > > > +struct mv64xxx_i2c_regs mv64xxx_i2c_regs_sun4i = { > > + .addr = 0x00, > > + .ext_addr = 0x04, > > + .data = 0x08, > > + .control = 0x0c, > > + .status = 0x10, > > + .clock = 0x14, > > + .soft_reset = 0x18, > > +}; > > + > > Hmm, header doesn't look like a correct place for a structure definition, > especially when the structure isn't even static. I put it there because it had to be used by the board files to fill their plateform_data. Obviously, it will be moved back into the driver. Thanks, Maxime
On Wed, Jun 12, 2013 at 12:56:13PM +0200, Andrew Lunn wrote: > On Wed, Jun 12, 2013 at 10:07:13AM +0200, Maxime Ripard wrote: > > Add the compatible string for the Allwinner A10 i2c controller and the > > associated register layout. > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > > drivers/i2c/busses/Kconfig | 3 ++- > > drivers/i2c/busses/i2c-mv64xxx.c | 1 + > > include/linux/mv643xx_i2c.h | 10 ++++++++++ > > 3 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > index 631736e..5dc4148 100644 > > --- a/drivers/i2c/busses/Kconfig > > +++ b/drivers/i2c/busses/Kconfig > > @@ -507,10 +507,11 @@ config I2C_MPC > > > > config I2C_MV64XXX > > tristate "Marvell mv64xxx I2C Controller" > > - depends on (MV64X60 || PLAT_ORION) > > + depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI) > > Hi Maxime. > > What about MV64X60? It also needs to pass the registers in its > platform data. I grep'ed for the users mv64xxx_i2c_pdata in arch/arm, and the plat-orion/common.c was the only user. Obviously, I forgot to think about the PPC architecture. Anyway, this patch will be dropped. Thanks! Maxime
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 631736e..5dc4148 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -507,10 +507,11 @@ config I2C_MPC config I2C_MV64XXX tristate "Marvell mv64xxx I2C Controller" - depends on (MV64X60 || PLAT_ORION) + depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI) help If you say yes to this option, support will be included for the built-in I2C interface on the Marvell 64xxx line of host bridges. + This driver is also used for Allwinner SoCs I2C controllers. This driver can also be built as a module. If so, the module will be called i2c-mv64xxx. diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index f9e076e..febf5ba 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -490,6 +490,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = { */ #ifdef CONFIG_OF static const struct of_device_id mv64xxx_i2c_of_match_table[] = { + { .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i}, { .compatible = "marvell,mv64xxx-i2c", .data = &mv64xxx_i2c_regs_mv64xxx}, {} }; diff --git a/include/linux/mv643xx_i2c.h b/include/linux/mv643xx_i2c.h index 9304c94..737108e 100644 --- a/include/linux/mv643xx_i2c.h +++ b/include/linux/mv643xx_i2c.h @@ -32,6 +32,16 @@ struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = { .soft_reset = 0x1c, }; +struct mv64xxx_i2c_regs mv64xxx_i2c_regs_sun4i = { + .addr = 0x00, + .ext_addr = 0x04, + .data = 0x08, + .control = 0x0c, + .status = 0x10, + .clock = 0x14, + .soft_reset = 0x18, +}; + /* i2c Platform Device, Driver Data */ struct mv64xxx_i2c_pdata { u32 freq_m;
Add the compatible string for the Allwinner A10 i2c controller and the associated register layout. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- drivers/i2c/busses/Kconfig | 3 ++- drivers/i2c/busses/i2c-mv64xxx.c | 1 + include/linux/mv643xx_i2c.h | 10 ++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-)