Linux Media Controller development
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Hans Verkuil" <hverkuil+cisco@kernel.org>,
	"Sasha Levin" <sashal@kernel.org>,
	lars@metafoo.de, linux-media@vger.kernel.org
Subject: [PATCH AUTOSEL 6.17-5.15] media: adv7180: Add missing lock in suspend callback
Date: Sat, 25 Oct 2025 11:58:49 -0400	[thread overview]
Message-ID: <20251025160905.3857885-298-sashal@kernel.org> (raw)
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>

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;
-- 
2.51.0


  parent reply	other threads:[~2025-10-25 16:23 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:53 ` [PATCH AUTOSEL 6.17] media: nxp: imx8-isi: Fix streaming cleanup on release Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] drm/xe: improve dma-resv handling for backup object Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17-5.15] media: adv7180: Only validate format in querystd Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17-6.6] media: ov08x40: Fix the horizontal flip control Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-5.15] media: em28xx: add special case for legacy gpiolib interface Sasha Levin
2025-10-27  9:24   ` Arnd Bergmann
2025-11-04 13:55     ` Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-5.15] media: adv7180: Do not write format to device in set_fmt Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17] media: imx-mipi-csis: Only set clock rate when specified in DT Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] media: qcom: camss: csiphy-3ph: Add CSIPHY 2ph DPHY v2.0.1 init sequence Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-5.4] media: redrat3: use int type to store negative error codes Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-5.4] media: pci: ivtv: Don't create fake v4l2_fh Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-5.4] media: imon: make send_packet() more robust Sasha Levin
2025-10-25 15:58 ` Sasha Levin [this message]
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-6.1] media: verisilicon: Explicitly disable selection api ioctls for decoders Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-6.12] media: pci: mgb4: Fix timings comparison in VIDIOC_S_DV_TIMINGS Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-6.12] media: ipu6: isys: Set embedded data type correctly for metadata formats Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-6.1] media: i2c: og01a1b: Specify monochrome media bus format instead of Bayer Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.1] media: amphion: Delete v4l2_fh synchronously in .release() Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251025160905.3857885-298-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=hverkuil+cisco@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-media@vger.kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox