[PATCH, RFC] Earlier I2C initialization

[PATCH, RFC] Earlier I2C initialization

[Ryan Mallon]

David Brownell wrote:
> On Monday 09 June 2008, Ryan Mallon wrote:
>   
>>> Talk to i2c and framebuffer people about changing the link order.
>>>
>>> i2c should really be initialised before framebuffer devices because
>>> framebuffer devices tend to want to read DDC from monitors, which is
>>> basically a I2C EEPROM in the monitor.
>>>
>>> ... but there's probably some reason why it's done the way it is today,
>>> and changing it could well cause stuff to break.
>>>       
>> We have made i2c the first driver subsystem to come up in our 2.6.20
>> kernel since we use i2c io expanders for power domain control. All we
>> did was change drivers/Makefile so that obj-$(CONFIG_I2C) += i2c/ is at
>> the very top of the file. We didn't have any problems with doing this.
>> YMMV of course.
>>     
>
> OMAP does much the same thing, for the same reason, and the I2C
> adapter gets initialized earlier too (so power management chips
> will be fully usable before driver_initcall code runs).
>
> Unless there's a downside on x86, I'd just suggest someone submit
> a patch moving I2C init "early" so it merges in 2.6.27 ... cc to
> LKML to scare out more potential problems, but I have a hard time
> imagining there'd really be any.
>
> - Dave
>   
Okay, heres the patch. Is untested though (other than our experience 
under 2.6.20), so it probably needs some people to test. I'm not 
subscribed to LKML, so can people CC me if necessary.

Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>

diff --git a/drivers/Makefile b/drivers/Makefile
index f65deda..9eaf236 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -5,6 +5,7 @@
 # Rewritten to use lists instead of if-statements.
 #
 
+obj-y				+= i2c/
 obj-$(CONFIG_HAVE_GPIO_LIB)	+= gpio/
 obj-$(CONFIG_PCI)		+= pci/
 obj-$(CONFIG_PARISC)		+= parisc/
@@ -61,7 +62,6 @@ obj-$(CONFIG_GAMEPORT)		+= input/gameport/
 obj-$(CONFIG_INPUT)		+= input/
 obj-$(CONFIG_I2O)		+= message/
 obj-$(CONFIG_RTC_LIB)		+= rtc/
-obj-y				+= i2c/
 obj-$(CONFIG_W1)		+= w1/
 obj-$(CONFIG_POWER_SUPPLY)	+= power/
 obj-$(CONFIG_HWMON)		+= hwmon/

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

On Tue, 10 Jun 2008 09:27:34 +1200, Ryan Mallon wrote:
> David Brownell wrote:
> > On Monday 09 June 2008, Ryan Mallon wrote:
> >   
> >>> Talk to i2c and framebuffer people about changing the link order.
> >>>
> >>> i2c should really be initialised before framebuffer devices because
> >>> framebuffer devices tend to want to read DDC from monitors, which is
> >>> basically a I2C EEPROM in the monitor.

This is already the case. i2c-core is initialized with
subsys_initcall(), so it's available to all drivers initialized with
module_init().

> >>> ... but there's probably some reason why it's done the way it is today,
> >>> and changing it could well cause stuff to break.
> >>>       
> >> We have made i2c the first driver subsystem to come up in our 2.6.20
> >> kernel since we use i2c io expanders for power domain control. All we
> >> did was change drivers/Makefile so that obj-$(CONFIG_I2C) += i2c/ is at
> >> the very top of the file. We didn't have any problems with doing this.
> >> YMMV of course.

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.

> > OMAP does much the same thing, for the same reason, and the I2C
> > adapter gets initialized earlier too (so power management chips
> > will be fully usable before driver_initcall code runs).
> >
> > Unless there's a downside on x86, I'd just suggest someone submit
> > a patch moving I2C init "early" so it merges in 2.6.27 ... cc to
> > LKML to scare out more potential problems, but I have a hard time
> > imagining there'd really be any.
>
> Okay, heres the patch. Is untested though (other than our experience 
> under 2.6.20), so it probably needs some people to test. I'm not 
> subscribed to LKML, so can people CC me if necessary.
> 
> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index f65deda..9eaf236 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -5,6 +5,7 @@
>  # Rewritten to use lists instead of if-statements.
>  #
>  
> +obj-y				+= i2c/
>  obj-$(CONFIG_HAVE_GPIO_LIB)	+= gpio/

Some i2c bus drivers bit-bang GPIO pins...

>  obj-$(CONFIG_PCI)		+= pci/

... and many are PCI devices, so will this work OK?

>  obj-$(CONFIG_PARISC)		+= parisc/
> @@ -61,7 +62,6 @@ obj-$(CONFIG_GAMEPORT)		+= input/gameport/
>  obj-$(CONFIG_INPUT)		+= input/
>  obj-$(CONFIG_I2O)		+= message/
>  obj-$(CONFIG_RTC_LIB)		+= rtc/
> -obj-y				+= i2c/
>  obj-$(CONFIG_W1)		+= w1/
>  obj-$(CONFIG_POWER_SUPPLY)	+= power/
>  obj-$(CONFIG_HWMON)		+= hwmon/


-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[Uli Luckas]

OT: David Brownell, I get bounces from your mail server refering me to a site 
that is down:
http://worldnet.att.net/general-info/bls_info/block_inquiry.html

On Tuesday, 10. June 2008, Ryan Mallon wrote:
> Jean Delvare wrote:
> > On Tue, 10 Jun 2008 09:27:34 +1200, Ryan Mallon wrote:
> >> David Brownell wrote:
> >>> On Monday 09 June 2008, Ryan Mallon wrote:
> >>>> We have made i2c the first driver subsystem to come up in our 2.6.20
> >>>> kernel since we use i2c io expanders for power domain control. All we
> >>>> did was change drivers/Makefile so that obj-$(CONFIG_I2C) += i2c/ is
> >>>> at the very top of the file. We didn't have any problems with doing
> >>>> this. YMMV of course.
> >
> > 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.
>
> How does this work for embedded devices where the same architecture is
> used in many different configurations? For example, we have a PXA270
> setup where we need i2c early, but many other PXA setups do not, so
> making i2c-pxa subsys_initcall to support a single board is maybe the
> wrong way to go?
>
Why would an architecture not want i2c to be available early? The only reason 
would be dependencies of the bus driver. These are constant over all pxa 
platforms though. So most likely the platforms that don't _need_ early i2c 
just don't care.

-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 64 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing directors: Hans-Peter Constien, Hubertus von Streit

Re: [PATCH, RFC] Earlier I2C initialization

[David Brownell]

> > >>> i2c should really be initialised before framebuffer devices because
> > >>> framebuffer devices tend to want to read DDC from monitors, which is
> > >>> basically a I2C EEPROM in the monitor.
> 
> This is already the case. i2c-core is initialized with
> subsys_initcall(), so it's available to all drivers initialized with
> module_init().

That's only one of many examples.  The more general reason to
want to change the position of I2C so it's early in the link
order (like many other "system" busses) is to ensure that I2C
drivers can rely on that initialization no matter where they
are in *link order* ... it's not just an init sequence issue.


> > >>> ... but there's probably some reason why it's done the way it is today,
> > >>> and changing it could well cause stuff to break.
> > >>>       
> > >> We have made i2c the first driver subsystem to come up in our 2.6.20
> > >> kernel since we use i2c io expanders for power domain control. All we
> > >> did was change drivers/Makefile so that obj-$(CONFIG_I2C) += i2c/ is at
> > >> the very top of the file. We didn't have any problems with doing this.
> > >> YMMV of course.
> 
> 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.

If they happen to sit outside the I2C tree and *before* it in
link order, things will misbehave.

Agreed init sequencing declarations should Do The Right Thing.
But those declarations are of two types:  *_initcall() stuff,
and link order.  We've seen cases where the initcalls alone are
insufficient.

> >  
> > +obj-y				+= i2c/
> >  obj-$(CONFIG_HAVE_GPIO_LIB)	+= gpio/
> 
> Some i2c bus drivers bit-bang GPIO pins...
> 
> >  obj-$(CONFIG_PCI)		+= pci/
> 
> ... and many are PCI devices, so will this work OK?

The gpiochip_add() function needed a bit of care to ensure that
platforms can use GPIOs well before tasking and IRQs work, and
thus before any initcalls run.  So that's not a problem.

Re PCI ... someone could investigate.


For the record, the OMAP tree puts I2C in the link order
later than this.  It wasn't moved earlier mostly out of
paranoia like yours.  (See below.  I'm not sure why "cbus"
is listed twice, or what OMAP boards have similar issues
with serio ...)

- Dave

# we also need input/serio early so serio bus is initialized by the time
# serial drivers start registering their serio ports
obj-$(CONFIG_SERIO)             += input/serio/
obj-y                           += serial/
obj-$(CONFIG_PARPORT)           += parport/
obj-y                           += base/ block/ misc/ mfd/ net/ media/ cbus/
obj-y                           += i2c/
obj-y                           += cbus/
obj-$(CONFIG_ARCH_OMAP)         += dsp/dspgateway/
obj-$(CONFIG_NUBUS)             += nubus/
obj-$(CONFIG_ATM)               += atm/
obj-y                           += macintosh/
obj-$(CONFIG_IDE)               += ide/
obj-$(CONFIG_SCSI)              += scsi/
obj-$(CONFIG_ATA)               += ata/
obj-$(CONFIG_FUSION)            += message/
 

Re: [PATCH, RFC] Earlier I2C initialization

[Ryan Mallon]

Jean Delvare wrote:
> On Tue, 10 Jun 2008 09:27:34 +1200, Ryan Mallon wrote:
>   
>> David Brownell wrote:
>>     
>>> On Monday 09 June 2008, Ryan Mallon wrote:
>>>   
>>>       
>>>>> Talk to i2c and framebuffer people about changing the link order.
>>>>>
>>>>> i2c should really be initialised before framebuffer devices because
>>>>> framebuffer devices tend to want to read DDC from monitors, which is
>>>>> basically a I2C EEPROM in the monitor.
>>>>>           
>
> This is already the case. i2c-core is initialized with
> subsys_initcall(), so it's available to all drivers initialized with
> module_init().
>
>   
>>>>> ... but there's probably some reason why it's done the way it is today,
>>>>> and changing it could well cause stuff to break.
>>>>>       
>>>>>           
>>>> We have made i2c the first driver subsystem to come up in our 2.6.20
>>>> kernel since we use i2c io expanders for power domain control. All we
>>>> did was change drivers/Makefile so that obj-$(CONFIG_I2C) += i2c/ is at
>>>> the very top of the file. We didn't have any problems with doing this.
>>>> YMMV of course.
>>>>         
>
> 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.
>
>   
How does this work for embedded devices where the same architecture is
used in many different configurations? For example, we have a PXA270 
setup where we need i2c early, but many other PXA setups do not, so 
making i2c-pxa subsys_initcall to support a single board is maybe the
wrong way to go?

>>> OMAP does much the same thing, for the same reason, and the I2C
>>> adapter gets initialized earlier too (so power management chips
>>> will be fully usable before driver_initcall code runs).
>>>
>>> Unless there's a downside on x86, I'd just suggest someone submit
>>> a patch moving I2C init "early" so it merges in 2.6.27 ... cc to
>>> LKML to scare out more potential problems, but I have a hard time
>>> imagining there'd really be any.
>>>       
>> Okay, heres the patch. Is untested though (other than our experience 
>> under 2.6.20), so it probably needs some people to test. I'm not 
>> subscribed to LKML, so can people CC me if necessary.
>>
>> Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>
>>
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index f65deda..9eaf236 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -5,6 +5,7 @@
>>  # Rewritten to use lists instead of if-statements.
>>  #
>>  
>> +obj-y				+= i2c/
>>  obj-$(CONFIG_HAVE_GPIO_LIB)	+= gpio/
>>     
>
> Some i2c bus drivers bit-bang GPIO pins...
>
>   
>>  obj-$(CONFIG_PCI)		+= pci/
>>     
>
> ... and many are PCI devices, so will this work OK?
>   
Probably not :-). I didn't have hardware to test, it was just easy
to put together the patch. I figured a change like this would 
require extensive testing anyway, since it is bound to break
some obscure setup at least.

I still think that possibly a better solution is to allow the link
order for the driver subsystems to be configured somehow. At least
for the embedded space this is useful if a particular board has 
some dependency on i2c, spi or some other subsystem being available
early on, then it can be configured on a per board basis, rather
than per arch, or per driver.

I'm not sure how to accomplish this though, I don't think Kconfig
lends it self to this sort of thing very well, and I don't
understand the kernel build process well enough to attempt it
myself.

