From: Niklas Söderlund niklas.soderlund+renesas@ragnatech.se
[ Upstream commit 878c496ac5080f94a93a9216a8f70cfd67ace8c9 ]
The adv7180_set_power() utilizes adv7180_write() which in turn requires the state mutex to be held, take it before calling adv7180_set_power() to avoid tripping a lockdep_assert_held().
Signed-off-by: Niklas Söderlund niklas.soderlund+renesas@ragnatech.se Signed-off-by: Hans Verkuil hverkuil+cisco@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- Bug fixed: adv7180_set_power() calls adv7180_write(), which asserts the device mutex must be held. See the assertion in `drivers/media/i2c/adv7180.c:246` (`lockdep_assert_held(&state->mutex)`) and the write from adv7180_set_power() in `drivers/media/i2c/adv7180.c:286`. Without holding the mutex, callers can trip lockdep and risk racy register writes. - Missing lock covered in suspend: The change adds `guard(mutex)(&state->mutex);` before powering down in the PM suspend callback, ensuring the lockdep requirement is satisfied when calling adv7180_set_power(false). In this tree that guard is present at `drivers/media/i2c/adv7180.c:1555`, matching the intent of the commit you’re evaluating. - Missing lock covered in resume: Similarly, the resume path takes the mutex before calling adv7180_set_power() to restore power state. In this tree that guard is present at `drivers/media/i2c/adv7180.c:1566`. This avoids unprotected register writes during resume, when concurrency is common. - Missing lock in set_pad_format when changing field: The diff also adds the mutex guard around the active-format case where `state->field` changes and the code power-cycles the device and calls adv7180_set_field_mode(). Since that sequence invokes adv7180_set_power(), it must hold the mutex to satisfy the lockdep assertion. Protecting this block is consistent with the rest of the driver, which already guards other adv7180_set_power() call sites (e.g., `drivers/media/i2c/adv7180.c:933`, `drivers/media/i2c/adv7180.c:939`, `drivers/media/i2c/adv7180.c:948`). - Scope and risk: The change is small, localized to the adv7180 driver’s state mutex usage, and does not alter interfaces or architecture. It enforces an existing locking contract rather than introducing new behavior. - User impact: Without this fix, users can hit lockdep warnings and potential races during suspend/resume or when changing the field mode through pad format. With the fix, register accesses are serialized as intended, preventing subtle resume/configuration issues. - Stable criteria: It’s a clear bug fix, minimal and self-contained, with low regression risk in a single driver. While the commit message doesn’t include a Stable tag, it meets stable backport rules (correctness fix, no new features).
Note: Older stable trees that lack the `guard(mutex)` helper will need a trivial adaptation to explicit `mutex_lock(&state->mutex)`/`mutex_unlock(&state->mutex)` at the same points.
drivers/media/i2c/adv7180.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c index 5d90b8ab9b6df..84600fa75ae8a 100644 --- a/drivers/media/i2c/adv7180.c +++ b/drivers/media/i2c/adv7180.c @@ -813,6 +813,8 @@ static int adv7180_set_pad_format(struct v4l2_subdev *sd,
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) { if (state->field != format->format.field) { + guard(mutex)(&state->mutex); + state->field = format->format.field; adv7180_set_power(state, false); adv7180_set_field_mode(state); @@ -1549,6 +1551,8 @@ static int adv7180_suspend(struct device *dev) struct v4l2_subdev *sd = dev_get_drvdata(dev); struct adv7180_state *state = to_state(sd);
+ guard(mutex)(&state->mutex); + return adv7180_set_power(state, false); }
@@ -1562,6 +1566,8 @@ static int adv7180_resume(struct device *dev) if (ret < 0) return ret;
+ guard(mutex)(&state->mutex); + ret = adv7180_set_power(state, state->powered); if (ret) return ret;