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