| Message ID | 1370367222-19353-1-git-send-email-grygorii.strashko@ti.com |
|---|---|
| State | Rejected |
| Headers | show |
On Tue, Jun 04, 2013 at 08:33:42PM +0300, Grygorii Strashko wrote: > Currently the I2C devices instantiation Method 3 "Probe an I2C bus for > certain devices" (see Documentation/i2c/instantiating-devices) is always > enabled for all platforms (boards) and can't be disabled. Not true. Set .class = 0 for your adapter. I always ask authors of new drivers if they really need to set .class to something.
Hi On 06/04/2013 08:49 PM, Wolfram Sang wrote: > On Tue, Jun 04, 2013 at 08:33:42PM +0300, Grygorii Strashko wrote: > >> Currently the I2C devices instantiation Method 3 "Probe an I2C bus for >> certain devices" (see Documentation/i2c/instantiating-devices) is always >> enabled for all platforms (boards) and can't be disabled. > Not true. Set .class = 0 for your adapter. I always ask authors of new > drivers if they really need to set .class to something. > Agree, sorry, my statement is wrong. it would be right to say "..can't be disabled without kernel code modification". Few notes here: 1) boot delay issue isn't related to new drivers. There are hwmon/lm75.c, i2c/busses/i2c-gpio.c and i2c/busses/i2c-omap.c 2) Initially, I've fighted with it on TI K3.4 product kernel where DT isn't supported yet. And first thing, which I've tried to do is to correct .class parameter for adapter, but with assumption: "Default behavior shouldn't be changed as I2C detection might be used by some of OMAP boards". I've started from i2c-omap.c and OMAP4/5 (my target). As result, I was need to make changes in *7* files to set and pass platform parameter to i2c-omap.c driver which will allow to change .class to 0 on demand. At this point, I've realized that i still need to deal with i2c-gpio.c - which is generic driver. 3) Thinking about Mainline: To reach the same target - no I2C detection - and taking into account above assumption "No changes in default behavior" the following will need to be done: - change i2c-omap/i2c-gpio DT bindings and add parameter which will allow to change .class value for adapter. Not sure, it's possible because this parameter will be Linux and not HW specific (smth. like "i2c_disable_detection") - update drivers i2c-omap/i2c-gpio to use "i2c_disable_detection" - update OMAP4/5 DTS files So, It seemed a good solution for me to add 6 lines of code in i2c-core.c instead of doing all that stuff. Thanks/sorry for your time. - grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 3) Thinking about Mainline: To reach the same target - no I2C > detection - and taking > into account above assumption "No changes in default behavior" > the following will need to be done: > - change i2c-omap/i2c-gpio DT bindings and add parameter which will > allow to change > .class value for adapter. Not sure, it's possible because this parameter > will be Linux and not HW specific (smth. like "i2c_disable_detection") > - update drivers i2c-omap/i2c-gpio to use "i2c_disable_detection" > - update OMAP4/5 DTS files > > So, It seemed a good solution for me to add 6 lines of code in i2c-core.c > instead of doing all that stuff. Well... I understand the "default behaviour" issue, yet I still think that setting class to 0 is the right thing to do. OMAP is an embedded SoC which always had i2c_board_info or devictree which are the preferred ways of instantiating. Given that, I would accept a patch setting it to 0. The more user friendly way might be to introduce a new class which makes users aware of the issue. Proof of concept follows, only compile tested.
Hi Wolfram, On 06/07/2013 12:06 PM, Wolfram Sang wrote: >> 3) Thinking about Mainline: To reach the same target - no I2C >> detection - and taking >> into account above assumption "No changes in default behavior" >> the following will need to be done: >> - change i2c-omap/i2c-gpio DT bindings and add parameter which will >> allow to change >> .class value for adapter. Not sure, it's possible because this parameter >> will be Linux and not HW specific (smth. like "i2c_disable_detection") >> - update drivers i2c-omap/i2c-gpio to use "i2c_disable_detection" >> - update OMAP4/5 DTS files >> >> So, It seemed a good solution for me to add 6 lines of code in i2c-core.c >> instead of doing all that stuff. > Well... I understand the "default behaviour" issue, yet I still think > that setting class to 0 is the right thing to do. OMAP is an embedded > SoC which always had i2c_board_info or devictree which are the preferred > ways of instantiating. Given that, I would accept a patch setting it to > 0. The more user friendly way might be to introduce a new class which > makes users aware of the issue. Proof of concept follows, only compile > tested. > That sounds good to me - I can prepare patch for i2c-omap.c. But, there is still an open question regarding *i2c-gpio.c* which, actually, a source of biggest part of delay. Potentially, it can be tunned using "timeout" value. But, again, this is additional work/patches to workaround unneeded system feature: - "timeout" should not break interaction with I2C devices on i2c-gpio bus and, at same time, speed up boot; - DTS/board files need to be updated. May be, just may be), we can continue with CONFIG_I2C_DISABLE_DEVICE_DETECTION, and print deprecation warning for each registered adapter if this config option is defined. - grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> That sounds good to me - I can prepare patch for i2c-omap.c. > But, there is still an open question regarding *i2c-gpio.c* which, > actually, a source of biggest part of delay. Why should the DEPRECATED flag not work with i2c-gpio?
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index e380c6e..f7b220b 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -124,4 +124,14 @@ config I2C_DEBUG_BUS a problem with I2C support and want to see more of what is going on. +config I2C_DISABLE_DEVICE_DETECTION + bool "I2C Disable device detection" + default n + help + Say Y here, if you have explicitly defined I2C device topology in DT + or platform code and don't need to detect presence of supported I2C + devices automatically (Documentation/i2c/instantiating-devices - + Method 1 and Method 2 are used only). Enabling this option allows to + reduce system boot time, especially in case when i2c-gpio driver is used. + endif # I2C diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 48e31ed..d5ab7a3 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -54,7 +54,9 @@ static DEFINE_MUTEX(core_lock); static DEFINE_IDR(i2c_adapter_idr); static struct device_type i2c_client_type; +#ifndef CONFIG_I2C_DISABLE_DEVICE_DETECTION static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver); +#endif /* ------------------------------------------------------------------------- */ @@ -958,7 +960,9 @@ static int i2c_do_add_adapter(struct i2c_driver *driver, struct i2c_adapter *adap) { /* Detect supported devices on that bus, and instantiate them */ +#ifndef CONFIG_I2C_DISABLE_DEVICE_DETECTION i2c_detect(adap, driver); +#endif /* Let legacy drivers scan this bus for matching devices */ if (driver->attach_adapter) { @@ -1672,6 +1676,7 @@ static int i2c_default_probe(struct i2c_adapter *adap, unsigned short addr) return err >= 0; } +#ifndef CONFIG_I2C_DISABLE_DEVICE_DETECTION static int i2c_detect_address(struct i2c_client *temp_client, struct i2c_driver *driver) { @@ -1760,6 +1765,7 @@ static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver) kfree(temp_client); return err; } +#endif int i2c_probe_func_quick_read(struct i2c_adapter *adap, unsigned short addr) {
Currently the I2C devices instantiation Method 3 "Probe an I2C bus for certain devices" (see Documentation/i2c/instantiating-devices) is always enabled for all platforms (boards) and can't be disabled. This feature introduces a system boot time delay for boards, where I2C devices topology is explicitly defined in DT or platform code (Method 1 and Method 2 are used only) and especially when i2c-gpio is used. For example: When CONFIG_SENSORS_LM75 option was enabled for omap4-sdp board - it introduces 5-6 ms boot delay. When CONFIG_SENSORS_LM75 option was enabled for omap5-uevm board, where i2c-gpio is used for HDMI edid reading - it introduces up to 5 sec boot delay. Hence, introduce CONFIG_I2C_DISABLE_DEVICE_DETECTION configuration option to allow disabling of I2C devices instantiation Method 3 "Probe an I2C bus for certain devices" if it's not needed (usually for embedded systems). CC: linux-i2c@vger.kernel.org CC: linux-kernel@vger.kernel.org CC: linux-omap@vger.kernel.org Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> --- drivers/i2c/Kconfig | 10 ++++++++++ drivers/i2c/i2c-core.c | 6 ++++++ 2 files changed, 16 insertions(+)