~Ryan

Re: [PATCH, RFC] Earlier I2C initialization

[Ryan Mallon]

Jean Delvare wrote:
> On Tue, 10 Jun 2008 09:27:34 +1200, Ryan Mallon wrote:
>   
>> David Brownell wrote:
>>     
>>> On Monday 09 June 2008, Ryan Mallon wrote:
>>>   
>>>       
>>>>> Talk to i2c and framebuffer people about changing the link order.
>>>>>
>>>>> i2c should really be initialised before framebuffer devices because
>>>>> framebuffer devices tend to want to read DDC from monitors, which is
>>>>> basically a I2C EEPROM in the monitor.
>>>>>           
>
> This is already the case. i2c-core is initialized with
> subsys_initcall(), so it's available to all drivers initialized with
> module_init().
>
>   
>>>>> ... but there's probably some reason why it's done the way it is today,
>>>>> and changing it could well cause stuff to break.
>>>>>       
>>>>>           
>>>> We have made i2c the first driver subsystem to come up in our 2.6.20
>>>> kernel since we use i2c io expanders for power domain control. All we
>>>> did was change drivers/Makefile so that obj-$(CONFIG_I2C) += i2c/ is at
>>>> the very top of the file. We didn't have any problems with doing this.
>>>> YMMV of course.
>>>>         
>
> 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 :-)

~Ryan

Signed-off-by: Ryan Mallon <ryan@bluewatersys.com>

 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 +-
 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 +-
 drivers/i2c/i2c-dev.c                  |    2 +-
 include/linux/i2c.h                    |    6 ++++++
 67 files changed, 74 insertions(+), 65 deletions(-)

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 9686734..ab54207 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -23,6 +23,9 @@ menuconfig I2C
 
 if I2C
 
+config I2C_EARLY_INIT
+	boolean
+
 config I2C_BOARDINFO
 	boolean
 	default y
diff --git a/drivers/i2c/busses/i2c-acorn.c b/drivers/i2c/busses/i2c-acorn.c
index 7c2be35..fc4f941 100644
--- a/drivers/i2c/busses/i2c-acorn.c
+++ b/drivers/i2c/busses/i2c-acorn.c
@@ -94,4 +94,4 @@ static int __init i2c_ioc_init(void)
 	return i2c_bit_add_bus(&ioc_ops);
 }
 
-module_init(i2c_ioc_init);
+i2c_module_init(i2c_ioc_init);
diff --git a/drivers/i2c/busses/i2c-ali1535.c b/drivers/i2c/busses/i2c-ali1535.c
index f14372a..ae8e3e3 100644
--- a/drivers/i2c/busses/i2c-ali1535.c
+++ b/drivers/i2c/busses/i2c-ali1535.c
@@ -532,5 +532,5 @@ MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>, "
 MODULE_DESCRIPTION("ALI1535 SMBus driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_ali1535_init);
+i2c_module_init(i2c_ali1535_init);
 module_exit(i2c_ali1535_exit);
diff --git a/drivers/i2c/busses/i2c-ali1563.c b/drivers/i2c/busses/i2c-ali1563.c
index 6b68074..0cd833b 100644
--- a/drivers/i2c/busses/i2c-ali1563.c
+++ b/drivers/i2c/busses/i2c-ali1563.c
@@ -421,7 +421,7 @@ static int __init ali1563_init(void)
 	return pci_register_driver(&ali1563_pci_driver);
 }
 
-module_init(ali1563_init);
+i2c_module_init(ali1563_init);
 
 static void __exit ali1563_exit(void)
 {
diff --git a/drivers/i2c/busses/i2c-ali15x3.c b/drivers/i2c/busses/i2c-ali15x3.c
index 93bf87d..be1f5ed 100644
--- a/drivers/i2c/busses/i2c-ali15x3.c
+++ b/drivers/i2c/busses/i2c-ali15x3.c
@@ -526,5 +526,5 @@ MODULE_AUTHOR ("Frodo Looijaard <frodol@dds.nl>, "
 MODULE_DESCRIPTION("ALI15X3 SMBus driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_ali15x3_init);
+i2c_module_init(i2c_ali15x3_init);
 module_exit(i2c_ali15x3_exit);
diff --git a/drivers/i2c/busses/i2c-amd756-s4882.c b/drivers/i2c/busses/i2c-amd756-s4882.c
index c38a0a1..f3ec72b 100644
--- a/drivers/i2c/busses/i2c-amd756-s4882.c
+++ b/drivers/i2c/busses/i2c-amd756-s4882.c
@@ -260,5 +260,5 @@ MODULE_AUTHOR("Jean Delvare <khali@linux-fr.org>");
 MODULE_DESCRIPTION("S4882 SMBus multiplexing");
 MODULE_LICENSE("GPL");
 
-module_init(amd756_s4882_init);
+i2c_module_init(amd756_s4882_init);
 module_exit(amd756_s4882_exit);
diff --git a/drivers/i2c/busses/i2c-amd756.c b/drivers/i2c/busses/i2c-amd756.c
index 43508d6..9f7e541 100644
--- a/drivers/i2c/busses/i2c-amd756.c
+++ b/drivers/i2c/busses/i2c-amd756.c
@@ -428,5 +428,5 @@ MODULE_LICENSE("GPL");
 
 EXPORT_SYMBOL(amd756_smbus);
 
-module_init(amd756_init)
+i2c_module_init(amd756_init)
 module_exit(amd756_exit)
diff --git a/drivers/i2c/busses/i2c-amd8111.c b/drivers/i2c/busses/i2c-amd8111.c
index 5d1a27e..7e8309a 100644
--- a/drivers/i2c/busses/i2c-amd8111.c
+++ b/drivers/i2c/busses/i2c-amd8111.c
@@ -416,5 +416,5 @@ static void __exit i2c_amd8111_exit(void)
 	pci_unregister_driver(&amd8111_driver);
 }
 
-module_init(i2c_amd8111_init);
+i2c_module_init(i2c_amd8111_init);
 module_exit(i2c_amd8111_exit);
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 73d6194..f4a6a3a 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -321,7 +321,7 @@ static void __exit at91_i2c_exit(void)
 	platform_driver_unregister(&at91_i2c_driver);
 }
 
-module_init(at91_i2c_init);
+i2c_module_init(at91_i2c_init);
 module_exit(at91_i2c_exit);
 
 MODULE_AUTHOR("Rick Bronson");
diff --git a/drivers/i2c/busses/i2c-au1550.c b/drivers/i2c/busses/i2c-au1550.c
index cae9dc8..9acecc4 100644
--- a/drivers/i2c/busses/i2c-au1550.c
+++ b/drivers/i2c/busses/i2c-au1550.c
@@ -474,5 +474,5 @@ MODULE_DESCRIPTION("SMBus adapter Alchemy pb1550");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:au1xpsc_smbus");
 
-module_init (i2c_au1550_init);
+i2c_module_init (i2c_au1550_init);
 module_exit (i2c_au1550_exit);
diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 48d084b..caa8e74 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -735,7 +735,7 @@ static void __exit i2c_bfin_twi_exit(void)
 	platform_driver_unregister(&i2c_bfin_twi_driver);
 }
 
-module_init(i2c_bfin_twi_init);
+i2c_module_init(i2c_bfin_twi_init);
 module_exit(i2c_bfin_twi_exit);
 
 MODULE_AUTHOR("Bryan Wu, Sonic Zhang");
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 7ecbfc4..14ac794 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -570,7 +570,7 @@ static int __init davinci_i2c_init_driver(void)
 {
 	return platform_driver_register(&davinci_i2c_driver);
 }
-subsys_initcall(davinci_i2c_init_driver);
+i2c_module_init(davinci_i2c_init_driver);
 
 static void __exit davinci_i2c_exit_driver(void)
 {
diff --git a/drivers/i2c/busses/i2c-elektor.c b/drivers/i2c/busses/i2c-elektor.c
index b7a9977..d8efced 100644
--- a/drivers/i2c/busses/i2c-elektor.c
+++ b/drivers/i2c/busses/i2c-elektor.c
@@ -346,5 +346,5 @@ module_param(clock, int, 0);
 module_param(own, int, 0);
 module_param(mmapped, int, 0);
 
-module_init(i2c_pcfisa_init);
+i2c_module_init(i2c_pcfisa_init);
 module_exit(i2c_pcfisa_exit);
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index 7c1b762..fea7ccd 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -209,7 +209,7 @@ static int __init i2c_gpio_init(void)
 
 	return ret;
 }
-module_init(i2c_gpio_init);
+i2c_module_init(i2c_gpio_init);
 
 static void __exit i2c_gpio_exit(void)
 {
diff --git a/drivers/i2c/busses/i2c-hydra.c b/drivers/i2c/busses/i2c-hydra.c
index f9972f9..98a6d76 100644
--- a/drivers/i2c/busses/i2c-hydra.c
+++ b/drivers/i2c/busses/i2c-hydra.c
@@ -177,6 +177,6 @@ MODULE_AUTHOR("Geert Uytterhoeven <geert@linux-m68k.org>");
 MODULE_DESCRIPTION("i2c for Apple Hydra Mac I/O");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_hydra_init);
+i2c_module_init(i2c_hydra_init);
 module_exit(i2c_hydra_exit);
 
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b0f771f..148391c 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -739,5 +739,5 @@ MODULE_AUTHOR("Mark D. Studebaker <mdsxyz123@yahoo.com>, "
 MODULE_DESCRIPTION("I801 SMBus driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_i801_init);
+i2c_module_init(i2c_i801_init);
 module_exit(i2c_i801_exit);
diff --git a/drivers/i2c/busses/i2c-i810.c b/drivers/i2c/busses/i2c-i810.c
index 42e8d94..c14d67d 100644
--- a/drivers/i2c/busses/i2c-i810.c
+++ b/drivers/i2c/busses/i2c-i810.c
@@ -256,5 +256,5 @@ MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>, "
 MODULE_DESCRIPTION("I810/I815 I2C/DDC driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_i810_init);
+i2c_module_init(i2c_i810_init);
 module_exit(i2c_i810_exit);
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 85dbf34..511e9d5 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -1013,5 +1013,5 @@ static void __exit iic_exit(void)
 }
 #endif /* CONFIG_IBM_OCP */
 
-module_init(iic_init);
+i2c_module_init(iic_init);
 module_exit(iic_exit);
diff --git a/drivers/i2c/busses/i2c-iop3xx.c b/drivers/i2c/busses/i2c-iop3xx.c
index 39884e7..d494417 100644
--- a/drivers/i2c/busses/i2c-iop3xx.c
+++ b/drivers/i2c/busses/i2c-iop3xx.c
@@ -544,7 +544,7 @@ i2c_iop3xx_exit (void)
 	return;
 }
 
-module_init (i2c_iop3xx_init);
+i2c_module_init (i2c_iop3xx_init);
 module_exit (i2c_iop3xx_exit);
 
 MODULE_AUTHOR("D-TACQ Solutions Ltd <www.d-tacq.com>");
diff --git a/drivers/i2c/busses/i2c-ixp2000.c b/drivers/i2c/busses/i2c-ixp2000.c
index 5af9e65..4e549a6 100644
--- a/drivers/i2c/busses/i2c-ixp2000.c
+++ b/drivers/i2c/busses/i2c-ixp2000.c
@@ -158,7 +158,7 @@ static void __exit ixp2000_i2c_exit(void)
 	platform_driver_unregister(&ixp2000_i2c_driver);
 }
 
-module_init(ixp2000_i2c_init);
+i2c_module_init(ixp2000_i2c_init);
 module_exit(ixp2000_i2c_exit);
 
 MODULE_AUTHOR ("Deepak Saxena <dsaxena@plexity.net>");
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index a076129..34dd681 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -413,7 +413,7 @@ static void __exit fsl_i2c_exit(void)
 	platform_driver_unregister(&fsl_i2c_driver);
 }
 
-module_init(fsl_i2c_init);
+i2c_module_init(fsl_i2c_init);
 module_exit(fsl_i2c_exit);
 
 MODULE_AUTHOR("Adrian Cox <adrian@humboldt.co.uk>");
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 036e6a8..07c7047 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -597,7 +597,7 @@ mv64xxx_i2c_exit(void)
 	platform_driver_unregister(&mv64xxx_i2c_driver);
 }
 
-module_init(mv64xxx_i2c_init);
+i2c_module_init(mv64xxx_i2c_init);
 module_exit(mv64xxx_i2c_exit);
 
 MODULE_AUTHOR("Mark A. Greer <mgreer@mvista.com>");
