Messages in this thread |  | | From | Chen-Yu Tsai <> | Date | Sat, 7 Jan 2023 13:43:57 +0800 | Subject | Re: [PATCH] platform/chromeos: cros_ec: Use per-device lockdep key |
| |
On Fri, Jan 6, 2023 at 5:08 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote: > > On Fri, Jan 06, 2023 at 12:55:37PM +0800, Chen-Yu Tsai wrote: > > 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 cros-ec-i2c (passthrough) I2C bus 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 > > happen to expose a cros-ec interface, but do not have a passthrough > > I2C bus > > > > In short, the "dependencies" are actually on different devices. > > Path of 4 looks weird to me. > > Could you point out where sbs_probe() gets to acquire ec_dev->lock?
sbs_probe() calls sbs_get_battery_presence_and_health(), which
-> does an I2C transfer. This SBS instance is connected on the I2C bus on the EC, so the I2C transfer
-> acquires i2c_adapter->bus_lock, and
-> calls ec_i2c_xfer(), which
-> calls cros_ec_cmd_xfer_status(), which
-> calls cros_ec_cmd_xfer(), which
-> acquires ec_dev->lock
> I may misunderstand: I thought there is no such I2C bus for passthrough > from kernel's point of view (as the bus and devices behind the EC). > See also [2].
It is an I2C adapter on the EC, also known as i2c-cros-ec-tunnel. Passthrough probably isn't the right word.
> [2]: https://elixir.bootlin.com/linux/v6.2-rc2/source/drivers/platform/chrome/cros_ec.c#L241 > > > On a related note, for the commit title: s/chromeos/chrome/ if it gets > chance to have next version.
OK.
Thanks ChenYu
|  |