Re: [PATCH 1/3] ARM: OMAP: Add helper module for board specific I2C bus registration

[Jarkko Nikula <jarkko.nikula@nokia.com>]

Thanks for review David. Comments below.

"ext David Brownell" <david-b@pacbell.net> wrote:

> > +
> > +/*
> > + * FIXME: This is mostly helper function for those boards that
> > don't do
> > + * I2C pin muxing by themself
> > + */
> > +static void omap_i2c_mux_pins(int bus_id)
> 
> So what's to FIX?  I'd think that it should be able to handle
> all the pinmux options.  Are there cases where boards NEED to
> mux the pins themselves?  (Like, a choice of ballout for either
> of the I2C signals on a given bus.)
> 
> This ought to be fully board-independant code...
> 
Exactly. This function was bit problematic since I don't know what
boards need muxing changes so I just collected here what
plat-omap/device.c and mach-omap2/device.c are doing currently.

Obviously better option is to move muxing into those board files which
require them. I don't think that every OMAP1 board require

	omap_cfg_reg(I2C_SCL);
	omap_cfg_reg(I2C_SDA);

nor every OMAP24xx for second I2C (except H4 and 2430sdp)

	omap_cfg_reg(J15_24XX_I2C2_SCL);
	omap_cfg_reg(H19_24XX_I2C2_SDA);

Do Tony or someone know which boards might require above muxing changes
so that we can get rid of that omap_i2c_mux_pins from here?

> > +void omap_register_i2c_bus(int bus_id, u32 clkrate)
> 
> Consider passing the i2c_board_info in here too, so that
> boards can just say "configure this bus at N kHz, with
> these devices".  Not that having two I2C init calls in
> each board's init logic unreasonable, but if you're going
> to clean up the I2C init logic then this would be another
> thing to clean up.  And it'd help get rid of potential
> errors, by grouping all the related items together.
> 
Good point. I'll think it is better to have all of these modifications
at once. Will add it.


    Jarkko