Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare <khali@linux-fr.org>]

Hi Ryan,

On Wed, 11 Jun 2008 15:12:44 +1200, Ryan Mallon wrote:
> Jean Delvare wrote:
> > Why don't you simply initialize the drivers in question with
> > subsys_initcall()? That's what i2c-pnx, i2c-omap, i2c-davinci and
> > tps65010 are doing at the moment.
>
> I still think its a bit nasty marking a driver as subsys_initcall 
> just because one particular setup needs it to be that way. We will
> eventually end up with half of the busses/drivers in i2c marked
> as subsys_initcall for random boards :-).
> 
> How about this patch instead, which replaces module_init and 
> subsys_initcalls in drivers/i2c with a new macro called
> i2c_module_init. If CONFIG_I2C_EARLY is set then it will #define
> to subsys_initcall, otherwise it becomes module_init. This way
> boards that need i2c early can just select CONFIG_I2C_EARLY and
> get all of i2c at subsys_initcall time. The link order should
> guarantee that everything still comes up in the correct order in
> the i2c driver subsystem.
> 
> I have tested this on an ARM ep93xx board with a bit-bashed
> i2c bus, a ds1339 rtc, and two max7311 IO expanders (using 
> the pca953x) both with and without CONFIG_I2C_EARLY_INIT selected.
> Of course it would need much more testing to verify it :-)

Making this a compile time option means that you need different kernels
for different machines, that's highly inconvenient. Or you end up using
the I2C_EARLY_INIT one for all systems, but then why have a
configuration option for it in the first place?

Which problem are you trying to solve? Is there any actual drawback to
abusing subsys_initcall() for the handful of i2c bus drivers which may
need to come up early?

>  drivers/i2c/Kconfig                    |    3 +++
>  drivers/i2c/busses/i2c-acorn.c         |    2 +-
>  drivers/i2c/busses/i2c-ali1535.c       |    2 +-
>  drivers/i2c/busses/i2c-ali1563.c       |    2 +-
>  drivers/i2c/busses/i2c-ali15x3.c       |    2 +-
>  drivers/i2c/busses/i2c-amd756-s4882.c  |    2 +-
>  drivers/i2c/busses/i2c-amd756.c        |    2 +-
>  drivers/i2c/busses/i2c-amd8111.c       |    2 +-
>  drivers/i2c/busses/i2c-at91.c          |    2 +-
>  drivers/i2c/busses/i2c-au1550.c        |    2 +-
>  drivers/i2c/busses/i2c-bfin-twi.c      |    2 +-
>  drivers/i2c/busses/i2c-davinci.c       |    2 +-
>  drivers/i2c/busses/i2c-elektor.c       |    2 +-
>  drivers/i2c/busses/i2c-gpio.c          |    2 +-
>  drivers/i2c/busses/i2c-hydra.c         |    2 +-
>  drivers/i2c/busses/i2c-i801.c          |    2 +-
>  drivers/i2c/busses/i2c-i810.c          |    2 +-
>  drivers/i2c/busses/i2c-ibm_iic.c       |    2 +-
>  drivers/i2c/busses/i2c-iop3xx.c        |    2 +-
>  drivers/i2c/busses/i2c-ixp2000.c       |    2 +-
>  drivers/i2c/busses/i2c-mpc.c           |    2 +-
>  drivers/i2c/busses/i2c-mv64xxx.c       |    2 +-
>  drivers/i2c/busses/i2c-nforce2.c       |    2 +-
>  drivers/i2c/busses/i2c-ocores.c        |    2 +-
>  drivers/i2c/busses/i2c-omap.c          |    2 +-
>  drivers/i2c/busses/i2c-parport-light.c |    2 +-
>  drivers/i2c/busses/i2c-parport.c       |    2 +-
>  drivers/i2c/busses/i2c-pasemi.c        |    2 +-
>  drivers/i2c/busses/i2c-pca-isa.c       |    2 +-
>  drivers/i2c/busses/i2c-pca-platform.c  |    2 +-
>  drivers/i2c/busses/i2c-piix4.c         |    2 +-
>  drivers/i2c/busses/i2c-pmcmsp.c        |    2 +-
>  drivers/i2c/busses/i2c-pnx.c           |    2 +-
>  drivers/i2c/busses/i2c-powermac.c      |    2 +-
>  drivers/i2c/busses/i2c-prosavage.c     |    2 +-
>  drivers/i2c/busses/i2c-pxa.c           |    2 +-
>  drivers/i2c/busses/i2c-s3c2410.c       |    2 +-
>  drivers/i2c/busses/i2c-savage4.c       |    2 +-
>  drivers/i2c/busses/i2c-sh7760.c        |    2 +-
>  drivers/i2c/busses/i2c-sh_mobile.c     |    2 +-
>  drivers/i2c/busses/i2c-sibyte.c        |    2 +-
>  drivers/i2c/busses/i2c-simtec.c        |    2 +-
>  drivers/i2c/busses/i2c-sis5595.c       |    2 +-
>  drivers/i2c/busses/i2c-sis630.c        |    2 +-
>  drivers/i2c/busses/i2c-sis96x.c        |    2 +-
>  drivers/i2c/busses/i2c-stub.c          |    2 +-
>  drivers/i2c/busses/i2c-taos-evm.c      |    2 +-
>  drivers/i2c/busses/i2c-tiny-usb.c      |    2 +-
>  drivers/i2c/busses/i2c-versatile.c     |    2 +-
>  drivers/i2c/busses/i2c-via.c           |    2 +-
>  drivers/i2c/busses/i2c-viapro.c        |    2 +-
>  drivers/i2c/busses/i2c-voodoo3.c       |    2 +-
>  drivers/i2c/busses/scx200_acb.c        |    2 +-
>  drivers/i2c/busses/scx200_i2c.c        |    2 +-

