Skip to content

Commit 961a325

Browse files
wensTzung-Bi Shih
authored andcommitted
platform/chrome: cros_ec: Use per-device lockdep key
Lockdep reports a bogus possible deadlock on MT8192 Chromebooks due to the following lock sequences: 1. lock(i2c_register_adapter) [1]; lock(&ec_dev->lock) 2. lock(&ec_dev->lock); lock(prepare_lock); The actual dependency chains are much longer. The shortened version looks somewhat like: 1. cros-ec-rpmsg on mtk-scp ec_dev->lock -> prepare_lock 2. In rt5682_i2c_probe() on native I2C bus: prepare_lock -> regmap->lock -> (possibly) i2c_adapter->bus_lock 3. In rt5682_i2c_probe() on native I2C bus: regmap->lock -> i2c_adapter->bus_lock 4. In sbs_probe() on i2c-cros-ec-tunnel I2C bus attached on cros-ec: i2c_adapter->bus_lock -> ec_dev->lock While lockdep is correct that the shared lockdep classes have a circular dependency, it is bogus because a) 2+3 happen on a native I2C bus b) 4 happens on the actual EC on ChromeOS devices c) 1 happens on the SCP coprocessor on MediaTek Chromebooks that just happens to expose a cros-ec interface, but does not have an i2c-cros-ec-tunnel I2C bus In short, the "dependencies" are actually on different devices. Setup a per-device lockdep key for cros_ec devices so lockdep can tell the two instances apart. This helps with getting rid of the bogus lockdep warning. For ChromeOS devices that only have one cros-ec instance this doesn't change anything. Also add a missing mutex_destroy, just to make the teardown complete. [1] This is likely the per I2C bus lock with shared lockdep class Signed-off-by: Chen-Yu Tsai <[email protected]> Signed-off-by: Tzung-Bi Shih <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 5fa1dd8 commit 961a325

File tree

2 files changed

+15
-3
lines changed

2 files changed

+15
-3
lines changed

drivers/platform/chrome/cros_ec.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,14 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
199199
if (!ec_dev->dout)
200200
return -ENOMEM;
201201

202+
lockdep_register_key(&ec_dev->lockdep_key);
202203
mutex_init(&ec_dev->lock);
204+
lockdep_set_class(&ec_dev->lock, &ec_dev->lockdep_key);
203205

204206
err = cros_ec_query_all(ec_dev);
205207
if (err) {
206208
dev_err(dev, "Cannot identify the EC: error %d\n", err);
207-
return err;
209+
goto destroy_mutex;
208210
}
209211

210212
if (ec_dev->irq > 0) {
@@ -216,7 +218,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
216218
if (err) {
217219
dev_err(dev, "Failed to request IRQ %d: %d\n",
218220
ec_dev->irq, err);
219-
return err;
221+
goto destroy_mutex;
220222
}
221223
}
222224

@@ -227,7 +229,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
227229
if (IS_ERR(ec_dev->ec)) {
228230
dev_err(ec_dev->dev,
229231
"Failed to create CrOS EC platform device\n");
230-
return PTR_ERR(ec_dev->ec);
232+
err = PTR_ERR(ec_dev->ec);
233+
goto destroy_mutex;
231234
}
232235

233236
if (ec_dev->max_passthru) {
@@ -293,6 +296,9 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
293296
exit:
294297
platform_device_unregister(ec_dev->ec);
295298
platform_device_unregister(ec_dev->pd);
299+
destroy_mutex:
300+
mutex_destroy(&ec_dev->lock);
301+
lockdep_unregister_key(&ec_dev->lockdep_key);
296302
return err;
297303
}
298304
EXPORT_SYMBOL(cros_ec_register);
@@ -310,6 +316,8 @@ void cros_ec_unregister(struct cros_ec_device *ec_dev)
310316
if (ec_dev->pd)
311317
platform_device_unregister(ec_dev->pd);
312318
platform_device_unregister(ec_dev->ec);
319+
mutex_destroy(&ec_dev->lock);
320+
lockdep_unregister_key(&ec_dev->lockdep_key);
313321
}
314322
EXPORT_SYMBOL(cros_ec_unregister);
315323

include/linux/platform_data/cros_ec_proto.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#define __LINUX_CROS_EC_PROTO_H
1010

1111
#include <linux/device.h>
12+
#include <linux/lockdep_types.h>
1213
#include <linux/mutex.h>
1314
#include <linux/notifier.h>
1415

@@ -122,6 +123,8 @@ struct cros_ec_command {
122123
* command. The caller should check msg.result for the EC's result
123124
* code.
124125
* @pkt_xfer: Send packet to EC and get response.
126+
* @lockdep_key: Lockdep class for each instance. Unused if CONFIG_LOCKDEP is
127+
* not enabled.
125128
* @lock: One transaction at a time.
126129
* @mkbp_event_supported: 0 if MKBP not supported. Otherwise its value is
127130
* the maximum supported version of the MKBP host event
@@ -176,6 +179,7 @@ struct cros_ec_device {
176179
struct cros_ec_command *msg);
177180
int (*pkt_xfer)(struct cros_ec_device *ec,
178181
struct cros_ec_command *msg);
182+
struct lock_class_key lockdep_key;
179183
struct mutex lock;
180184
u8 mkbp_event_supported;
181185
bool host_sleep_v1;

0 commit comments

Comments
 (0)