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

[David Brownell <david-b@pacbell.net>]

On Monday 05 November 2007, Jarkko Nikula wrote:
> 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?

Keep it simple.  If you're going to use a given i2c bus,
always have that setup routine mux it.  If board-specific
code does it too, that can't hurt ... and that board code
can be removed someday.


Your latest code goes:

> +/*
> + * REVISIT: How many boards really require muxing here? It should be done
> + * by the bootloader 

Erm, not as a rule.  The last policy I recall being discussed
was that Linux should not trust the boot loader to have set
up pin muxing.  If a product is being developed with a boot
loader which does that, fine -- resolve that by configuring
the pinmux support out of Linux.  That would be an OPTION.

But in all cases, the Linux code shold include pinmux support.
That way drivers etc can be properly debugged without needing
to depend on bootloader bugfixes of any type.


> 		and if not, then the muxing should go into those 
> + * board files. Since this is not known yet, do it here like how previous
> + * implementation was doing. However, eventually this function shall disappear + */

Again, no.  This would be the hook through which Linux always
ensures muxing is done correctly.  The exception would be that
you could compile out all pinmux support, using standard
conditional compilation tools (CONFIG_OMAP_MUX=n).


> +static void omap_i2c_mux_pins(int bus_id)
> +{
> +       switch (bus_id) {
> +       case 1:
> +               if (cpu_class_is_omap1()) {
> +                       omap_cfg_reg(I2C_SCL);
> +                       omap_cfg_reg(I2C_SDA);
> +               }

There should be an omap2 case here too, like

		if (cpu_is_omap24xx()) {
			omap_cfg_reg(M19_24XX_I2C1_SCL);
			omap_cfg_reg(L15_24XX_I2C1_SDA);
		}

I didn't check if that's the reset default, but again
the rule should be that Linux does *NOT* require a
fully-configuring and fully debugged boot loader to run.

If you've got such a boot loader, great -- you can disable
CONFIG_OMAP_MUX and the kernel will be somewhat smaller.
Meanwhile, not having such a boot loader should never be
a blocking factor in system development.

- Dave


> +               break;
> +       case 2:
> +               if (cpu_is_omap24xx()) {
> +                       omap_cfg_reg(J15_24XX_I2C2_SCL);
> +                       omap_cfg_reg(H19_24XX_I2C2_SDA);
> +               }
> +               break;
> +       }
> +}