| Message ID | 20090304215105.21101.88365.stgit@localhost (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Tony Lindgren |
| Headers | show |
* Tony Lindgren <tony@atomide.com> [090304 13:51]: > From: Jarkko Nikula <jarkko.nikula@nokia.com> > > This patch adds a new command line option "i2c_bus=bus_id,clkrate" into > I2C bus registration helper. Purpose of the option is to override the > default board specific bus speed which is supplied by the > omap_register_i2c_bus. > > The default bus speed is typically set to speed of slowest I2C chip on the > bus and overriding allow to use some experimental configurations or updated > chip versions without any kernel modifications. Jarkko, this should also be in Documentation/kernel-parameters.txt. Can you please reply with a patch for that, and I'll fold it into this patch? Also, maybe it should be called omap_i2c_bus instead of i2c_bus? Regards, Tony > Signed-off-by: Jarkko Nikula <jarkko.nikula@nokia.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > arch/arm/plat-omap/i2c.c | 54 +++++++++++++++++++++++++++++++++++++--------- > 1 files changed, 44 insertions(+), 10 deletions(-) > > diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c > index 3e95954..aa70e43 100644 > --- a/arch/arm/plat-omap/i2c.c > +++ b/arch/arm/plat-omap/i2c.c > @@ -119,6 +119,46 @@ static void __init omap_i2c_mux_pins(int bus) > omap_cfg_reg(scl); > } > > +static int __init omap_i2c_nr_ports(void) > +{ > + int ports = 0; > + > + if (cpu_class_is_omap1()) > + ports = 1; > + else if (cpu_is_omap24xx()) > + ports = 2; > + else if (cpu_is_omap34xx()) > + ports = 3; > + > + return ports; > +} > + > +/** > + * omap_i2c_bus_setup - Process command line options for the I2C bus speed > + * @str: String of options > + * > + * This function allow to override the default I2C bus speed for given I2C > + * bus with a command line option. > + * > + * Format: i2c_bus=bus_id,clkrate (in kHz) > + * > + * Returns 1 on success, 0 otherwise. > + */ > +static int __init omap_i2c_bus_setup(char *str) > +{ > + int ports; > + int ints[3]; > + > + ports = omap_i2c_nr_ports(); > + get_options(str, 3, ints); > + if (ints[0] < 2 || ints[1] < 1 || ints[1] > ports) > + return 0; > + i2c_rate[ints[1] - 1] = ints[2]; > + > + return 1; > +} > +__setup("i2c_bus=", omap_i2c_bus_setup); > + > /** > * omap_register_i2c_bus - register I2C bus with device descriptors > * @bus_id: bus id counting from number 1 > @@ -132,19 +172,12 @@ int __init omap_register_i2c_bus(int bus_id, u32 clkrate, > struct i2c_board_info const *info, > unsigned len) > { > - int ports, err; > + int err; > struct platform_device *pdev; > struct resource *res; > resource_size_t base, irq; > > - if (cpu_class_is_omap1()) > - ports = 1; > - else if (cpu_is_omap24xx()) > - ports = 2; > - else if (cpu_is_omap34xx()) > - ports = 3; > - > - BUG_ON(bus_id < 1 || bus_id > ports); > + BUG_ON(bus_id < 1 || bus_id > omap_i2c_nr_ports()); > > if (info) { > err = i2c_register_board_info(bus_id, info, len); > @@ -153,7 +186,8 @@ int __init omap_register_i2c_bus(int bus_id, u32 clkrate, > } > > pdev = &omap_i2c_devices[bus_id - 1]; > - *(u32 *)pdev->dev.platform_data = clkrate; > + if (i2c_rate[bus_id - 1] == 0) > + i2c_rate[bus_id - 1] = clkrate; > > if (bus_id == 1) { > res = pdev->resource; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" 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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 05, 2009 at 08:20:43AM -0800, Tony Lindgren wrote:
> Also, maybe it should be called omap_i2c_bus instead of i2c_bus?
or maybe not, then other archs would have the opportunity to setup their
own function to handle i2c_bus= cmdline without having to add a brand
new <arch>_i2c_bus one ;-)
On Thu, 5 Mar 2009 17:20:43 +0100 ext Tony Lindgren <tony@atomide.com> wrote: > Jarkko, this should also be in Documentation/kernel-parameters.txt. > Can you please reply with a patch for that, and I'll fold it into this > patch? > Ah, good, will do it over weekend - early next week. Probably better to handle as a separate patch for easier merging with kernel-parameters.txt? > Also, maybe it should be called omap_i2c_bus instead of i2c_bus? > I had similar thought as Felipe that it looks more generic this way. But don't know now immediately would multibuild will work? Was that your concern? E.g. __setup("i2c_bus=", arm_xxx_i2c_bus_setup); __setup("i2c_bus=", omap_i2c_bus_setup); Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Jarkko Nikula <jarkko.nikula@nokia.com> [090305 23:12]: > On Thu, 5 Mar 2009 17:20:43 +0100 > ext Tony Lindgren <tony@atomide.com> wrote: > > > Jarkko, this should also be in Documentation/kernel-parameters.txt. > > Can you please reply with a patch for that, and I'll fold it into this > > patch? > > > Ah, good, will do it over weekend - early next week. Probably better to > handle as a separate patch for easier merging with > kernel-parameters.txt? I think they should get merged as a single patch. > > Also, maybe it should be called omap_i2c_bus instead of i2c_bus? > > > I had similar thought as Felipe that it looks more generic this way. > But don't know now immediately would multibuild will work? Was that > your concern? E.g. > > __setup("i2c_bus=", arm_xxx_i2c_bus_setup); > __setup("i2c_bus=", omap_i2c_bus_setup); Well is it generic enough to work for everybody? And if so, we should run it by the LKML and the linux-i2c lists. Any comments from other ARM platforms? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi These patches are for linux-omap but we want to wide discussion also to linux-i2c is the new command line option "i2c_bus=bus_id,clkrate" generic enough to be used for other architectures as well in the future. Patches are generated on top of mainline 2.6.29-rc7. Purpose of this command line option is to both allow to override the default board specific bus speed (patch 2) and register additional busses that are not registered from board initialization code (patch 3). Example for patch 2 would be the case where some on board component is upgraded and which allows to use e.g. higher clock rate on the bus without other major board changes. Patch 3 can have more practical use however. E.g. second I2C bus on TI BeagleBoard doesn't have on board components and thus it is not registered by default with omap_register_i2c_bus. This bus is routed to expansion pin header so with this patch it is able to register dynamically when needed. These patches are previously discussed in these threads: http://marc.info/?l=linux-omap&m=123635604511892&w=2 http://marc.info/?l=linux-omap&m=123661915517228&w=2
On Fri, Mar 06, 2009 at 08:13:50AM -0800, Tony Lindgren wrote: > * Jarkko Nikula <jarkko.nikula@nokia.com> [090305 23:12]: > > On Thu, 5 Mar 2009 17:20:43 +0100 > > ext Tony Lindgren <tony@atomide.com> wrote: > > > > > Jarkko, this should also be in Documentation/kernel-parameters.txt. > > > Can you please reply with a patch for that, and I'll fold it into this > > > patch? > > > > > Ah, good, will do it over weekend - early next week. Probably better to > > handle as a separate patch for easier merging with > > kernel-parameters.txt? > > I think they should get merged as a single patch. > > > > Also, maybe it should be called omap_i2c_bus instead of i2c_bus? > > > > > I had similar thought as Felipe that it looks more generic this way. > > But don't know now immediately would multibuild will work? Was that > > your concern? E.g. > > > > __setup("i2c_bus=", arm_xxx_i2c_bus_setup); > > __setup("i2c_bus=", omap_i2c_bus_setup); > > Well is it generic enough to work for everybody? And if so, we should > run it by the LKML and the linux-i2c lists. Any comments from the I2C list? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 18 Mar 2009 20:30:45 +0100 ext Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > I had similar thought as Felipe that it looks more generic this > > > way. But don't know now immediately would multibuild will work? > > > Was that your concern? E.g. > > > > > > __setup("i2c_bus=", arm_xxx_i2c_bus_setup); > > > __setup("i2c_bus=", omap_i2c_bus_setup); > > > > Well is it generic enough to work for everybody? And if so, we > > should run it by the LKML and the linux-i2c lists. > > Any comments from the I2C list? > Unfortunately not. Are there some conference or why the list have been so idle recently? http://marc.info/?l=linux-i2c&r=1&b=200903&w=2 Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c index 3e95954..aa70e43 100644 --- a/arch/arm/plat-omap/i2c.c +++ b/arch/arm/plat-omap/i2c.c @@ -119,6 +119,46 @@ static void __init omap_i2c_mux_pins(int bus) omap_cfg_reg(scl); } +static int __init omap_i2c_nr_ports(void) +{ + int ports = 0; + + if (cpu_class_is_omap1()) + ports = 1; + else if (cpu_is_omap24xx()) + ports = 2; + else if (cpu_is_omap34xx()) + ports = 3; + + return ports; +} + +/** + * omap_i2c_bus_setup - Process command line options for the I2C bus speed + * @str: String of options + * + * This function allow to override the default I2C bus speed for given I2C + * bus with a command line option. + * + * Format: i2c_bus=bus_id,clkrate (in kHz) + * + * Returns 1 on success, 0 otherwise. + */ +static int __init omap_i2c_bus_setup(char *str) +{ + int ports; + int ints[3]; + + ports = omap_i2c_nr_ports(); + get_options(str, 3, ints); + if (ints[0] < 2 || ints[1] < 1 || ints[1] > ports) + return 0; + i2c_rate[ints[1] - 1] = ints[2]; + + return 1; +} +__setup("i2c_bus=", omap_i2c_bus_setup); + /** * omap_register_i2c_bus - register I2C bus with device descriptors * @bus_id: bus id counting from number 1 @@ -132,19 +172,12 @@ int __init omap_register_i2c_bus(int bus_id, u32 clkrate, struct i2c_board_info const *info, unsigned len) { - int ports, err; + int err; struct platform_device *pdev; struct resource *res; resource_size_t base, irq; - if (cpu_class_is_omap1()) - ports = 1; - else if (cpu_is_omap24xx()) - ports = 2; - else if (cpu_is_omap34xx()) - ports = 3; - - BUG_ON(bus_id < 1 || bus_id > ports); + BUG_ON(bus_id < 1 || bus_id > omap_i2c_nr_ports()); if (info) { err = i2c_register_board_info(bus_id, info, len); @@ -153,7 +186,8 @@ int __init omap_register_i2c_bus(int bus_id, u32 clkrate, } pdev = &omap_i2c_devices[bus_id - 1]; - *(u32 *)pdev->dev.platform_data = clkrate; + if (i2c_rate[bus_id - 1] == 0) + i2c_rate[bus_id - 1] = clkrate; if (bus_id == 1) { res = pdev->resource;