Re: [PATCH, RFC] Earlier I2C initialization

From: Jean Delvare
Date: Wed Jun 11 2008 - 03:41:21 EST

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/