| Message ID | 1421419194-1849-1-git-send-email-p.osmialowsk@samsung.com |
|---|---|
| State | RFC |
| Headers | show |
Hi, [CCing more people] 2015-01-16 23:39 GMT+09:00 Paul Osmialowski <p.osmialowsk@samsung.com>: > This enhancement of i2c API is designed to address following problem > caused by circular lock dependency: > > -> #1 (prepare_lock){+.+.+.}: > [ 2.730502] [<c0061e50>] __lock_acquire+0x3c0/0x8a4 > [ 2.735970] [<c0062868>] lock_acquire+0x6c/0x8c > [ 2.741090] [<c04a2724>] mutex_lock_nested+0x68/0x464 > [ 2.746733] [<c0395e84>] clk_prepare_lock+0x40/0xe8 > [ 2.752201] [<c0397698>] clk_unprepare+0x18/0x28 > [ 2.757409] [<c034cbb0>] s3c24xx_i2c_xfer+0xc8/0x124 > [ 2.762964] [<c03457e0>] __i2c_transfer+0x74/0x8c > [ 2.768259] [<c0347234>] i2c_transfer+0x78/0xec > [ 2.773380] [<c02a177c>] regmap_i2c_read+0x48/0x64 > [ 2.778761] [<c029d5c0>] _regmap_raw_read+0xa8/0xfc > [ 2.784230] [<c029d920>] _regmap_bus_read+0x28/0x48 > [ 2.789699] [<c029ce00>] _regmap_read+0x74/0xdc > [ 2.794819] [<c029d0ec>] _regmap_update_bits+0x24/0x70 > [ 2.800549] [<c029e348>] regmap_update_bits+0x40/0x5c > [ 2.806190] [<c024c3c4>] _regulator_do_disable+0x68/0x7c > [ 2.812093] [<c024f4d8>] _regulator_disable+0x90/0x12c > [ 2.817822] [<c024f5a8>] regulator_disable+0x34/0x60 > [ 2.823377] [<c0363070>] mmc_regulator_set_ocr+0x74/0xdc > [ 2.829279] [<c03783e8>] sdhci_set_power+0x38/0x20c > [ 2.834748] [<c0379c10>] sdhci_do_set_ios+0x180/0x450 > [ 2.840389] [<c0379f00>] sdhci_set_ios+0x20/0x2c > [ 2.845597] [<c0364978>] mmc_set_initial_state+0x5c/0xbc > [ 2.851500] [<c0364f48>] mmc_power_off+0x2c/0x60 > [ 2.856708] [<c0365c00>] mmc_rescan+0x268/0x27c > [ 2.861829] [<c003a128>] process_one_work+0x18c/0x3f8 > [ 2.867471] [<c003a400>] worker_thread+0x38/0x2f8 > [ 2.872766] [<c003f66c>] kthread+0xe4/0x104 > [ 2.877540] [<c000f0a8>] ret_from_fork+0x14/0x2c > [ 2.882749] > -> #0 (&map->mutex){+.+...}: > [ 2.886828] [<c0061224>] validate_chain.isra.25+0x3bc/0x548 > [ 2.892990] [<c0061e50>] __lock_acquire+0x3c0/0x8a4 > [ 2.898459] [<c0062868>] lock_acquire+0x6c/0x8c > [ 2.903580] [<c04a2724>] mutex_lock_nested+0x68/0x464 > [ 2.909222] [<c029ce9c>] regmap_read+0x34/0x5c > [ 2.914257] [<c039a994>] max_gen_clk_is_prepared+0x1c/0x38 > [ 2.920333] [<c0396ec4>] clk_unprepare_unused_subtree+0x64/0x98 > [ 2.926842] [<c0396f78>] clk_disable_unused+0x80/0xd8 > [ 2.932484] [<c00089d0>] do_one_initcall+0xac/0x1f0 > [ 2.937953] [<c068bd44>] do_basic_setup+0x90/0xc8 > [ 2.943248] [<c068be00>] kernel_init_freeable+0x84/0x120 > [ 2.949150] [<c0491248>] kernel_init+0x8/0xec > [ 2.954097] [<c000f0a8>] ret_from_fork+0x14/0x2c > [ 2.959307] > [ 2.959307] other info that might help us debug this: > [ 2.959307] > [ 2.967293] Possible unsafe locking scenario: > [ 2.967293] > [ 2.973194] CPU0 CPU1 > [ 2.977708] ---- ---- > [ 2.982221] lock(prepare_lock); > [ 2.985520] lock(&map->mutex); > [ 2.991248] lock(prepare_lock); > [ 2.997063] lock(&map->mutex); > [ 3.000276] > [ 3.000276] *** DEADLOCK *** > > Apparently regulator and clock try to acquire lock which is protecting the > same regmap. Communication over i2c requires clock to be started. Both things > require access to the same regmap in order to complete. I stumbled upon this issue (and reported it) quite long time ago already, but apparently nobody cared too much (including myself, unfortunately). Please see [1] for details. [1] https://lkml.org/lkml/2014/7/2/171 We have here a typical ABBA deadlock scenario, between I2C clock providers and other (logical) devices on the same (physical) I2C device, for which a regmap is used to share the registers between drivers. Remaining factor here is I2C controller driver, which must perform clock gating in I2C ops to trigger this deadlock. The problem is that any operation on such I2C clock will first try to acquire clk_prepare_mutex and then, through driver's callback, access the regmap, acquiring its mutex (then an I2C transfer will happen, but it irrelevant in this context). On opposite side we have drivers for other functionality exposed by this I2C device, which will access the regmap, acquiring its mutex and causing I2C transfers to happen. The key here is that I2C transfers might require some clocks to be prepared, so clk_prepare() might get called from this context and cause a deadlock, because clk_prepare_mutex might have been already acquired by another context, waiting for regmap mutex, which has been already acquired by this context. Now, for the solution, the approach proposed by Paul, as far as I could understand it by reading the code (it's definitely lacking a cover letter with detailed explanation), should solve the issue by enforcing correct locking order at regmap level. However I wonder if we really need a heavy solution like this or we could just make I2C drivers not require clock preparation in I2C transfers, as suggested by Peter De Schrijver in [1], which should fix the issue as well. So, the question is, do we actually have hardware that _really_ requires _actual_ preparation or all the clk_prepare_enable()s in I2C drivers (at least in i2c-s3c2410) are just to simplify the code? Best regards, Tomasz -- 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
W dniu 18.01.2015 o 07:30, Tomasz Figa pisze: > Hi, > > [CCing more people] > > 2015-01-16 23:39 GMT+09:00 Paul Osmialowski <p.osmialowsk@samsung.com>: >> This enhancement of i2c API is designed to address following problem >> caused by circular lock dependency: >> >> -> #1 (prepare_lock){+.+.+.}: >> [ 2.730502] [<c0061e50>] __lock_acquire+0x3c0/0x8a4 >> [ 2.735970] [<c0062868>] lock_acquire+0x6c/0x8c >> [ 2.741090] [<c04a2724>] mutex_lock_nested+0x68/0x464 >> [ 2.746733] [<c0395e84>] clk_prepare_lock+0x40/0xe8 >> [ 2.752201] [<c0397698>] clk_unprepare+0x18/0x28 >> [ 2.757409] [<c034cbb0>] s3c24xx_i2c_xfer+0xc8/0x124 >> [ 2.762964] [<c03457e0>] __i2c_transfer+0x74/0x8c >> [ 2.768259] [<c0347234>] i2c_transfer+0x78/0xec >> [ 2.773380] [<c02a177c>] regmap_i2c_read+0x48/0x64 >> [ 2.778761] [<c029d5c0>] _regmap_raw_read+0xa8/0xfc >> [ 2.784230] [<c029d920>] _regmap_bus_read+0x28/0x48 >> [ 2.789699] [<c029ce00>] _regmap_read+0x74/0xdc >> [ 2.794819] [<c029d0ec>] _regmap_update_bits+0x24/0x70 >> [ 2.800549] [<c029e348>] regmap_update_bits+0x40/0x5c >> [ 2.806190] [<c024c3c4>] _regulator_do_disable+0x68/0x7c >> [ 2.812093] [<c024f4d8>] _regulator_disable+0x90/0x12c >> [ 2.817822] [<c024f5a8>] regulator_disable+0x34/0x60 >> [ 2.823377] [<c0363070>] mmc_regulator_set_ocr+0x74/0xdc >> [ 2.829279] [<c03783e8>] sdhci_set_power+0x38/0x20c >> [ 2.834748] [<c0379c10>] sdhci_do_set_ios+0x180/0x450 >> [ 2.840389] [<c0379f00>] sdhci_set_ios+0x20/0x2c >> [ 2.845597] [<c0364978>] mmc_set_initial_state+0x5c/0xbc >> [ 2.851500] [<c0364f48>] mmc_power_off+0x2c/0x60 >> [ 2.856708] [<c0365c00>] mmc_rescan+0x268/0x27c >> [ 2.861829] [<c003a128>] process_one_work+0x18c/0x3f8 >> [ 2.867471] [<c003a400>] worker_thread+0x38/0x2f8 >> [ 2.872766] [<c003f66c>] kthread+0xe4/0x104 >> [ 2.877540] [<c000f0a8>] ret_from_fork+0x14/0x2c >> [ 2.882749] >> -> #0 (&map->mutex){+.+...}: >> [ 2.886828] [<c0061224>] validate_chain.isra.25+0x3bc/0x548 >> [ 2.892990] [<c0061e50>] __lock_acquire+0x3c0/0x8a4 >> [ 2.898459] [<c0062868>] lock_acquire+0x6c/0x8c >> [ 2.903580] [<c04a2724>] mutex_lock_nested+0x68/0x464 >> [ 2.909222] [<c029ce9c>] regmap_read+0x34/0x5c >> [ 2.914257] [<c039a994>] max_gen_clk_is_prepared+0x1c/0x38 >> [ 2.920333] [<c0396ec4>] clk_unprepare_unused_subtree+0x64/0x98 >> [ 2.926842] [<c0396f78>] clk_disable_unused+0x80/0xd8 >> [ 2.932484] [<c00089d0>] do_one_initcall+0xac/0x1f0 >> [ 2.937953] [<c068bd44>] do_basic_setup+0x90/0xc8 >> [ 2.943248] [<c068be00>] kernel_init_freeable+0x84/0x120 >> [ 2.949150] [<c0491248>] kernel_init+0x8/0xec >> [ 2.954097] [<c000f0a8>] ret_from_fork+0x14/0x2c >> [ 2.959307] >> [ 2.959307] other info that might help us debug this: >> [ 2.959307] >> [ 2.967293] Possible unsafe locking scenario: >> [ 2.967293] >> [ 2.973194] CPU0 CPU1 >> [ 2.977708] ---- ---- >> [ 2.982221] lock(prepare_lock); >> [ 2.985520] lock(&map->mutex); >> [ 2.991248] lock(prepare_lock); >> [ 2.997063] lock(&map->mutex); >> [ 3.000276] >> [ 3.000276] *** DEADLOCK *** >> >> Apparently regulator and clock try to acquire lock which is protecting the >> same regmap. Communication over i2c requires clock to be started. Both things >> require access to the same regmap in order to complete. > > I stumbled upon this issue (and reported it) quite long time ago > already, but apparently nobody cared too much (including myself, > unfortunately). Please see [1] for details. > > [1] https://lkml.org/lkml/2014/7/2/171 > > We have here a typical ABBA deadlock scenario, between I2C clock > providers and other (logical) devices on the same (physical) I2C > device, for which a regmap is used to share the registers between > drivers. Remaining factor here is I2C controller driver, which must > perform clock gating in I2C ops to trigger this deadlock. > > The problem is that any operation on such I2C clock will first try to > acquire clk_prepare_mutex and then, through driver's callback, access > the regmap, acquiring its mutex (then an I2C transfer will happen, but > it irrelevant in this context). On opposite side we have drivers for > other functionality exposed by this I2C device, which will access the > regmap, acquiring its mutex and causing I2C transfers to happen. > > The key here is that I2C transfers might require some clocks to be > prepared, so clk_prepare() might get called from this context and > cause a deadlock, because clk_prepare_mutex might have been already > acquired by another context, waiting for regmap mutex, which has been > already acquired by this context. > > Now, for the solution, the approach proposed by Paul, as far as I > could understand it by reading the code (it's definitely lacking a > cover letter with detailed explanation), should solve the issue by > enforcing correct locking order at regmap level. However I wonder if > we really need a heavy solution like this or we could just make I2C > drivers not require clock preparation in I2C transfers, as suggested > by Peter De Schrijver in [1], which should fix the issue as well. > > So, the question is, do we actually have hardware that _really_ > requires _actual_ preparation or all the clk_prepare_enable()s in I2C > drivers (at least in i2c-s3c2410) are just to simplify the code? Hi Tomasz, I completely forgot that you already thought about this deadlock in 2014. I think we can try the no-prepare way for i2c-s3c2410. However this would be only workaround for specific chip. Other buses (like SPI) would require similar changes. I wondered why this was not observed (at least not observed by me with lockdep) on Gear 2 (Rinato) board. This is quite similar case: the S2MPS14 PMIC provides regulators and 32kHz clocks. I think it is exactly the same pattern as for max77686... but somehow lockdep never reported that deadlock there. Anyway thanks for pointing out old discussion. Best regards, Krzysztof > > Best regards, > Tomasz > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- 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
On Sun, Jan 18, 2015 at 11:54:56AM +0100, Krzysztof Kozlowski wrote: > W dniu 18.01.2015 o 07:30, Tomasz Figa pisze: > >So, the question is, do we actually have hardware that _really_ > >requires _actual_ preparation or all the clk_prepare_enable()s in I2C > >drivers (at least in i2c-s3c2410) are just to simplify the code? > I completely forgot that you already thought about this deadlock in 2014. I > think we can try the no-prepare way for i2c-s3c2410. However this would be > only workaround for specific chip. Other buses (like SPI) would require > similar changes. Right, and it's every single driver which would need an update too which is a bit icky and sad. On the other hand a more detailed fix is going to involve trying to make the clock API locking more fine grained which isn't fun. > I wondered why this was not observed (at least not observed by me with > lockdep) on Gear 2 (Rinato) board. This is quite similar case: the S2MPS14 > PMIC provides regulators and 32kHz clocks. I think it is exactly the same > pattern as for max77686... but somehow lockdep never reported that deadlock > there. Mostly the clocks on PMICs never get changed at runtime which helps a lot here.
Quoting Mark Brown (2015-01-18 05:41:24) > On Sun, Jan 18, 2015 at 11:54:56AM +0100, Krzysztof Kozlowski wrote: > > W dniu 18.01.2015 o 07:30, Tomasz Figa pisze: > > > >So, the question is, do we actually have hardware that _really_ > > >requires _actual_ preparation or all the clk_prepare_enable()s in I2C > > >drivers (at least in i2c-s3c2410) are just to simplify the code? > > > I completely forgot that you already thought about this deadlock in 2014. I > > think we can try the no-prepare way for i2c-s3c2410. However this would be > > only workaround for specific chip. Other buses (like SPI) would require > > similar changes. > > Right, and it's every single driver which would need an update too which > is a bit icky and sad. On the other hand a more detailed fix is going > to involve trying to make the clock API locking more fine grained which > isn't fun. Not fun is right. Please see Stephen's attempt here: http://lkml.kernel.org/r/<1409792466-5092-1-git-send-email-sboyd@codeaurora.org> I'm hoping this approach will be revisited soon. Regards, Mike > > > I wondered why this was not observed (at least not observed by me with > > lockdep) on Gear 2 (Rinato) board. This is quite similar case: the S2MPS14 > > PMIC provides regulators and 32kHz clocks. I think it is exactly the same > > pattern as for max77686... but somehow lockdep never reported that deadlock > > there. > > Mostly the clocks on PMICs never get changed at runtime which helps a > lot here. -- 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
diff --git a/Documentation/i2c/writing-clients b/Documentation/i2c/writing-clients index a755b14..d6732a8 100644 --- a/Documentation/i2c/writing-clients +++ b/Documentation/i2c/writing-clients @@ -343,6 +343,21 @@ stop bit is sent between transaction. The i2c_msg structure contains for each message the client address, the number of bytes of the message and the message data itself. + int i2c_transfer_prepare(struct i2c_adapter *adap); + void i2c_transfer_unprepare(struct i2c_adapter *adap); + +These routines are used for calling exposed prepare and unprepare stages of +the i2c_transfer() function. Normally i2c_transfer() does some operations +(e.g. clock preparation) before actual transfer. These operations are undone +when transfer is finished. Unfortunately these preparations may take locks, +even causing circular lock dependency. Sometimes the only way to avoid this +is to call i2c_transfer_prepare() earlier. Note that these routines do +nothing when bus driver does not implement `master_prepare_xfer' nor +`master_unprepare_xfer' callbacks pointed by struct i2c_algorithm. +Note that underlying bus driver implementing `master_xfer` callback +should be aware of the fact that preparation stage was or was not invoked +before i2c_transfer() function call and act accordingly. + You can read the file `i2c-protocol' for more information about the actual I2C protocol. diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 39d25a8..d649475 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -2064,6 +2064,111 @@ module_exit(i2c_exit); */ /** + * __i2c_transfer_prepare - unlocked flavor of prepare stage of transfer of + * a single or combined I2C message (may sleep). + * @adap: Handle to I2C bus + * + * This stage, if exposed by underlying driver, can be called before transfer + * in order to avoid deadlock caused by circular lock dependency. + * + * Returns 0 on success. + * + * Adapter lock must be held when calling this function. No debug logging + * takes place. + */ +int __i2c_transfer_prepare(struct i2c_adapter *adap) +{ + if (adap->algo->master_prepare_xfer) + return adap->algo->master_prepare_xfer(adap); + + return 0; +} +EXPORT_SYMBOL(__i2c_transfer_prepare); + +/** + * i2c_transfer_prepare - prepare transfer of a single or combined I2C message + * (may sleep). + * @adap: Handle to I2C bus + * + * Returns 0 on success. + * + * This stage, if exposed by underlying driver, can be called before transfer + * in order to avoid deadlock caused by circular lock dependency. + */ +int i2c_transfer_prepare(struct i2c_adapter *adap) +{ + int ret = 0; + + if (adap->algo->master_prepare_xfer) { + if (in_atomic() || irqs_disabled()) { + ret = i2c_trylock_adapter(adap); + if (!ret) + /* I2C activity is ongoing. */ + return -EAGAIN; + } else { + i2c_lock_adapter(adap); + } + ret = __i2c_transfer_prepare(adap); + i2c_unlock_adapter(adap); + } + + return ret; +} +EXPORT_SYMBOL(i2c_transfer_prepare); + +/** + * __i2c_transfer_unprepare - unlocked flavor of prepare stage of transfer of + * a single or combined I2C message (may sleep). + * @adap: Handle to I2C bus + * + * This stage, if exposed by underlying driver, can be called afrer transfer + * in order to avoid deadlock caused by circular lock dependency. + * + * Adapter lock must be held when calling this function. No debug logging + * takes place. + */ +void __i2c_transfer_unprepare(struct i2c_adapter *adap) +{ + if (adap->algo->master_unprepare_xfer) + adap->algo->master_unprepare_xfer(adap); +} +EXPORT_SYMBOL(__i2c_transfer_unprepare); + +/** + * i2c_transfer_unprepare - unprepare after transfer of a single or combined + * I2C message (may sleep). + * @adap: Handle to I2C bus + * + * This stage, if exposed by underlying driver, can be called afrer transfer + * in order to avoid deadlock caused by circular lock dependency. + */ +void i2c_transfer_unprepare(struct i2c_adapter *adap) +{ + if (adap->algo->master_unprepare_xfer) { + if (in_atomic() || irqs_disabled()) { + /* Wait max 20 ms */ + int timeout = 20; + + while (!(i2c_trylock_adapter(adap))) { + if (timeout == 0) { + /* I2C activity is still ongoing. */ + pr_err("%s: can't unprepare transfer\n", + __func__); + return; + } + timeout--; + mdelay(1); + } + } else { + i2c_lock_adapter(adap); + } + __i2c_transfer_unprepare(adap); + i2c_unlock_adapter(adap); + } +} +EXPORT_SYMBOL(i2c_transfer_unprepare); + +/** * __i2c_transfer - unlocked flavor of i2c_transfer * @adap: Handle to I2C bus * @msgs: One or more messages to execute before STOP is issued to diff --git a/include/linux/i2c.h b/include/linux/i2c.h index e3a1721..fce2a5a 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -64,6 +64,14 @@ extern int i2c_master_send(const struct i2c_client *client, const char *buf, extern int i2c_master_recv(const struct i2c_client *client, char *buf, int count); +/* Transfer prepare/unprepare stage (if needed/exposed). + */ +extern int i2c_transfer_prepare(struct i2c_adapter *adap); +extern void i2c_transfer_unprepare(struct i2c_adapter *adap); +/* Unlocked flavor */ +extern int __i2c_transfer_prepare(struct i2c_adapter *adap); +extern void __i2c_transfer_unprepare(struct i2c_adapter *adap); + /* Transfer num messages. */ extern int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, @@ -371,6 +379,12 @@ i2c_register_board_info(int busnum, struct i2c_board_info const *info, * @master_xfer: Issue a set of i2c transactions to the given I2C adapter * defined by the msgs array, with num messages available to transfer via * the adapter specified by adap. + * @master_prepare_xfer: Exposed preparation stage of @master_xfer - use when + * master_xfer must be split into stages: prepare, xfer, unprepare. + * Set to NULL when not exposed. + * @master_unprepare_xfer: Exposed unpreparation stage of @master_xfer - + * use when master_xfer must be split into stages: prepare, xfer, unprepare. + * Set to NULL when not exposed. * @smbus_xfer: Issue smbus transactions to the given I2C adapter. If this * is not present, then the bus layer will try and convert the SMBus calls * into I2C transfers instead. @@ -397,6 +411,8 @@ struct i2c_algorithm { processed, or a negative value on error */ int (*master_xfer)(struct i2c_adapter *adap, struct i2c_msg *msgs, int num); + int (*master_prepare_xfer)(struct i2c_adapter *adap); + void (*master_unprepare_xfer)(struct i2c_adapter *adap); int (*smbus_xfer) (struct i2c_adapter *adap, u16 addr, unsigned short flags, char read_write, u8 command, int size, union i2c_smbus_data *data);
This enhancement of i2c API is designed to address following problem caused by circular lock dependency: -> #1 (prepare_lock){+.+.+.}: [ 2.730502] [<c0061e50>] __lock_acquire+0x3c0/0x8a4 [ 2.735970] [<c0062868>] lock_acquire+0x6c/0x8c [ 2.741090] [<c04a2724>] mutex_lock_nested+0x68/0x464 [ 2.746733] [<c0395e84>] clk_prepare_lock+0x40/0xe8 [ 2.752201] [<c0397698>] clk_unprepare+0x18/0x28 [ 2.757409] [<c034cbb0>] s3c24xx_i2c_xfer+0xc8/0x124 [ 2.762964] [<c03457e0>] __i2c_transfer+0x74/0x8c [ 2.768259] [<c0347234>] i2c_transfer+0x78/0xec [ 2.773380] [<c02a177c>] regmap_i2c_read+0x48/0x64 [ 2.778761] [<c029d5c0>] _regmap_raw_read+0xa8/0xfc [ 2.784230] [<c029d920>] _regmap_bus_read+0x28/0x48 [ 2.789699] [<c029ce00>] _regmap_read+0x74/0xdc [ 2.794819] [<c029d0ec>] _regmap_update_bits+0x24/0x70 [ 2.800549] [<c029e348>] regmap_update_bits+0x40/0x5c [ 2.806190] [<c024c3c4>] _regulator_do_disable+0x68/0x7c [ 2.812093] [<c024f4d8>] _regulator_disable+0x90/0x12c [ 2.817822] [<c024f5a8>] regulator_disable+0x34/0x60 [ 2.823377] [<c0363070>] mmc_regulator_set_ocr+0x74/0xdc [ 2.829279] [<c03783e8>] sdhci_set_power+0x38/0x20c [ 2.834748] [<c0379c10>] sdhci_do_set_ios+0x180/0x450 [ 2.840389] [<c0379f00>] sdhci_set_ios+0x20/0x2c [ 2.845597] [<c0364978>] mmc_set_initial_state+0x5c/0xbc [ 2.851500] [<c0364f48>] mmc_power_off+0x2c/0x60 [ 2.856708] [<c0365c00>] mmc_rescan+0x268/0x27c [ 2.861829] [<c003a128>] process_one_work+0x18c/0x3f8 [ 2.867471] [<c003a400>] worker_thread+0x38/0x2f8 [ 2.872766] [<c003f66c>] kthread+0xe4/0x104 [ 2.877540] [<c000f0a8>] ret_from_fork+0x14/0x2c [ 2.882749] -> #0 (&map->mutex){+.+...}: [ 2.886828] [<c0061224>] validate_chain.isra.25+0x3bc/0x548 [ 2.892990] [<c0061e50>] __lock_acquire+0x3c0/0x8a4 [ 2.898459] [<c0062868>] lock_acquire+0x6c/0x8c [ 2.903580] [<c04a2724>] mutex_lock_nested+0x68/0x464 [ 2.909222] [<c029ce9c>] regmap_read+0x34/0x5c [ 2.914257] [<c039a994>] max_gen_clk_is_prepared+0x1c/0x38 [ 2.920333] [<c0396ec4>] clk_unprepare_unused_subtree+0x64/0x98 [ 2.926842] [<c0396f78>] clk_disable_unused+0x80/0xd8 [ 2.932484] [<c00089d0>] do_one_initcall+0xac/0x1f0 [ 2.937953] [<c068bd44>] do_basic_setup+0x90/0xc8 [ 2.943248] [<c068be00>] kernel_init_freeable+0x84/0x120 [ 2.949150] [<c0491248>] kernel_init+0x8/0xec [ 2.954097] [<c000f0a8>] ret_from_fork+0x14/0x2c [ 2.959307] [ 2.959307] other info that might help us debug this: [ 2.959307] [ 2.967293] Possible unsafe locking scenario: [ 2.967293] [ 2.973194] CPU0 CPU1 [ 2.977708] ---- ---- [ 2.982221] lock(prepare_lock); [ 2.985520] lock(&map->mutex); [ 2.991248] lock(prepare_lock); [ 2.997063] lock(&map->mutex); [ 3.000276] [ 3.000276] *** DEADLOCK *** Apparently regulator and clock try to acquire lock which is protecting the same regmap. Communication over i2c requires clock to be started. Both things require access to the same regmap in order to complete. To solve this, i2c clock should be started before attempting operation on regulator (which requires locked regmap). Exposing preparation and unpreparation stage of i2c transfer serves this purpose. Note that this change does not require modifications in other places: old behaviour is kept preserved. Anyone who requires this new way of using i2c transfer can adapt their drivers voluntarily. Separate commit adapts regmap to use this new feature. Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com> --- Documentation/i2c/writing-clients | 15 ++++++ drivers/i2c/i2c-core.c | 105 ++++++++++++++++++++++++++++++++++++++ include/linux/i2c.h | 16 ++++++ 3 files changed, 136 insertions(+)