Bus drivers i2c-ali1535, i2c-ali1563, i2c-ali15x3, i2c-amd756-s4882,
i2c-amd756, i2c-amd8111, i2c-i801, i2c-nforce2, i2c-piix4, i2c-sis5595,
i2c-sis630, i2c-sis96x, i2c-via and i2c-viapro are for PC machines,
where I2C is never needed early.

Bus drivers i2c-i810, i2c-prosavage and i2c-savage4 are gone, no need
to touch them.

Bus drivers i2c-parport-light, i2c-parport, i2c-taos-evm and
i2c-tiny-usb are for external adapters, so initializing them early
isn't going to work... They depend on parport, serio and usb,
respectively.

i2c-stub is a software only driver for testing purposes, initializing
it early is pointless (it's really only useful as a module.)

>  drivers/i2c/chips/ds1682.c             |    2 +-
>  drivers/i2c/chips/eeprom.c             |    2 +-
>  drivers/i2c/chips/isp1301_omap.c       |    2 +-
>  drivers/i2c/chips/max6875.c            |    2 +-
>  drivers/i2c/chips/menelaus.c           |    2 +-
>  drivers/i2c/chips/pca9539.c            |    2 +-
>  drivers/i2c/chips/pcf8574.c            |    2 +-
>  drivers/i2c/chips/pcf8575.c            |    2 +-
>  drivers/i2c/chips/pcf8591.c            |    2 +-
>  drivers/i2c/chips/tps65010.c           |    2 +-
>  drivers/i2c/chips/tsl2550.c            |    2 +-

Most of these chip drivers only expose sysfs interfaces. Having them
initializing early is pointless. Only a few power management drivers
may be needed early: isp1301_omap, menelaus, tps65010.

>  drivers/i2c/i2c-dev.c                  |    2 +-

User-space interface, never needed early.

>  include/linux/i2c.h                    |    6 ++++++
>  67 files changed, 74 insertions(+), 65 deletions(-)

At the very least, you should only touch the drivers which you know
need to be touched, rather than all drivers "just in case". But I don't
think this is the way to go anyway, unless you can point out an actual
problem with using subsys_initcall() unconditionally.

-- 
Jean Delvare