diff --git a/drivers/i2c/busses/i2c-nforce2.c b/drivers/i2c/busses/i2c-nforce2.c
index 43c9f8d..f927d08 100644
--- a/drivers/i2c/busses/i2c-nforce2.c
+++ b/drivers/i2c/busses/i2c-nforce2.c
@@ -434,6 +434,6 @@ static void __exit nforce2_exit(void)
 	pci_unregister_driver(&nforce2_driver);
 }
 
-module_init(nforce2_init);
+i2c_module_init(nforce2_init);
 module_exit(nforce2_exit);
 
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index f145692..c50f966 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -334,7 +334,7 @@ static void __exit ocores_i2c_exit(void)
 	platform_driver_unregister(&ocores_i2c_driver);
 }
 
-module_init(ocores_i2c_init);
+i2c_module_init(ocores_i2c_init);
 module_exit(ocores_i2c_exit);
 
 MODULE_AUTHOR("Peter Korsgaard <jacmet@sunsite.dk>");
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index e7eb7bf..abadcbe 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -682,7 +682,7 @@ omap_i2c_init_driver(void)
 {
 	return platform_driver_register(&omap_i2c_driver);
 }
-subsys_initcall(omap_i2c_init_driver);
+i2c_module_init(omap_i2c_init_driver);
 
 static void __exit omap_i2c_exit_driver(void)
 {
diff --git a/drivers/i2c/busses/i2c-parport-light.c b/drivers/i2c/busses/i2c-parport-light.c
index c6faf9b..73e3a34 100644
--- a/drivers/i2c/busses/i2c-parport-light.c
+++ b/drivers/i2c/busses/i2c-parport-light.c
@@ -261,5 +261,5 @@ MODULE_AUTHOR("Jean Delvare <khali@linux-fr.org>");
 MODULE_DESCRIPTION("I2C bus over parallel port (light)");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_parport_init);
+i2c_module_init(i2c_parport_init);
 module_exit(i2c_parport_exit);
diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
index 59ba208..1a786d0 100644
--- a/drivers/i2c/busses/i2c-parport.c
+++ b/drivers/i2c/busses/i2c-parport.c
@@ -264,5 +264,5 @@ MODULE_AUTHOR("Jean Delvare <khali@linux-fr.org>");
 MODULE_DESCRIPTION("I2C bus over parallel port");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_parport_init);
+i2c_module_init(i2c_parport_init);
 module_exit(i2c_parport_exit);
diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index 1603c81..c50a388 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -428,5 +428,5 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR ("Olof Johansson <olof@lixom.net>");
 MODULE_DESCRIPTION("PA Semi PWRficient SMBus driver");
 
-module_init(pasemi_smb_init);
+i2c_module_init(pasemi_smb_init);
 module_exit(pasemi_smb_exit);
diff --git a/drivers/i2c/busses/i2c-pca-isa.c b/drivers/i2c/busses/i2c-pca-isa.c
index a119784..c8bb260 100644
--- a/drivers/i2c/busses/i2c-pca-isa.c
+++ b/drivers/i2c/busses/i2c-pca-isa.c
@@ -193,5 +193,5 @@ MODULE_PARM_DESC(irq, "IRQ");
 module_param(clock, int, 0);
 MODULE_PARM_DESC(clock, "Clock rate as described in table 1 of PCA9564 datasheet");
 
-module_init(pca_isa_init);
+i2c_module_init(pca_isa_init);
 module_exit(pca_isa_exit);
diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
index 9d75f51..53e3f15 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -293,6 +293,6 @@ MODULE_AUTHOR("Wolfram Sang <w.sang@pengutronix.de>");
 MODULE_DESCRIPTION("I2C-PCA9564 platform driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_pca_pf_init);
+i2c_module_init(i2c_pca_pf_init);
 module_exit(i2c_pca_pf_exit);
 
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index ac91659..0e89912 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -491,5 +491,5 @@ MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl> and "
 MODULE_DESCRIPTION("PIIX4 SMBus driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_piix4_init);
+i2c_module_init(i2c_piix4_init);
 module_exit(i2c_piix4_exit);
diff --git a/drivers/i2c/busses/i2c-pmcmsp.c b/drivers/i2c/busses/i2c-pmcmsp.c
index 63b3e2c..b155077 100644
--- a/drivers/i2c/busses/i2c-pmcmsp.c
+++ b/drivers/i2c/busses/i2c-pmcmsp.c
@@ -652,5 +652,5 @@ static void __exit pmcmsptwi_exit(void)
 MODULE_DESCRIPTION("PMC MSP TWI/SMBus/I2C driver");
 MODULE_LICENSE("GPL");
 
-module_init(pmcmsptwi_init);
+i2c_module_init(pmcmsptwi_init);
 module_exit(pmcmsptwi_exit);
diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index 1ca2108..82e86a8 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -700,5 +700,5 @@ MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:pnx-i2c");
 
 /* We need to make sure I2C is initialized before USB */
-subsys_initcall(i2c_adap_pnx_init);
+i2c_module_init(i2c_adap_pnx_init);
 module_exit(i2c_adap_pnx_exit);
diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
index 22f6d5c..a2fda94 100644
--- a/drivers/i2c/busses/i2c-powermac.c
+++ b/drivers/i2c/busses/i2c-powermac.c
@@ -287,5 +287,5 @@ static void __exit i2c_powermac_cleanup(void)
 	platform_driver_unregister(&i2c_powermac_driver);
 }
 
-module_init(i2c_powermac_init);
+i2c_module_init(i2c_powermac_init);
 module_exit(i2c_powermac_cleanup);
diff --git a/drivers/i2c/busses/i2c-prosavage.c b/drivers/i2c/busses/i2c-prosavage.c
index 07c1f1e..dd8cb43 100644
--- a/drivers/i2c/busses/i2c-prosavage.c
+++ b/drivers/i2c/busses/i2c-prosavage.c
@@ -321,5 +321,5 @@ MODULE_AUTHOR("Henk Vergonet");
 MODULE_DESCRIPTION("ProSavage VIA 8365/8375 smbus driver");
 MODULE_LICENSE("GPL");
 
-module_init (i2c_prosavage_init);
+i2c_module_init (i2c_prosavage_init);
 module_exit (i2c_prosavage_exit);
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index eb69fba..e34bf3c 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1134,5 +1134,5 @@ static void __exit i2c_adap_pxa_exit(void)
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:pxa2xx-i2c");
 
-module_init(i2c_adap_pxa_init);
+i2c_module_init(i2c_adap_pxa_init);
 module_exit(i2c_adap_pxa_exit);
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 1305ef1..a89b670 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -942,7 +942,7 @@ static void __exit i2c_adap_s3c_exit(void)
 	platform_driver_unregister(&s3c2440_i2c_driver);
 }
 
-module_init(i2c_adap_s3c_init);
+i2c_module_init(i2c_adap_s3c_init);
 module_exit(i2c_adap_s3c_exit);
 
 MODULE_DESCRIPTION("S3C24XX I2C Bus driver");
diff --git a/drivers/i2c/busses/i2c-savage4.c b/drivers/i2c/busses/i2c-savage4.c
index 8adf4ab..9e7e4c8 100644
--- a/drivers/i2c/busses/i2c-savage4.c
+++ b/drivers/i2c/busses/i2c-savage4.c
@@ -181,5 +181,5 @@ MODULE_AUTHOR("Alexander Wold <awold@bigfoot.com> "
 MODULE_DESCRIPTION("Savage4 I2C/SMBus driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_savage4_init);
+i2c_module_init(i2c_savage4_init);
 module_exit(i2c_savage4_exit);
diff --git a/drivers/i2c/busses/i2c-sh7760.c b/drivers/i2c/busses/i2c-sh7760.c
index 5e0e254..62ed4a4 100644
--- a/drivers/i2c/busses/i2c-sh7760.c
+++ b/drivers/i2c/busses/i2c-sh7760.c
@@ -569,7 +569,7 @@ static void __exit sh7760_i2c_exit(void)
 	platform_driver_unregister(&sh7760_i2c_drv);
 }
 
-module_init(sh7760_i2c_init);
+i2c_module_init(sh7760_i2c_init);
 module_exit(sh7760_i2c_exit);
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 840e634..ccf4185 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -492,7 +492,7 @@ static void __exit sh_mobile_i2c_adap_exit(void)
 	platform_driver_unregister(&sh_mobile_i2c_driver);
 }
 
-module_init(sh_mobile_i2c_adap_init);
+i2c_module_init(sh_mobile_i2c_adap_init);
 module_exit(sh_mobile_i2c_adap_exit);
 
 MODULE_DESCRIPTION("SuperH Mobile I2C Bus Controller driver");
diff --git a/drivers/i2c/busses/i2c-sibyte.c b/drivers/i2c/busses/i2c-sibyte.c
index 114634d..de68ed6 100644
--- a/drivers/i2c/busses/i2c-sibyte.c
+++ b/drivers/i2c/busses/i2c-sibyte.c
@@ -190,7 +190,7 @@ static void __exit i2c_sibyte_exit(void)
 	i2c_del_adapter(&sibyte_board_adapter[1]);
 }
 
-module_init(i2c_sibyte_init);
+i2c_module_init(i2c_sibyte_init);
 module_exit(i2c_sibyte_exit);
 
 MODULE_AUTHOR("Kip Walker (Broadcom Corp.), Steven J. Hill <sjhill@realitydiluted.com>");
diff --git a/drivers/i2c/busses/i2c-simtec.c b/drivers/i2c/busses/i2c-simtec.c
index 042fda2..55f918e 100644
--- a/drivers/i2c/busses/i2c-simtec.c
+++ b/drivers/i2c/busses/i2c-simtec.c
@@ -181,7 +181,7 @@ static void __exit i2c_adap_simtec_exit(void)
 	platform_driver_unregister(&simtec_i2c_driver);
 }
 
-module_init(i2c_adap_simtec_init);
+i2c_module_init(i2c_adap_simtec_init);
 module_exit(i2c_adap_simtec_exit);
 
 MODULE_DESCRIPTION("Simtec Generic I2C Bus driver");
diff --git a/drivers/i2c/busses/i2c-sis5595.c b/drivers/i2c/busses/i2c-sis5595.c
index 9ca8f91..3d9f7fe 100644
--- a/drivers/i2c/busses/i2c-sis5595.c
+++ b/drivers/i2c/busses/i2c-sis5595.c
@@ -424,5 +424,5 @@ MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
 MODULE_DESCRIPTION("SIS5595 SMBus driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_sis5595_init);
+i2c_module_init(i2c_sis5595_init);
 module_exit(i2c_sis5595_exit);
diff --git a/drivers/i2c/busses/i2c-sis630.c b/drivers/i2c/busses/i2c-sis630.c
index 3765dd7..986342e 100644
--- a/drivers/i2c/busses/i2c-sis630.c
+++ b/drivers/i2c/busses/i2c-sis630.c
@@ -519,5 +519,5 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Alexander Malysh <amalysh@web.de>");
 MODULE_DESCRIPTION("SIS630 SMBus driver");
 
-module_init(i2c_sis630_init);
+i2c_module_init(i2c_sis630_init);
 module_exit(i2c_sis630_exit);
diff --git a/drivers/i2c/busses/i2c-sis96x.c b/drivers/i2c/busses/i2c-sis96x.c
index dc235bb..3f50c88 100644
--- a/drivers/i2c/busses/i2c-sis96x.c
+++ b/drivers/i2c/busses/i2c-sis96x.c
@@ -343,6 +343,6 @@ MODULE_DESCRIPTION("SiS96x SMBus driver");
 MODULE_LICENSE("GPL");
 
 /* Register initialization functions using helper macros */
-module_init(i2c_sis96x_init);
+i2c_module_init(i2c_sis96x_init);
 module_exit(i2c_sis96x_exit);
 
diff --git a/drivers/i2c/busses/i2c-stub.c b/drivers/i2c/busses/i2c-stub.c
index d08eeec..e68e6d8 100644
--- a/drivers/i2c/busses/i2c-stub.c
+++ b/drivers/i2c/busses/i2c-stub.c
@@ -188,6 +188,6 @@ MODULE_AUTHOR("Mark M. Hoffman <mhoffman@lightlink.com>");
 MODULE_DESCRIPTION("I2C stub driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_stub_init);
+i2c_module_init(i2c_stub_init);
 module_exit(i2c_stub_exit);
 
diff --git a/drivers/i2c/busses/i2c-taos-evm.c b/drivers/i2c/busses/i2c-taos-evm.c
index de9db49..5b56a98 100644
--- a/drivers/i2c/busses/i2c-taos-evm.c
+++ b/drivers/i2c/busses/i2c-taos-evm.c
@@ -325,5 +325,5 @@ MODULE_AUTHOR("Jean Delvare <khali@linux-fr.org>");
 MODULE_DESCRIPTION("TAOS evaluation module driver");
 MODULE_LICENSE("GPL");
 
-module_init(taos_init);
+i2c_module_init(taos_init);
 module_exit(taos_exit);
diff --git a/drivers/i2c/busses/i2c-tiny-usb.c b/drivers/i2c/busses/i2c-tiny-usb.c
index b1c050f..963be25 100644
--- a/drivers/i2c/busses/i2c-tiny-usb.c
+++ b/drivers/i2c/busses/i2c-tiny-usb.c
@@ -271,7 +271,7 @@ static void __exit usb_i2c_tiny_usb_exit(void)
 	usb_deregister(&i2c_tiny_usb_driver);
 }
 
-module_init(usb_i2c_tiny_usb_init);
+i2c_module_init(usb_i2c_tiny_usb_init);
 module_exit(usb_i2c_tiny_usb_exit);
 
 /* ----- end of usb layer ------------------------------------------------ */
diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
index 4678bab..9d478b9 100644
--- a/drivers/i2c/busses/i2c-versatile.c
+++ b/drivers/i2c/busses/i2c-versatile.c
@@ -146,7 +146,7 @@ static void __exit i2c_versatile_exit(void)
 	platform_driver_unregister(&i2c_versatile_driver);
 }
 
