Hi Jean, On Tue, 12 Oct 2010 19:34:25 +0800 Jean Delvare <khali@xxxxxxxxxxxx> wrote: > Hi Feng, > > On Tue, 12 Oct 2010 17:30:28 +0800, Feng Tang wrote: > > On Tue, 12 Oct 2010 16:47:07 +0800 > > Jean Delvare <khali@xxxxxxxxxxxx> wrote: > > > Of course i2c_detect gets called at times, otherwise we would > > > delete it altogether ;) > > > > > > Please point me to the I2C adapter driver(s) you care about, and > > > also the platform init code for your system. I would like to see > > > how it looks like. > > > > > > I think it would also help if I had a global picture of your > > > project and what you are trying to achieve. I presume we are > > > talking about an embedded system? With a custom kernel maybe? > > > Which platform/architecture? > > > > Yes, we are working a x86 based embedded system, Intel's Moorestown > > and Medfield systems, its current platform init code is > > arch/x86/kernel/mrst.c, but the I2c init code hasn't been pushed > > upstream yet :(, but basically it get i2c devices info from a SFI > > table (drivers/sfi/) and use i2c_register_board_info for them. > > > > The adapter driver is i2c-mrst.c which is not in current i2c > > subsystem yet, seems it has been submitted to i2c devel list > > before. And our platform has many identical adapters. > > I remember that driver, I reviewed it long ago (more than 1 year ago I > think). Apparently driver was renamed to i2c-intel-mid meanwhile. > Still not upstream indeed, but the last version of the driver posted > by Alan Cox [1] does _not_ set the i2c_adapter.class field. > > [1] http://marc.info/?l=linux-i2c&m=128292492431251&w=2 Yes, it was. But the latest code we are using set the class, that's why we hit the problem :( > > > > (...) > > > But you normally only load the hwmon drivers you need on a given > > > machine. That's only a couple of them. Or are you, by any chance, > > > building a monolithic kernel with all hwmon drivers included? This > > > would indeed be a problem, the i2c subsystem isn't prepared for > > > that. > > > > Actually we run into a long boot time problem while we build in only > > one i2c device driver (drivers/hwmon/emc1403.c) which has 4 > > addresses in its address lit, it took more than 10 seconds for the > > driver to init on our platforms. So if we build in more i2c hwmon > > drivers, things will get much worse. That's why Jacob posted the > > patch. > > This suggests that the i2c adapter driver is pretty bad at handling > NACKs when trying to access a non-existing device. It might be worth > checking if this can be improved, because that case could happen not > only during device detection but also in other circumstances. > > > I did read code about setting adapter->class, seems most driver set > > its class in their proble function, trying set it in platform code > > may looks not very clean. > > Actually this is very clean and by far my preferred way to solve your > problem. This is exactly how things are done on the PXA platform > (check i2c-pxa.c.) Yes, setting the class to 0 should fix our problem, though still need the driver author to confirm the change doesn't have other side effects. Thanks for your time helping figure this out. But I still have some concern (not specific to our platforms), I just checked, there are 42 i2c adapter drivers set their class to I2C_CLASS_HWMON and 50 hwmon i2c device drivers set it as well. So if a kernel has an adapter driver and hwmon driver which both set I2C_CLASS_HWMON, the long init time problem will show up again, and it will be worse if there are multiple adapters HW there. Thanks, Feng > > > Yours suggestion of using a build time option definitely will > > shrink the kernel size. But our platform is under arch/x86, and one > > general rule is to one generic kernel config should work for all > > x86 platforms, so I'm afraid it can't solve our problem. > > OK, fair enough. > > > So I still prefer the option of adding a flag, and leave the choice > > for platform code whether to do the detect. If you agree, I would > > make a cleaner patch. > > And I prefer that you set the class flag value in the platform data > and have your i2c adapter driver read it. This is already supported > and should work very fine without any change to i2c-core. I see no > point in adding a global flag when we already have a much > finer-grained setting available. > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
