On 2018-06-14 17:41, Wolfram Sang wrote: > >> So, maybe the easier thing to do is change i2c_lock_adapter to only >> lock the segment, and then have the callers beneath drivers/i2c/ >> (plus the above mlx90614 driver) that really want to lock the root >> adapter instead of the segment adapter call a new function named >> i2c_lock_root (or something like that). Admittedly, that will be >> a few more trivial changes, but all but one will be under the I2C >> umbrella and thus require less interaction. >> >> Wolfram, what do you think? > > It sounds tempting, yet I am concerned about regressions. From that > point of view, it is safer to introduce i2c_lock_segment() and convert the > users which would benefit from that. How many drivers would be affected? Right, there is also the aspect that changing a function like this might surprise people. Maybe i2c_lock_adapter should be killed and all callers changed to one of i2c_lock_segment and i2c_lock_root? It's not that much churn... Current callers (I didn't hunt the very latest sources, nor -next) of i2c_lock_adapter are: drivers/i2c/i2c-core-slave.c Locks the adapter during (un)registration of the slave. Should keep locking the root adapter. drivers/i2c/busses/i2c-brcmstb.c Locks the adapter during suspend/resume. Should keep locking the root adapter. drivers/i2c/busses/i2c-davinci.c Locks the adapter if/when the CPU frequency changes. Should keep locking the root adapter. drivers/i2c/busses/i2c-gpio.c Locks the adapter while twiddling the lines from debugfs. Should keep locking the root adapter. drivers/i2c/busses/i2c-s3c2410.c Locks the adapter if/when the CPU frequency changes.Should keep locking the root adapter. drivers/i2c/busses/i2c-sprd.c Locks the adapter during suspend/resume. Should keep locking the root adapter. drivers/i2c/busses/i2c-tegra.c Locks the adapter during suspend/resume. Should keep locking the root adapter. drivers/i2c/muxes/i2c-mux-pca9541.c Locks the adapter during probe to make I2C transfers. Should only need to lock the segment. drivers/iio/temperature/mlx90614.c Mentioned previously up-thread, suspend/resume apparently does nasty things with the bus. Should probably keep locking the root adapter. drivers/char/tpm/tpm_i2c_infineon.c Does a bunch of __i2c_transfer calls inside the locked regions (and some sleeping). Should only need to lock the segment. drivers/input/touchscreen/rohm_bu21023.c Does a couple of __i2c_transfer calls inside the locked region. Should only need to lock the segment. drivers/media/dvb-frontends/af9013.c Does a one or two __i2c_transfer calls inside the locked regions. Should only need to lock the segment. drivers/media/dvb-frontends/drxk_hard.c This one is a bit hairy. It does all sorts of things inside the locked region. And it is a bit hard to say if the root adapter really needs to be locked. drivers/media/dvb-frontends/rtl2830.c Does regmap accesses inside the locked regions, with the regmap ops overridden to use __i2c_transfer. Should only need to lock the segment. drivers/media/dvb-frontends/tda1004x.c Does a bunch of __i2c_transfer calls inside the locked region. Should only need to lock the segment. drivers/media/tuners/tda18271-common.c Seems to opens a gate in conjunction with locking the adapter. So, I'm a bit uncertain what will happen on the other side of that gate if there is any stray thing happening on the I2C bus. drivers/mfd/88pm860x-i2c.c Does a bunch of adap->algo->master_xfer calls inside the locked regions. Should only need to lock the segment (and should probably be using __i2c_transfer). So, drxk_hard.c and tda18271-common.c are a bit uncertain. However, since these drivers do make calls to __i2c_transfer from inside their locked regions, both drivers will deadlock if the chips sit downstream from a mux-locked I2C mux. And if they don't, locking the segment and the adapter are equivalent. So, the risk of regressions are nil AFAICT. Famous last words... Cheers, Peter