-module_init(i2c_versatile_init);
+i2c_module_init(i2c_versatile_init);
 module_exit(i2c_versatile_exit);
 
 MODULE_DESCRIPTION("ARM Versatile I2C bus driver");
diff --git a/drivers/i2c/busses/i2c-via.c b/drivers/i2c/busses/i2c-via.c
index 61716f6..bb064cd 100644
--- a/drivers/i2c/busses/i2c-via.c
+++ b/drivers/i2c/busses/i2c-via.c
@@ -180,5 +180,5 @@ MODULE_AUTHOR("Kyösti Mälkki <kmalkki@cc.hut.fi>");
 MODULE_DESCRIPTION("i2c for Via vt82c586b southbridge");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_vt586b_init);
+i2c_module_init(i2c_vt586b_init);
 module_exit(i2c_vt586b_exit);
diff --git a/drivers/i2c/busses/i2c-viapro.c b/drivers/i2c/busses/i2c-viapro.c
index 77b13d0..7eb9724 100644
--- a/drivers/i2c/busses/i2c-viapro.c
+++ b/drivers/i2c/busses/i2c-viapro.c
@@ -489,5 +489,5 @@ MODULE_AUTHOR("Kyosti Malkki <kmalkki@cc.hut.fi>, "
 MODULE_DESCRIPTION("vt82c596 SMBus driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_vt596_init);
+i2c_module_init(i2c_vt596_init);
 module_exit(i2c_vt596_exit);
diff --git a/drivers/i2c/busses/i2c-voodoo3.c b/drivers/i2c/busses/i2c-voodoo3.c
index 88a3447..86a0430 100644
--- a/drivers/i2c/busses/i2c-voodoo3.c
+++ b/drivers/i2c/busses/i2c-voodoo3.c
@@ -249,5 +249,5 @@ MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>, "
 MODULE_DESCRIPTION("Voodoo3 I2C/SMBus driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_voodoo3_init);
+i2c_module_init(i2c_voodoo3_init);
 module_exit(i2c_voodoo3_exit);
diff --git a/drivers/i2c/busses/scx200_acb.c b/drivers/i2c/busses/scx200_acb.c
index 61abe0f..624e6b6 100644
--- a/drivers/i2c/busses/scx200_acb.c
+++ b/drivers/i2c/busses/scx200_acb.c
@@ -651,5 +651,5 @@ static void __exit scx200_acb_cleanup(void)
 	mutex_unlock(&scx200_acb_list_mutex);
 }
 
-module_init(scx200_acb_init);
+i2c_module_init(scx200_acb_init);
 module_exit(scx200_acb_cleanup);
diff --git a/drivers/i2c/busses/scx200_i2c.c b/drivers/i2c/busses/scx200_i2c.c
index c3022a0..e8458ef 100644
--- a/drivers/i2c/busses/scx200_i2c.c
+++ b/drivers/i2c/busses/scx200_i2c.c
@@ -120,7 +120,7 @@ static void scx200_i2c_cleanup(void)
 	i2c_del_adapter(&scx200_i2c_ops);
 }
 
-module_init(scx200_i2c_init);
+i2c_module_init(scx200_i2c_init);
 module_exit(scx200_i2c_cleanup);
 
 /*
diff --git a/drivers/i2c/chips/ds1682.c b/drivers/i2c/chips/ds1682.c
index 23be4d4..cf6dd46 100644
--- a/drivers/i2c/chips/ds1682.c
+++ b/drivers/i2c/chips/ds1682.c
@@ -264,5 +264,5 @@ MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
 MODULE_DESCRIPTION("DS1682 Elapsed Time Indicator driver");
 MODULE_LICENSE("GPL");
 
-module_init(ds1682_init);
+i2c_module_init(ds1682_init);
 module_exit(ds1682_exit);
diff --git a/drivers/i2c/chips/eeprom.c b/drivers/i2c/chips/eeprom.c
index 7dee001..c04232a 100644
--- a/drivers/i2c/chips/eeprom.c
+++ b/drivers/i2c/chips/eeprom.c
@@ -266,5 +266,5 @@ MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl> and "
 MODULE_DESCRIPTION("I2C EEPROM driver");
 MODULE_LICENSE("GPL");
 
-module_init(eeprom_init);
+i2c_module_init(eeprom_init);
 module_exit(eeprom_exit);
diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index b1b45dd..548b85b 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -1638,7 +1638,7 @@ static int __init isp_init(void)
 {
 	return i2c_add_driver(&isp1301_driver);
 }
-module_init(isp_init);
+i2c_module_init(isp_init);
 
 static void __exit isp_exit(void)
 {
diff --git a/drivers/i2c/chips/max6875.c b/drivers/i2c/chips/max6875.c
index cf507b3..9356935 100644
--- a/drivers/i2c/chips/max6875.c
+++ b/drivers/i2c/chips/max6875.c
@@ -266,5 +266,5 @@ MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
 MODULE_DESCRIPTION("MAX6875 driver");
 MODULE_LICENSE("GPL");
 
-module_init(max6875_init);
+i2c_module_init(max6875_init);
 module_exit(max6875_exit);
diff --git a/drivers/i2c/chips/menelaus.c b/drivers/i2c/chips/menelaus.c
index b36db17..0866663 100644
--- a/drivers/i2c/chips/menelaus.c
+++ b/drivers/i2c/chips/menelaus.c
@@ -1282,5 +1282,5 @@ MODULE_AUTHOR("Texas Instruments, Inc. (and others)");
 MODULE_DESCRIPTION("I2C interface for Menelaus.");
 MODULE_LICENSE("GPL");
 
-module_init(menelaus_init);
+i2c_module_init(menelaus_init);
 module_exit(menelaus_exit);
diff --git a/drivers/i2c/chips/pca9539.c b/drivers/i2c/chips/pca9539.c
index f43c4e7..0b99b16 100644
--- a/drivers/i2c/chips/pca9539.c
+++ b/drivers/i2c/chips/pca9539.c
@@ -191,6 +191,6 @@ MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
 MODULE_DESCRIPTION("PCA9539 driver");
 MODULE_LICENSE("GPL");
 
-module_init(pca9539_init);
+i2c_module_init(pca9539_init);
 module_exit(pca9539_exit);
 
diff --git a/drivers/i2c/chips/pcf8574.c b/drivers/i2c/chips/pcf8574.c
index e5b3132..9d52a57 100644
--- a/drivers/i2c/chips/pcf8574.c
+++ b/drivers/i2c/chips/pcf8574.c
@@ -230,5 +230,5 @@ MODULE_AUTHOR
 MODULE_DESCRIPTION("PCF8574 driver");
 MODULE_LICENSE("GPL");
 
-module_init(pcf8574_init);
+i2c_module_init(pcf8574_init);
 module_exit(pcf8574_exit);
diff --git a/drivers/i2c/chips/pcf8575.c b/drivers/i2c/chips/pcf8575.c
index 3ea08ac..36c4ebb 100644
--- a/drivers/i2c/chips/pcf8575.c
+++ b/drivers/i2c/chips/pcf8575.c
@@ -210,5 +210,5 @@ MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>, "
 MODULE_DESCRIPTION("pcf8575 driver");
 MODULE_LICENSE("GPL");
 
-module_init(pcf8575_init);
+i2c_module_init(pcf8575_init);
 module_exit(pcf8575_exit);
diff --git a/drivers/i2c/chips/pcf8591.c b/drivers/i2c/chips/pcf8591.c
index 66c7c3b..bb6713f 100644
--- a/drivers/i2c/chips/pcf8591.c
+++ b/drivers/i2c/chips/pcf8591.c
@@ -338,5 +338,5 @@ MODULE_AUTHOR("Aurelien Jarno <aurelien@aurel32.net>");
 MODULE_DESCRIPTION("PCF8591 driver");
 MODULE_LICENSE("GPL");
 
-module_init(pcf8591_init);
+i2c_module_init(pcf8591_init);
 module_exit(pcf8591_exit);
diff --git a/drivers/i2c/chips/tps65010.c b/drivers/i2c/chips/tps65010.c
index 8594968..a86a4d4 100644
--- a/drivers/i2c/chips/tps65010.c
+++ b/drivers/i2c/chips/tps65010.c
@@ -1054,7 +1054,7 @@ static int __init tps_init(void)
  * That is, much earlier than on PC-type systems, which don't often use
  * I2C as a core system bus.
  */
-subsys_initcall(tps_init);
+i2c_module_init(tps_init);
 
 static void __exit tps_exit(void)
 {
diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
index 1a9cc13..7007e17 100644
--- a/drivers/i2c/chips/tsl2550.c
+++ b/drivers/i2c/chips/tsl2550.c
@@ -485,5 +485,5 @@ MODULE_DESCRIPTION("TSL2550 ambient light sensor driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRIVER_VERSION);
 
-module_init(tsl2550_init);
+i2c_module_init(tsl2550_init);
 module_exit(tsl2550_exit);
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index d34c14c..626b7b4 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -611,5 +611,5 @@ MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl> and "
 MODULE_DESCRIPTION("I2C /dev entries driver");
 MODULE_LICENSE("GPL");
 
-module_init(i2c_dev_init);
+i2c_module_init(i2c_dev_init);
 module_exit(i2c_dev_exit);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index fb9af6a..86283bb 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -556,6 +556,12 @@ union i2c_smbus_data {
 
 #ifdef __KERNEL__
 
+#ifdef CONFIG_I2C_EARLY_INIT
+#define i2c_module_init subsys_initcall
+#else
+#define i2c_module_init module_init
+#endif
+
 /* These defines are used for probing i2c client addresses */
 /* The length of the option lists */
 #define I2C_CLIENT_MAX_OPTS 48

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

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

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

Hi David,

On Tue, 10 Jun 2008 13:55:07 -0700, David Brownell 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.
> 
> If they happen to sit outside the I2C tree and *before* it in
> link order, things will misbehave.

Well, i2c system bus drivers shouldn't sit outside of the I2C tree, so
that's not a problem. If you start accepting that drivers live at
random places in the source tree, then there's simply no way to get
things right.

As for i2c chip drivers, that's the exact reason why I am urging
developers to create function-based subsystems rather than dumping
their drivers in drivers/i2c/chips. If we have all drivers implementing
a given function in a dedicated directory then we can sequence this
directory properly in the link order.

> Agreed init sequencing declarations should Do The Right Thing.
> But those declarations are of two types:  *_initcall() stuff,
> and link order.  We've seen cases where the initcalls alone are
> insufficient.
> 
> > >  
> > > +obj-y				+= i2c/
> > >  obj-$(CONFIG_HAVE_GPIO_LIB)	+= gpio/
> > 
> > Some i2c bus drivers bit-bang GPIO pins...
> > 
> > >  obj-$(CONFIG_PCI)		+= pci/
> > 
> > ... and many are PCI devices, so will this work OK?
> 
> The gpiochip_add() function needed a bit of care to ensure that
> platforms can use GPIOs well before tasking and IRQs work, and
> thus before any initcalls run.  So that's not a problem.
> 
> Re PCI ... someone could investigate.

These were only two examples. We have i2c bus drivers depending on PCI,
parport, USB, ISA, GPIO and serio. Given the current linking order,
this makes it impossible to move I2C up in the link order without
moving all these too.

Also note that we are planing on depending on ACPI for the PC I2C bus
drivers. Which is a problem because video apparently wants to
initialize before ACPI, and now we want to initialize i2c before video.

> For the record, the OMAP tree puts I2C in the link order
> later than this.  It wasn't moved earlier mostly out of
> paranoia like yours.  (See below.  I'm not sure why "cbus"
> is listed twice, or what OMAP boards have similar issues
> with serio ...)

That's easier to get right if you restrict yourself to a single
platform. For the vanilla kernel, the order of the dependencies is way
more difficult to figure out and get right. There are some hints in
drivers/Makefile but most dependencies aren't spelled out.

My feeling is that we won't be able to solve this without first moving
the different type of i2c bus drivers (and possibly chip drivers) to
separate directories. For example, moving external I2C bus drivers
(i2c-parport-light, i2c-parport, i2c-taos-evm and i2c-tiny-usb) to a
separate directory that is always initialized late, would remove the
dependencies on parport, serio and USB for the "must initialize i2c
early" problem.

I've already attempted a categorization of the i2c bus drivers:
http://lists.lm-sensors.org/pipermail/i2c/2008-May/003713.html
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-group-bus-drivers.patch
I would welcome comments on this, and suggestions for further
categorization of group "other".

I guess we would need to go one step beyond and move the different
groups to different directories. I'm not too happy about moving files
as it makes history navigation a bit harder, but in this specific case
I see no other way.

-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[Russell King - ARM Linux]

On Wed, Jun 11, 2008 at 10:11:30AM +0200, Jean Delvare wrote:
> Hi David,
> 
> On Tue, 10 Jun 2008 13:55:07 -0700, David Brownell 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.
> > 
> > If they happen to sit outside the I2C tree and *before* it in
> > link order, things will misbehave.
> 
> Well, i2c system bus drivers shouldn't sit outside of the I2C tree, so
> that's not a problem. If you start accepting that drivers live at
> random places in the source tree, then there's simply no way to get
> things right.

That's simply not a realistic view.  As I've already pointed out,
framebuffer devices have I2C busses for reading the DDC information
from monitors.  These I2C bus drivers live in drivers/video.

Video grabbers have I2C busses for controlling, eg, tuners and video
decoders.  These live in drivers/media.

If I follow your argument, would you like cyber2000fb.c to be moved
entirely from drivers/video into drivers/i2c/busses because it contains
an i2c bus driver?  Clearly not.

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

Hi Russell,

On Wed, 11 Jun 2008 10:00:16 +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 11, 2008 at 10:11:30AM +0200, Jean Delvare wrote:
> > Hi David,
> > 
> > On Tue, 10 Jun 2008 13:55:07 -0700, David Brownell 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.
> > > 
> > > If they happen to sit outside the I2C tree and *before* it in
> > > link order, things will misbehave.
> > 
> > Well, i2c system bus drivers shouldn't sit outside of the I2C tree, so
> > that's not a problem. If you start accepting that drivers live at
> > random places in the source tree, then there's simply no way to get
> > things right.
> 
> That's simply not a realistic view.  As I've already pointed out,
> framebuffer devices have I2C busses for reading the DDC information
> from monitors.  These I2C bus drivers live in drivers/video.
> 
> Video grabbers have I2C busses for controlling, eg, tuners and video
> decoders.  These live in drivers/media.
> 
> If I follow your argument, would you like cyber2000fb.c to be moved
> entirely from drivers/video into drivers/i2c/busses because it contains
> an i2c bus driver?  Clearly not.

Please read what I wrote again. I said: I2C SYSTEM BUS drivers should
live under drivers/i2c. DDC channels on graphics adapters and I2C buses
on multimedia adapters definitely do not qualify as system buses (and
don't need to be initialized early, either.)

Thanks,
-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

On Thu, 12 Jun 2008 08:23:14 +1200, Ryan Mallon wrote:
> Jean Delvare wrote:
> >
> > That's easier to get right if you restrict yourself to a single
> > platform. For the vanilla kernel, the order of the dependencies is way
> > more difficult to figure out and get right. There are some hints in
> > drivers/Makefile but most dependencies aren't spelled out.
> >
> > My feeling is that we won't be able to solve this without first moving
> > the different type of i2c bus drivers (and possibly chip drivers) to
> > separate directories. For example, moving external I2C bus drivers
> > (i2c-parport-light, i2c-parport, i2c-taos-evm and i2c-tiny-usb) to a
> > separate directory that is always initialized late, would remove the
> > dependencies on parport, serio and USB for the "must initialize i2c
> > early" problem.
> >
> > I've already attempted a categorization of the i2c bus drivers:
> > http://lists.lm-sensors.org/pipermail/i2c/2008-May/003713.html
> > http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-group-bus-drivers.patch
> > I would welcome comments on this, and suggestions for further
> > categorization of group "other".
>
> I like this idea. Is it possible to move (or mark as subsys_initcall) the
> i2c busses which are likely to be needed early: pxa, omap, gpio, etc and
> leave the PC/external busses alone. Then having the i2c chip drivers in
> the correct place (ie drivers/gpio) would effectively fix the problem.

That's certainly possible to move them, the only problem is: how do we
select the drivers which need it? As you can see from the categories I
made, there's a fairly large list of "other" drivers are the moment.
All drivers you mentioned, and the ones already using
subsys_initcall(), are there. They fall under the "embedded" category
but I suspect that many drivers in the group "other" are not. So first
of all I think we need to refine my original grouping. If we can't make
a definitive list right away, maybe we can make an "embedded" group and
each developer or maintainer moves the drivers they know are embedded
there.

But that's only one side of the problem. The other side is that is is
entirely possible that some embedded platforms don't need the early i2c
initialization, and non-embedded platforms do. I have no example at the
moment but obviously it could happen. So, while moving the external bus
adapter drivers to a separate directory is fine (they just can't be
initialized early), for the rest it's mainly a per-driver attribute, so
maybe we just want an "early" directory with drivers that must be
initialized early, regardless of their platform type.

If we want a general solution to the problem, then I think that's the
way to go. If you only want to fix the problem at hand, just use
subsys_initcall() in your bus driver for now.

-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

On Thu, 12 Jun 2008 08:13:09 +1200, Ryan Mallon wrote:
> Jean Delvare wrote:
> > 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?
>
> On many embedded devices there is a need for i2c to be early since it is
> often used for core functionality. It seems at the moment, that the 
> answer to this is to juggle a few of the drivers which people need to
> get this to work. There are the drivers in drivers/i2c which use 
> subsys_initcall. It does work, but it feels a bit untidy. Some of the i2c
> IO expander drivers are now in drivers/gpio since that comes up early.
> This can lead to confusion (see drivers/gpio/pca953x.c and 
> drivers/i2c/chips/pca9539.c).

The GPIO drivers under drivers/i2c/chips are deprecated and tagged for
removal. We only need a user-space interface for the new GPIO drivers
(I think we do by now) and a way to instantiate these new GPIO devices
from user-space (not done yet) before the deprecated drivers can be
removed. The only remaining users for these deprecated drivers are
electronics enthusiasts controlling GPIO chips over the PC parallel
port. In the context of embedded platforms, there's no room for
confusion here.

>                               As David suggested, if i2c is needed early
> in enough cases, why not just move it early in the link order? My patch
> was just an alternative approach which mimics the current behaviour, but
> makes it possible to get any i2c driver early. Why not just mark all of
> the drivers/busses that get used on embedded devices as subsys_initcall,
> just in case somebody needs them early?

Because this is an abuse of subsys_initcall? I guess that was
acceptable when only a couple drivers were doing that, but making it
official sounds bad.

> (...)
> I just ran a sed script over the drivers/i2c directory. The intent was to
> mark the entire subsystem to come up early if CONFIG_I2C_EARLY is set,
> and use i2c_module_init every where since it makes it more consistent, and
> doesn't cost anything on setups where CONFIG_I2C_EARLY isn't defined.

It costs readability and build time.

> The idea was basically that a board could clearly say: I want i2c early,
> and have any busses and devices drivers it has configured as builtins
> automatically be present early on.

I get the idea, but I don't buy the implementation.

-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[David Brownell]

On Wednesday 11 June 2008, Jean Delvare wrote:
> Hi David,
> 
> On Tue, 10 Jun 2008 13:55:07 -0700, David Brownell 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.
> > 
> > If they happen to sit outside the I2C tree and *before* it in
> > link order, things will misbehave.
> 
> Well, i2c system bus drivers shouldn't sit outside of the I2C tree, so
> that's not a problem. If you start accepting that drivers live at
> random places in the source tree, then there's simply no way to get
> things right.

The "drivers in question" aren't necessarily all i2c_adapter drivers,
or even *only* i2c_adapter drivers (they could expose an i2c_adapter
interface as a secondary function).

I think I introduced the term "system bus driver", but you didn't read
it the way I meant it.  To elaborate a bit:  "system bus" as in "a bus
used for system bringup and management".  On PCs, the main such bus is
PCI but there are little fragments of other stuff too.  Many smaller
embedded systems use I2C for that (and don't have PCI).  The drivers
relevant are both (a) the bus driver itself, e.g. i2c_adapter or its
PCI analogue, and (b) drivers for devices on that bus.

So while (a) having i2c and some i2c_adapter drivers at subsys_initcall
is an essential first step, the *link order* issue comes up in two other
ways:  (b1) drivers "outside the I2C tree and *before* it in link order"
that may need to be subsys_initcall too, thus depending on core I2C
stuff being available, and (b2) some other subsystems may need to rely
on I2C in their bringup, such as by enabling a specific power domain
and the chips using it.


> As for i2c chip drivers, that's the exact reason why I am urging
> developers to create function-based subsystems rather than dumping
> their drivers in drivers/i2c/chips. If we have all drivers implementing
> a given function in a dedicated directory then we can sequence this
> directory properly in the link order.

But even given that, I2C is currently sequenced *late* for systems where
it serves as a system control/management bus; see (b1) and (b2) above.


> These were only two examples. We have i2c bus drivers depending on PCI,
> parport, USB, ISA, GPIO and serio. Given the current linking order,
> this makes it impossible to move I2C up in the link order without
> moving all these too.

Not really; those particular i2c_adapter drivers don't actually get used
early in system bringup.  They aren't subsys_initcall, and neither of
the (b1) or (b2) cases apply.  Moving those adapters doesn't imply moving
those other subsystems.

Example:  USB subsystem is initialized early (subsystem_init).  From then
on, drivers can register freely -- including i2c-tiny-usb.  The usbcore
code remembers them, and then when the HCDs (analogues: i2c_adapter) start
to connect, usb devices enumerate and get bound to devices as needed.  So
it doesn't matter *when* i2c-tiny-usb does its module_init(), so long as
it's after usbcore does its subsys_initcall.

Likewise for PCI.  As I said earlier, GPIO is already covered (has to
be usable before initcalls could be run, etc).  I think i2c-parport
should behave OK too, and i2c-parport-light.


> My feeling is that we won't be able to solve this without first moving
> the different type of i2c bus drivers (and possibly chip drivers) to
> separate directories. For example, moving external I2C bus drivers
> (i2c-parport-light, i2c-parport, i2c-taos-evm and i2c-tiny-usb) to a
> separate directory that is always initialized late, would remove the
> dependencies on parport, serio and USB for the "must initialize i2c
> early" problem.

I think you're looking at this wrong.  First, as I noted already,
most of those drivers don't have this issue.  Second, if there *are*
such issues, they'd be bus-specific issues to resolve, not reasons
to avoid fixing the (b1) and (b2) problems by moving I2C earlier.

- Dave

Re: [PATCH, RFC] Earlier I2C initialization

[Ryan Mallon]

Jean Delvare wrote:
> 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?
>   
Fair point. 

> 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?
On many embedded devices there is a need for i2c to be early since it is
often used for core functionality. It seems at the moment, that the 
answer to this is to juggle a few of the drivers which people need to
get this to work. There are the drivers in drivers/i2c which use 
subsys_initcall. It does work, but it feels a bit untidy. Some of the i2c
IO expander drivers are now in drivers/gpio since that comes up early.
This can lead to confusion (see drivers/gpio/pca953x.c and 
drivers/i2c/chips/pca9539.c). As David suggested, if i2c is needed early
in enough cases, why not just move it early in the link order? My patch
was just an alternative approach which mimics the current behaviour, but
makes it possible to get any i2c driver early. Why not just mark all of
the drivers/busses that get used on embedded devices as subsys_initcall,
just in case somebody needs them early?

<snip>

> 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.)
>   
<snip>

>
> 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.
>
>   
I just ran a sed script over the drivers/i2c directory. The intent was to
mark the entire subsystem to come up early if CONFIG_I2C_EARLY is set,
and use i2c_module_init every where since it makes it more consistent, and
doesn't cost anything on setups where CONFIG_I2C_EARLY isn't defined.

The idea was basically that a board could clearly say: I want i2c early,
and have any busses and devices drivers it has configured as builtins
automatically be present early on.

~Ryan

Re: [PATCH, RFC] Earlier I2C initialization

[Ryan Mallon]

Jean Delvare wrote:
>
> That's easier to get right if you restrict yourself to a single
> platform. For the vanilla kernel, the order of the dependencies is way
> more difficult to figure out and get right. There are some hints in
> drivers/Makefile but most dependencies aren't spelled out.
>
> My feeling is that we won't be able to solve this without first moving
> the different type of i2c bus drivers (and possibly chip drivers) to
> separate directories. For example, moving external I2C bus drivers
> (i2c-parport-light, i2c-parport, i2c-taos-evm and i2c-tiny-usb) to a
> separate directory that is always initialized late, would remove the
> dependencies on parport, serio and USB for the "must initialize i2c
> early" problem.
>
> I've already attempted a categorization of the i2c bus drivers:
> http://lists.lm-sensors.org/pipermail/i2c/2008-May/003713.html
> http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-group-bus-drivers.patch
> I would welcome comments on this, and suggestions for further
> categorization of group "other".
>   
I like this idea. Is it possible to move (or mark as subsys_initcall) the
i2c busses which are likely to be needed early: pxa, omap, gpio, etc and
leave the PC/external busses alone. Then having the i2c chip drivers in
the correct place (ie drivers/gpio) would effectively fix the problem.

~Ryan

Re: [PATCH, RFC] Earlier I2C initialization

[David Brownell]

On Wednesday 11 June 2008, Jean Delvare wrote:
> On Thu, 12 Jun 2008 08:13:09 +1200, Ryan Mallon wrote:
> >                               As David suggested, if i2c is needed early
> > in enough cases, why not just move it early in the link order? My patch
> > was just an alternative approach which mimics the current behaviour, but
> > makes it possible to get any i2c driver early. Why not just mark all of
> > the drivers/busses that get used on embedded devices as subsys_initcall,
> > just in case somebody needs them early?
> 
> Because this is an abuse of subsys_initcall? I guess that was
> acceptable when only a couple drivers were doing that, but making it
> official sounds bad.

How would it be an abuse?  On those systems, I2C is a "system bus"
and needs to be initialized early for the same reasons PCI gets set
up very early on PC hardware.

There's no rule saying that subsystem initialization may not include
the essential drivers -- in this case, i2c_adapter drivers.  PCI hubs
and bridges are certainly initialized very early, before module_init
code runs...

And in fact it seems a bit odd to think that initializing any bus
subsystem shouldn't be allowed to include its bus adapters.  It's
not as if the subsystem has completed initializiation until those
adapters are usable!!

- Dave

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

On Wed, 11 Jun 2008 13:27:09 -0700, David Brownell wrote:
> On Wednesday 11 June 2008, Jean Delvare wrote:
> > On Thu, 12 Jun 2008 08:13:09 +1200, Ryan Mallon wrote:
> > >                               As David suggested, if i2c is needed early
> > > in enough cases, why not just move it early in the link order? My patch
> > > was just an alternative approach which mimics the current behaviour, but
> > > makes it possible to get any i2c driver early. Why not just mark all of
> > > the drivers/busses that get used on embedded devices as subsys_initcall,
> > > just in case somebody needs them early?
> > 
> > Because this is an abuse of subsys_initcall? I guess that was
> > acceptable when only a couple drivers were doing that, but making it
> > official sounds bad.
> 
> How would it be an abuse?  On those systems, I2C is a "system bus"
> and needs to be initialized early for the same reasons PCI gets set
> up very early on PC hardware.

But the pci subsystem doesn't make use of subsys_initcall(). Instead,
it is simply placed early in the link order.

That being said, I'm not sure if the comparison with the PCI subsystem
holds... I am under the impression that PCI bus handling doesn't
require dedicated drivers? At least I can't see any under drivers/pci.

> There's no rule saying that subsystem initialization may not include
> the essential drivers -- in this case, i2c_adapter drivers.  PCI hubs
> and bridges are certainly initialized very early, before module_init
> code runs...

Care to point me to actual code to backup this "certainly"?

> And in fact it seems a bit odd to think that initializing any bus
> subsystem shouldn't be allowed to include its bus adapters.  It's
> not as if the subsystem has completed initializiation until those
> adapters are usable!!

I think it makes a lot of sense to initialize the core of a subsystem
early, so that all devices and drivers can be registered. This doesn't
imply registering the hardware bus drivers too, even though in some
cases it is also needed. I doubt that whoever designed subsys_initcall
meant it to be used for all bus drivers, otherwise he/she would have
named it, say, busdrv_initcall.

But don't get me wrong: if subsys_initcall is the way to go, that's
alright with me, that's way less work than having to move drivers to
different directories and fixing the link order.

-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[Ryan Mallon]

Jean Delvare wrote:
> On Wed, 11 Jun 2008 13:27:09 -0700, David Brownell wrote:
>> On Wednesday 11 June 2008, Jean Delvare wrote:
>>> On Thu, 12 Jun 2008 08:13:09 +1200, Ryan Mallon wrote:
>>>>                               As David suggested, if i2c is needed early
>>>> in enough cases, why not just move it early in the link order? My patch
>>>> was just an alternative approach which mimics the current behaviour, but
>>>> makes it possible to get any i2c driver early. Why not just mark all of
>>>> the drivers/busses that get used on embedded devices as subsys_initcall,
>>>> just in case somebody needs them early?
>>> Because this is an abuse of subsys_initcall? I guess that was
>>> acceptable when only a couple drivers were doing that, but making it
>>> official sounds bad.
>> How would it be an abuse?  On those systems, I2C is a "system bus"
>> and needs to be initialized early for the same reasons PCI gets set
>> up very early on PC hardware.
> 
> But the pci subsystem doesn't make use of subsys_initcall(). Instead,
> it is simply placed early in the link order.
> 
> That being said, I'm not sure if the comparison with the PCI subsystem
> holds... I am under the impression that PCI bus handling doesn't
> require dedicated drivers? At least I can't see any under drivers/pci.
> 
>> There's no rule saying that subsystem initialization may not include
>> the essential drivers -- in this case, i2c_adapter drivers.  PCI hubs
>> and bridges are certainly initialized very early, before module_init
>> code runs...
> 
> Care to point me to actual code to backup this "certainly"?

ryan@okiwi:pci$ grep initcall * -R
pci-acpi.c:arch_initcall(acpi_pci_init);
pci.c:device_initcall(pci_init);
pci-driver.c:postcore_initcall(pci_driver_init);
pcie/aspm.c:fs_initcall(pcie_aspm_init);
pci-sysfs.c:late_initcall(pci_sysfs_init);
probe.c:postcore_initcall(pcibus_class_init);
proc.c:__initcall(pci_proc_init);

At a quick glance (I don't know much about the PCI subsystem) at least
the bus and class are registered early on.

>> And in fact it seems a bit odd to think that initializing any bus
>> subsystem shouldn't be allowed to include its bus adapters.  It's
>> not as if the subsystem has completed initializiation until those
>> adapters are usable!!
> 
> I think it makes a lot of sense to initialize the core of a subsystem
> early, so that all devices and drivers can be registered. This doesn't
> imply registering the hardware bus drivers too, even though in some
> cases it is also needed. I doubt that whoever designed subsys_initcall
> meant it to be used for all bus drivers, otherwise he/she would have
> named it, say, busdrv_initcall.

At least in the case of i2c on many embedded devices we want the bus
driver early, but many other i2c bus drivers don't need this. There
seem to be two main options:

1) Use subsys_initcall on the ones that may need to be early (current
approach).
2) Split the bus drivers into two directories, say drivers/i2c-early and
drivers/i2c, and put the former earlier in the link order.

Maybe a decent compromise approach is to move the drivers which are used
on embedded systems to a new directory such as drivers/i2c/embedded or
drivers/i2c/soc and then only use subsys_initcall on bus drivers in that
directory?

If all of the i2c drivers then go in the right place, ie drivers/gpio
instead of drivers/i2c/chips, then only the bus drivers, not the device
drivers, should ever need to use subsys_initcall right?

> But don't get me wrong: if subsys_initcall is the way to go, that's
> alright with me, that's way less work than having to move drivers to
> different directories and fixing the link order.

I don't think multiple directories for i2c at the drivers/ level is the
right way to go, that just going to get confusing. I think moving the
i2c bus drivers which may be needed early to a new sub directory under
drivers/i2c and then tidying up the Kconfig to categorise the bus
drivers as embedded (soc), pc, external, etc.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

       Ryan Mallon                              Unit 5, Amuri Park
       Phone: +64 3 3779127                     404 Barbadoes St
       Fax:   +64 3 3779135                     PO Box 13 889
       Email: ryan@bluewatersys.com             Christchurch, 8013
       Web:   http://www.bluewatersys.com       New Zealand
       Freecall Australia  1800 148 751         USA 1800 261 2934

Re: [PATCH, RFC] Earlier I2C initialization

[Maciej W. Rozycki]

On Wed, 11 Jun 2008, Jean Delvare wrote:

> That being said, I'm not sure if the comparison with the PCI subsystem
> holds... I am under the impression that PCI bus handling doesn't
> require dedicated drivers? At least I can't see any under drivers/pci.

 Of course it does require them.  It is just due to their very nature they
tend to be placed under arch/, although there are some cases where the
same system controller can be used for a range of processors (e.g. some
Marvell chips can be used either with MIPS or PowerPC CPUs) and they might
be arguably put in a place more suitable for sharing between
architectures.  See arch/mips/pci/ for an example of a generous bunch of
PCI host drivers.

  Maciej

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

Hi David,

On Wed, 11 Jun 2008 11:31:50 -0700, David Brownell wrote:
> On Wednesday 11 June 2008, Jean Delvare wrote:
> > Hi David,
> > 
> > On Tue, 10 Jun 2008 13:55:07 -0700, David Brownell 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.
> > > 
> > > If they happen to sit outside the I2C tree and *before* it in
> > > link order, things will misbehave.
> > 
> > Well, i2c system bus drivers shouldn't sit outside of the I2C tree, so
> > that's not a problem. If you start accepting that drivers live at
> > random places in the source tree, then there's simply no way to get
> > things right.
> 
> The "drivers in question" aren't necessarily all i2c_adapter drivers,
> or even *only* i2c_adapter drivers (they could expose an i2c_adapter
> interface as a secondary function).

Drivers exposing an i2c interface as a secondary function are, in my
experience, not drivers for system i2c bus. So, we just don't care
about them here.

Non-i2c_adapter drivers, are up to their respective subsystem
maintainers. i2c-core is initialized early thanks to subsys_initcall,
so if the needed i2c_adapter is also initialized early (be it thanks to
subsys_initcall or to the link order), my job, as the i2c subsystem
maintainer, is done.

> I think I introduced the term "system bus driver", but you didn't read
> it the way I meant it.  To elaborate a bit:  "system bus" as in "a bus
> used for system bringup and management".  On PCs, the main such bus is
> PCI but there are little fragments of other stuff too.  Many smaller
> embedded systems use I2C for that (and don't have PCI).  The drivers
> relevant are both (a) the bus driver itself, e.g. i2c_adapter or its
> PCI analogue, and (b) drivers for devices on that bus.
> 
> So while (a) having i2c and some i2c_adapter drivers at subsys_initcall
> is an essential first step, the *link order* issue comes up in two other
> ways:  (b1) drivers "outside the I2C tree and *before* it in link order"
> that may need to be subsys_initcall too, thus depending on core I2C
> stuff being available, and (b2) some other subsystems may need to rely
> on I2C in their bringup, such as by enabling a specific power domain
> and the chips using it.

(b1) and (b2) are the same problem as far as I can see, and is down to:
is the needed i2c_adapter driver initialized early enough. Which can be
solved by either using subsys_initcall in said driver, or by moving
said driver earlier in the link order. Do we agree on this? I think we
do.

> > (...)
> > These were only two examples. We have i2c bus drivers depending on PCI,
> > parport, USB, ISA, GPIO and serio. Given the current linking order,
> > this makes it impossible to move I2C up in the link order without
> > moving all these too.
> 
> Not really; those particular i2c_adapter drivers don't actually get used
> early in system bringup.  They aren't subsys_initcall, and neither of
> the (b1) or (b2) cases apply.  Moving those adapters doesn't imply moving
> those other subsystems.

If said subsystems are designed properly, I agree. But it they are not,
I suspect it's easier to not move the "external" i2c bus drivers, than
to fix these subsystems. But maybe I'm wrong on that.

> Example:  USB subsystem is initialized early (subsystem_init).  From then
> on, drivers can register freely -- including i2c-tiny-usb.  The usbcore
> code remembers them, and then when the HCDs (analogues: i2c_adapter) start
> to connect, usb devices enumerate and get bound to devices as needed.  So
> it doesn't matter *when* i2c-tiny-usb does its module_init(), so long as
> it's after usbcore does its subsys_initcall.

You're right, I had forgotten about subsys_initcall here, I was only
watching the link order. My bad.

> Likewise for PCI.  As I said earlier, GPIO is already covered (has to
> be usable before initcalls could be run, etc).  I think i2c-parport
> should behave OK too,

I have a doubt on parport, given that
  grep -r _initcall drivers/parport
doesn't return anything.

> and i2c-parport-light.

i2c-parport-light doesn't depend on anything (thus the name), so it
can't have any problem. I shouldn't have mentioned it, sorry.

> > My feeling is that we won't be able to solve this without first moving
> > the different type of i2c bus drivers (and possibly chip drivers) to
> > separate directories. For example, moving external I2C bus drivers
> > (i2c-parport-light, i2c-parport, i2c-taos-evm and i2c-tiny-usb) to a
> > separate directory that is always initialized late, would remove the
> > dependencies on parport, serio and USB for the "must initialize i2c
> > early" problem.
> 
> I think you're looking at this wrong.  First, as I noted already,
> most of those drivers don't have this issue.  Second, if there *are*
> such issues, they'd be bus-specific issues to resolve, not reasons
> to avoid fixing the (b1) and (b2) problems by moving I2C earlier.

Not a reason to not change the link order, you're right. But definitely
a good reason to watch for breakage as we do, so that we can fix them.

Now I am a bit confused as to your position about how to solve the
problem. In an earlier post, you were claiming that using
subsys_initcall in i2c bus drivers wasn't an abuse. And now you suggest
that moving i2c earlier in the build order is the way to go. As I
understand it we have no reason to do both. Or do we?

Thanks,
-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[David Brownell]

On Thursday 12 June 2008, Jean Delvare wrote:

> > So while (a) having i2c and some i2c_adapter drivers at subsys_initcall
> > is an essential first step, the *link order* issue comes up in two other
> > ways:  (b1) drivers "outside the I2C tree and *before* it in link order"
> > that may need to be subsys_initcall too, thus depending on core I2C
> > stuff being available, and (b2) some other subsystems may need to rely
> > on I2C in their bringup, such as by enabling a specific power domain
> > and the chips using it.
> 
> (b1) and (b2) are the same problem as far as I can see, and is down to:
> is the needed i2c_adapter driver initialized early enough.

... and *BEFORE* the "i2c_client" driver.  Moving I2C earlier in the
link order may be necessary to ensure that.

Yes, (b2) is essentially the "30,000 foot view" of the problem for
which (b1) is the lower altitude "5,000 foot view".  The PCI version
of that problem has the relevant init code running at arch_initcall
level (in at least those MIPS cases) and also getting linked before
all the code in the "drivers" subtree.


> Which can be 
> solved by either using subsys_initcall in said driver, or by moving
> said driver earlier in the link order. Do we agree on this?

No; that would be a case of moving other subsystems *AFTER* I2C in
the link order, not moving any i2c_client driver earlier.

Remember, you were wanting (appropriately!) to move drivers out of
the i2c subtree when they have a better home.  And in this case,
(b2) explains why that surfaces this requirement for earlier init
of I2C ... if those drivers were in drivers/i2c/chips, the link order
would not be an issue.


> > > (...)
> > > These were only two examples. We have i2c bus drivers depending on PCI,
> > > parport, USB, ISA, GPIO and serio. Given the current linking order,
> > > this makes it impossible to move I2C up in the link order without
> > > moving all these too.
> > 
> > Not really; those particular i2c_adapter drivers don't actually get used
> > early in system bringup.  They aren't subsys_initcall, and neither of
> > the (b1) or (b2) cases apply.  Moving those adapters doesn't imply moving
> > those other subsystems.
> 
> If said subsystems are designed properly, I agree. But it they are not,
> I suspect it's easier to not move the "external" i2c bus drivers, than
> to fix these subsystems. But maybe I'm wrong on that.

Missing my point:  since those i2c_adapter drivers aren't getting
used early (subsys_initcall etc), this issue *can't* come up with them.
Whether or not those subsystems "play well with others".  This issue
is *only* for drivers outside the i2c subtree which are essential for
system bringup, at the subsys_initcall level.


> > Likewise for PCI.  As I said earlier, GPIO is already covered (has to
> > be usable before initcalls could be run, etc).  I think i2c-parport
> > should behave OK too,
> 
> I have a doubt on parport, given that
>   grep -r _initcall drivers/parport
> doesn't return anything.

I see a bunch of module_init(), which means device_initcall(),
which means well after subsys_initcall().  So i2c-parport should
behave.


> > and i2c-parport-light.
> 
> i2c-parport-light doesn't depend on anything (thus the name), so it
> can't have any problem. I shouldn't have mentioned it, sorry.

Well, it's a legacy driver so automatically has that class of problems
(making its own device node etc) ... but those issues are unrelated to
the $SUBJECT issue.


> > > My feeling is that we won't be able to solve this without first moving
> > > the different type of i2c bus drivers (and possibly chip drivers) to
> > > separate directories. For example, moving external I2C bus drivers
> > > (i2c-parport-light, i2c-parport, i2c-taos-evm and i2c-tiny-usb) to a
> > > separate directory that is always initialized late, would remove the
> > > dependencies on parport, serio and USB for the "must initialize i2c
> > > early" problem.
> > 
> > I think you're looking at this wrong.  First, as I noted already,
> > most of those drivers don't have this issue.  Second, if there *are*
> > such issues, they'd be bus-specific issues to resolve, not reasons
> > to avoid fixing the (b1) and (b2) problems by moving I2C earlier.
> 
> Not a reason to not change the link order, you're right. But definitely
> a good reason to watch for breakage as we do, so that we can fix them.

Always.  :)


> Now I am a bit confused as to your position about how to solve the
> problem. In an earlier post, you were claiming that using
> subsys_initcall in i2c bus drivers wasn't an abuse. And now you suggest
> that moving i2c earlier in the build order is the way to go. As I
> understand it we have no reason to do both. Or do we?

We have reason to do both.  That's why I called out cases (a) vs (b)
(and b1/b2) above.  They address different problems.

This is confusing stuff, in part because this part of initialization
is all driven by side effects and is thus normally hidden.

The arch_initcall/subsys_initcall/device_initcall/... stuff (a) is at
least partially explicit.  But the sequencing of initcalls within a
given grouping (b) is purely a link order side effect.

So every time changing the link order comes up, confusion arises.

- Dave

Re: [PATCH, RFC] Earlier I2C initialization

[David Brownell]

On Wednesday 11 June 2008, Maciej W. Rozycki wrote:
> > That being said, I'm not sure if the comparison with the PCI subsystem
> > holds... I am under the impression that PCI bus handling doesn't
> > require dedicated drivers? At least I can't see any under drivers/pci.
> 
>  Of course it does require them.  It is just due to their very nature they
> tend to be placed under arch/,

PCI root hubs, yes.  The drivers/pci/hotplug bridges are slightly
more generic, ditto drivers/pci/pcie and the CardBus bridges in
drivers/pcmcia.  At one point, lack of a generic (non-hotplug) PCI
bridge driver was viewed as a weakness of that driver stack.

Also, drivers/acpi/pci_root.c binds the root at subsys_initcall.
That's done *after* some earlier PCI magic; I never bothered
to sort through that little maze.


> although there are some cases where the 
> same system controller can be used for a range of processors (e.g. some
> Marvell chips can be used either with MIPS or PowerPC CPUs) and they might
> be arguably put in a place more suitable for sharing between
> architectures.  See arch/mips/pci/ for an example of a generous bunch of
> PCI host drivers.

Which, for the record, get very early initialization using two
different mechanisms:

  - many use arch_initcall()

  - the "arch" subtree is linked before the "drivers" subtree

I don't think I2C needs to worry about arch_initcall just now,
but if necessary it could initialize earlier than subsys_initcall.

- Dave

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

Hi Ryan,

Sorry for the late answer.

On Thu, 12 Jun 2008 09:24:54 +1200, Ryan Mallon wrote:
> Jean Delvare wrote:
> > On Wed, 11 Jun 2008 13:27:09 -0700, David Brownell wrote:
> >> There's no rule saying that subsystem initialization may not include
> >> the essential drivers -- in this case, i2c_adapter drivers.  PCI hubs
> >> and bridges are certainly initialized very early, before module_init
> >> code runs...
> > 
> > Care to point me to actual code to backup this "certainly"?
> 
> ryan@okiwi:pci$ grep initcall * -R
> pci-acpi.c:arch_initcall(acpi_pci_init);
> pci.c:device_initcall(pci_init);
> pci-driver.c:postcore_initcall(pci_driver_init);
> pcie/aspm.c:fs_initcall(pcie_aspm_init);
> pci-sysfs.c:late_initcall(pci_sysfs_init);
> probe.c:postcore_initcall(pcibus_class_init);
> proc.c:__initcall(pci_proc_init);
> 
> At a quick glance (I don't know much about the PCI subsystem) at least
> the bus and class are registered early on.

Ah, my bad. I had been grepping for subsys_initcall, instead of just
_initcall. It's clearer now.

> > I think it makes a lot of sense to initialize the core of a subsystem
> > early, so that all devices and drivers can be registered. This doesn't
> > imply registering the hardware bus drivers too, even though in some
> > cases it is also needed. I doubt that whoever designed subsys_initcall
> > meant it to be used for all bus drivers, otherwise he/she would have
> > named it, say, busdrv_initcall.
> 
> At least in the case of i2c on many embedded devices we want the bus
> driver early, but many other i2c bus drivers don't need this. There
> seem to be two main options:
> 
> 1) Use subsys_initcall on the ones that may need to be early (current
> approach).
> 2) Split the bus drivers into two directories, say drivers/i2c-early and
> drivers/i2c, and put the former earlier in the link order.

I don't like option 2 at all. drivers/i2c/early could be, but not
drivers/i2c-early.

> Maybe a decent compromise approach is to move the drivers which are used
> on embedded systems to a new directory such as drivers/i2c/embedded or
> drivers/i2c/soc and then only use subsys_initcall on bus drivers in that
> directory?

If we are going to keep using subsys_initcall in bus drivers, then
moving the bus drivers using it to a separate directory is not needed.
Which doesn't mean we don't want to split the i2c bus drivers into
subdirectories for other reasons, but I want to see this as a separate
issue. BTW, I started categorizing the different types of i2c bus
drivers:
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-group-bus-drivers.patch
If you want to help with embedded stuff or SOC or whatever makes sense
to group together, please do! For now, everything I am not familiar
with is in group "Others".

> If all of the i2c drivers then go in the right place, ie drivers/gpio
> instead of drivers/i2c/chips, then only the bus drivers, not the device
> drivers, should ever need to use subsys_initcall right?

If drivers/gpio and everything else which may be needed early are moved
early in the linking order, yes. Otherwise they will need to use
subsys_initcall too, but I seem to understand that it might not be
sufficient, if other drivers depend on them and they are also using
subsys_initcall.

> > But don't get me wrong: if subsys_initcall is the way to go, that's
> > alright with me, that's way less work than having to move drivers to
> > different directories and fixing the link order.
> 
> I don't think multiple directories for i2c at the drivers/ level is the
> right way to go, that just going to get confusing. I think moving the
> i2c bus drivers which may be needed early to a new sub directory under
> drivers/i2c and then tidying up the Kconfig to categorise the bus
> drivers as embedded (soc), pc, external, etc.

I agree.

-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

Hi David,

Sorry for the late answer.

On Thu, 12 Jun 2008 12:57:47 -0700, David Brownell wrote:
> On Thursday 12 June 2008, Jean Delvare wrote:
> 
> > > So while (a) having i2c and some i2c_adapter drivers at subsys_initcall
> > > is an essential first step, the *link order* issue comes up in two other
> > > ways:  (b1) drivers "outside the I2C tree and *before* it in link order"
> > > that may need to be subsys_initcall too, thus depending on core I2C
> > > stuff being available, and (b2) some other subsystems may need to rely
> > > on I2C in their bringup, such as by enabling a specific power domain
> > > and the chips using it.
> > 
> > (b1) and (b2) are the same problem as far as I can see, and is down to:
> > is the needed i2c_adapter driver initialized early enough.
> 
> ... and *BEFORE* the "i2c_client" driver.  Moving I2C earlier in the
> link order may be necessary to ensure that.

Ah, I'm getting it now (it took time, sorry.) You want the i2c core and
bus driver to be available not to let the i2c client driver register (it
doesn't need the i2c bus driver for that) but to actually bind to and
initialize whatever device it is driving, because other subsystem need
this initialization to happen before themselves can work. That's the
part I had been missing from the beginning.

> Yes, (b2) is essentially the "30,000 foot view" of the problem for
> which (b1) is the lower altitude "5,000 foot view".  The PCI version
> of that problem has the relevant init code running at arch_initcall
> level (in at least those MIPS cases) and also getting linked before
> all the code in the "drivers" subtree.
> 
> 
> > Which can be 
> > solved by either using subsys_initcall in said driver, or by moving
> > said driver earlier in the link order. Do we agree on this?
> 
> No; that would be a case of moving other subsystems *AFTER* I2C in
> the link order, not moving any i2c_client driver earlier.

I think I had bus drivers in mind when I wrote that, not i2c client
drivers.

> Remember, you were wanting (appropriately!) to move drivers out of
> the i2c subtree when they have a better home.  And in this case,
> (b2) explains why that surfaces this requirement for earlier init
> of I2C ... if those drivers were in drivers/i2c/chips, the link order
> would not be an issue.

I think there would still be an issue as long as i2c is late in the
linking order. If i2c is early in the linking order and chip drivers
are in drivers/i2c/chips, I agree there is no problem. But as I already
said I don't want this to happen, it doesn't really matter.

> > (...)
> > Now I am a bit confused as to your position about how to solve the
> > problem. In an earlier post, you were claiming that using
> > subsys_initcall in i2c bus drivers wasn't an abuse. And now you suggest
> > that moving i2c earlier in the build order is the way to go. As I
> > understand it we have no reason to do both. Or do we?
> 
> We have reason to do both.  That's why I called out cases (a) vs (b)
> (and b1/b2) above.  They address different problems.
> 
> This is confusing stuff, in part because this part of initialization
> is all driven by side effects and is thus normally hidden.
> 
> The arch_initcall/subsys_initcall/device_initcall/... stuff (a) is at
> least partially explicit.  But the sequencing of initcalls within a
> given grouping (b) is purely a link order side effect.
> 
> So every time changing the link order comes up, confusion arises.

I think I have a clearer picture of the whole problem and dependencies
now, thanks. Meaning that anyone who has an actual problem with the
current link order is free to work up a patch and send it to me and
I'll review it. I won't be driving the effort myself, as I don't have
any system requiring these changes, I don't think I would be the right
person for the job.

-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[Ryan Mallon]

Jean Delvare wrote:
> Hi Ryan,
> 
> Sorry for the late answer.
> 
> On Thu, 12 Jun 2008 09:24:54 +1200, Ryan Mallon wrote:
>> Jean Delvare wrote:
>>> On Wed, 11 Jun 2008 13:27:09 -0700, David Brownell wrote:
>>>> There's no rule saying that subsystem initialization may not include
>>>> the essential drivers -- in this case, i2c_adapter drivers.  PCI hubs
>>>> and bridges are certainly initialized very early, before module_init
>>>> code runs...
>>> Care to point me to actual code to backup this "certainly"?
>> ryan@okiwi:pci$ grep initcall * -R
>> pci-acpi.c:arch_initcall(acpi_pci_init);
>> pci.c:device_initcall(pci_init);
>> pci-driver.c:postcore_initcall(pci_driver_init);
>> pcie/aspm.c:fs_initcall(pcie_aspm_init);
>> pci-sysfs.c:late_initcall(pci_sysfs_init);
>> probe.c:postcore_initcall(pcibus_class_init);
>> proc.c:__initcall(pci_proc_init);
>>
>> At a quick glance (I don't know much about the PCI subsystem) at least
>> the bus and class are registered early on.
> 
> Ah, my bad. I had been grepping for subsys_initcall, instead of just
> _initcall. It's clearer now.
> 
>>> I think it makes a lot of sense to initialize the core of a subsystem
>>> early, so that all devices and drivers can be registered. This doesn't
>>> imply registering the hardware bus drivers too, even though in some
>>> cases it is also needed. I doubt that whoever designed subsys_initcall
>>> meant it to be used for all bus drivers, otherwise he/she would have
>>> named it, say, busdrv_initcall.
>> At least in the case of i2c on many embedded devices we want the bus
>> driver early, but many other i2c bus drivers don't need this. There
>> seem to be two main options:
>>
>> 1) Use subsys_initcall on the ones that may need to be early (current
>> approach).
>> 2) Split the bus drivers into two directories, say drivers/i2c-early and
>> drivers/i2c, and put the former earlier in the link order.
> 
> I don't like option 2 at all. drivers/i2c/early could be, but not
> drivers/i2c-early.

I don't think its a good idea either. The point was that in order to
have two sets of i2c drivers with different link order (with regard to
the rest of the driver subsystems) you would need two top level
directories in drivers/. The other way of course, is to use one
directory, or subdirectories under drivers/i2c/ and use subsys_initcall.

> 
>> Maybe a decent compromise approach is to move the drivers which are used
>> on embedded systems to a new directory such as drivers/i2c/embedded or
>> drivers/i2c/soc and then only use subsys_initcall on bus drivers in that
>> directory?
> 
> If we are going to keep using subsys_initcall in bus drivers, then
> moving the bus drivers using it to a separate directory is not needed.
> Which doesn't mean we don't want to split the i2c bus drivers into
> subdirectories for other reasons, but I want to see this as a separate
> issue. BTW, I started categorizing the different types of i2c bus
> drivers:
> http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-group-bus-drivers.patch
> If you want to help with embedded stuff or SOC or whatever makes sense
> to group together, please do! For now, everything I am not familiar
> with is in group "Others".

I don't know all of the embedded systems, but at a glance the following
busses from your others section are used on embedded ARM processors:

i2c-at91
i2c-davinci
i2c-ixp2000
i2c-omap
i2c-pxa
i2c-s3c2410
i2c-versatile

I'm sure someone else can point out any I missed, and any embedded i2c
controllers from other architectures. i2c-gpio should probably also go
under an embedded section as I assume that it is mostly used on embedded
systems where there is no SoC i2c controller.

>> If all of the i2c drivers then go in the right place, ie drivers/gpio
>> instead of drivers/i2c/chips, then only the bus drivers, not the device
>> drivers, should ever need to use subsys_initcall right?
> 
> If drivers/gpio and everything else which may be needed early are moved
> early in the linking order, yes. Otherwise they will need to use
> subsys_initcall too, but I seem to understand that it might not be
> sufficient, if other drivers depend on them and they are also using
> subsys_initcall.

I think this is a problem in general for the Linux kernel driver model.
There is no real way to say that two otherwise unrelated drivers are
dependent on each other, and that one must precede the other in the link
order. I think this happens more in the embedded space where you get
things like an i2c io expander being used as gpios to control power to
other peripherals, etc.

~Ryan

Re: [PATCH, RFC] Earlier I2C initialization

[Jean Delvare]

Hi Ryan,

On Fri, 27 Jun 2008 09:12:54 +1200, Ryan Mallon wrote:
> Jean Delvare wrote:
> > I don't like option 2 at all. drivers/i2c/early could be, but not
> > drivers/i2c-early.
> 
> I don't think its a good idea either. The point was that in order to
> have two sets of i2c drivers with different link order (with regard to
> the rest of the driver subsystems) you would need two top level
> directories in drivers/.

I don't think so. Look at what the input subsystem does (from
drivers/Makefile):

obj-$(CONFIG_SERIO)		+= input/serio/
obj-$(CONFIG_GAMEPORT)		+= input/gameport/
obj-$(CONFIG_INPUT)		+= input/

And there are similar examples with video (video/i810/ and
video/intelfb/ are early in the list). We could do the same with i2c:

obj-$(CONFIG_I2C)		+= i2c/early/

(... lots of things ...)

obj-$(CONFIG_I2C)		+= i2c/

Of course this implies minor edits to drivers/i2c/Makefile. I don't
know if we want to do that, but from a technical point of view it seems
doable.

> The other way of course, is to use one
> directory, or subdirectories under drivers/i2c/ and use subsys_initcall.

> > (...)
> > If we are going to keep using subsys_initcall in bus drivers, then
> > moving the bus drivers using it to a separate directory is not needed.
> > Which doesn't mean we don't want to split the i2c bus drivers into
> > subdirectories for other reasons, but I want to see this as a separate
> > issue. BTW, I started categorizing the different types of i2c bus
> > drivers:
> > http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-group-bus-drivers.patch
> > If you want to help with embedded stuff or SOC or whatever makes sense
> > to group together, please do! For now, everything I am not familiar
> > with is in group "Others".
> 
> I don't know all of the embedded systems, but at a glance the following
> busses from your others section are used on embedded ARM processors:
> 
> i2c-at91
> i2c-davinci
> i2c-ixp2000
> i2c-omap
> i2c-pxa
> i2c-s3c2410
> i2c-versatile

OK, I've moved these to a section titled "Embebbed system I2C host
controller drivers", thanks.

> I'm sure someone else can point out any I missed, and any embedded i2c
> controllers from other architectures. i2c-gpio should probably also go
> under an embedded section as I assume that it is mostly used on embedded
> systems where there is no SoC i2c controller.

OK, I did this too. In theory, i2c-gpio could be used on any system,
but in practice you are most certainly right.

-- 
Jean Delvare

Re: [PATCH, RFC] Earlier I2C initialization

[Ryan Mallon]

Jean Delvare wrote:
> Hi Ryan,
> 
> On Fri, 27 Jun 2008 09:12:54 +1200, Ryan Mallon wrote:
>> Jean Delvare wrote:
>>> I don't like option 2 at all. drivers/i2c/early could be, but not
>>> drivers/i2c-early.
>> I don't think its a good idea either. The point was that in order to
>> have two sets of i2c drivers with different link order (with regard to
>> the rest of the driver subsystems) you would need two top level
>> directories in drivers/.
> 
> I don't think so. Look at what the input subsystem does (from
> drivers/Makefile):
> 
> obj-$(CONFIG_SERIO)		+= input/serio/
> obj-$(CONFIG_GAMEPORT)		+= input/gameport/
> obj-$(CONFIG_INPUT)		+= input/
> 
> And there are similar examples with video (video/i810/ and
> video/intelfb/ are early in the list). 

Ah, didn't realise that.

>We could do the same with i2c:
> 
> obj-$(CONFIG_I2C)		+= i2c/early/
> 
> (... lots of things ...)
> 
> obj-$(CONFIG_I2C)		+= i2c/
> 
> Of course this implies minor edits to drivers/i2c/Makefile. I don't
> know if we want to do that, but from a technical point of view it seems
> doable.

That certainly is a better solution if it is needed.

> 
>> The other way of course, is to use one
>> directory, or subdirectories under drivers/i2c/ and use subsys_initcall.

Yeah, its probably the best solution for now. Doing the categorisation
of the busses will make it easy to split the directories/link order up
later if required.

>>> (...)
>>> If we are going to keep using subsys_initcall in bus drivers, then
>>> moving the bus drivers using it to a separate directory is not needed.
>>> Which doesn't mean we don't want to split the i2c bus drivers into
>>> subdirectories for other reasons, but I want to see this as a separate
>>> issue. BTW, I started categorizing the different types of i2c bus
>>> drivers:
>>> http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-group-bus-drivers.patch
>>> If you want to help with embedded stuff or SOC or whatever makes sense
>>> to group together, please do! For now, everything I am not familiar
>>> with is in group "Others".
>> I don't know all of the embedded systems, but at a glance the following
>> busses from your others section are used on embedded ARM processors:
>>
>> i2c-at91
>> i2c-davinci
>> i2c-ixp2000
>> i2c-omap
>> i2c-pxa
>> i2c-s3c2410
>> i2c-versatile
> 
> OK, I've moved these to a section titled "Embebbed system I2C host
> controller drivers", thanks.

Cool.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

       Ryan Mallon                              Unit 5, Amuri Park
       Phone: +64 3 3779127                     404 Barbadoes St
       Fax:   +64 3 3779135                     PO Box 13 889
       Email: ryan@bluewatersys.com             Christchurch, 8013
       Web:   http://www.bluewatersys.com       New Zealand
       Freecall Australia  1800 148 751         USA 1800 261 2934