Hi all, On Mon, 19 Sep 2016 11:02:47 +0200, Jean Delvare wrote: > My own proposal was checking __i2c_first_dynamic_bus_num. To be honest > I can't see the added value of relying on i2c_get_adapter() instead. > Would the result not be exactly the same? Plus it seems racy, just > because i2c_get_adapter() returns NULL at one point in time doesn't > mean the bus numbers will not have been assigned by the time you call > i2c_add_numbered_adapter(). > (...) > If you consider it a workaround, what would be the "real fix" for you? > > I was wondering if selecting one of these drivers could set a Kconfig > option to initialize __i2c_first_dynamic_bus_num to a non-zero value. > Unfortunately there does not seem to be a way to set a numeric value to > a Kconfig option using select. We would have to do it indirectly as > with CONFIG_HZ: > > choice > default I2C_RESERVED_BUS_NR_0 > config I2C_RESERVED_BUS_NR_0 > config I2C_RESERVED_BUS_NR_2 > endchoice > > config I2C_RESERVED_BUS_NR > int > default 0 if I2C_RESERVED_BUS_NR_0 > default 2 if I2C_RESERVED_BUS_NR_2 > > config I2C_EG20T > tristate "Intel EG20T PCH/LAPIS Semicon IOH(ML7213/ML7223/ML7831) I2C" > depends on PCI && (X86_32 || MIPS || COMPILE_TEST) > select I2C_RESERVED_BUS_NR_2 > > And in i2c-core.c: > > __i2c_first_dynamic_bus_num = I2C_RESERVED_BUS_NR; > > If that's possible at all... I'm not sure if select works on choice > config options. > > Alternatively we could set the default directly based on driver > selection: > > config I2C_RESERVED_BUS_NR > int > default 0 > default 2 if I2C_EG20T > > This is more simple but a little harder to maintain. One possible > problem is if the number of buses isn't known at build time but could > change depending on the hardware, for example. Also I don't know if > more than one such driver can be included in the same kernel. > > Or we can make it a user-visible option and leave it on whoever > configures the kernel to get it right. In all cases it would move the > decision to build-time instead of being set dynamically at run-time. > > Note that I am not claiming this is necessarily better than my initial > proposal. I just wanted to mention the possibility. I slept over it and start wondering if we aren't trying to address the problem at the wrong end. The initial reason why i2c_add_numbered_adapter() was used in the i2c-eg20t driver (and, I suppose, i2c-pasemi and all other bus drivers) is because the platform initialization code/data (native or OF-based) may be declaring slave I2C devices on these buses. This bumps __i2c_first_dynamic_bus_num, and the bus driver needs to call i2c_add_numbered_adapter() to get bus numbers below that. Maybe in that case the bus driver should ALSO get the static bus number information from the platform initialization code/data. Then the bus driver would check that information, and use it to call i2c_add_numbered_adapter() as appropriate if present, or simply call i2c_add_adapter() if not (or ask i2c_add_numbered_adapter for bus number -1, which has the same result.) Looks like drivers i2c-s3c2410, i2c-designware-*, i2c-kempld and i2c-pxa are already getting this right. So maybe we don't need to introduce another mechanism, but instead use what is already in place? -- Jean Delvare SUSE L3 Support -- 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
