mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 00/21] media: i2c: it6625: address review feedback and adopt subdev state
@ 2026-09-30  6:41 Hermes Wu via B4 Relay
  2026-09-30  6:41 ` [PATCH v2 01/21] dt-bindings: media: ite,it6625: document the default CSI-2 bus type Hermes Wu via B4 Relay
                   ` (20 more replies)
  0 siblings, 21 replies; 23+ messages in thread
From: Hermes Wu via B4 Relay @ 2026-09-30  6:41 UTC (permalink / raw)
  To: Hermes Wu, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Hans Verkuil
  Cc: Sakari Ailus, linux-media, devicetree, linux-kernel, Hermes Wu

Review feedback came in on "[PATCH v10 2/2] media: i2c: add driver for
ITE IT6625/IT6626" and, separately, on "[PATCH v1 1/2]" (the dt-bindings
patch), after both had already been merged into next. This series
addresses that feedback as a follow-up, since the original commits can
no longer be amended.

Scope of this series:
 - one dt-bindings fix documenting the CSI-2 bus-type default;
 - one control-update error-handling fix;
 - mechanical and style cleanups (declaration ordering, unsigned loop
   indices, redundant boilerplate, unaligned-access helpers, an early
   return conversion);
 - a link-frequency reporting fix for one-/two-trio C-PHY configurations,
   found while implementing a related style request;
 - tightening DT endpoint parsing to require the documented endpoint
   instead of silently defaulting;
 - folding subdev initialization into probe() for explicit error
   handling;
 - adopting the subdev active-state model: moving the current format
   out of driver-private fields and into the pad format, sharing the
   control handler's own lock as the subdev state lock instead of
   introducing another private mutex for it, keeping the existing
   it6625_lock independent as the lock for serialized hardware/register
   operations and driver-private timing state (kept separate chiefly
   for the MCU host interface's own multi-step transactions), and
   calling v4l2_subdev_init_finalize()/v4l2_subdev_cleanup();
 - converting from the deprecated s_stream video op to the
   enable_streams/disable_streams pad ops, keeping
   v4l2_subdev_s_stream_helper for legacy callers.

The locking-model change is the one part of this series most likely to
need another look: it changes what the driver's own mutex protects and
who else already holds it by the time driver code runs. It was refined
during v1's review -- see patch 20's and patch 21's commit messages in
this revision for the current design (the control handler's own lock
shared as the subdev state lock, it6625_lock kept independent for
serialized hardware/register operations and driver-private timing
state), isolated to those two commits and kept separate from the
mechanical/bug-fix commits that precede them.

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
Changes in v2:
- Patch 02: propagate it6625_initial_setup()'s register-write errors
  and it6625_v4l2_sd_ctrl_update()'s control errors out of probe()
  instead of discarding them; probe() now fails via dev_err_probe() on
  either
- Patch 12: use HZ_PER_KHZ (kHz to Hz) instead of the wrongly-named
  KHZ_PER_MHZ -- both are numerically 1000, so this has no functional
  effect (found by sashiko.dev)
- Patch 13: define the detected-timings register-layout structs at file
  scope, immediately above the function that uses them, instead of as
  local variables
- Patch 17: demote -EPROBE_DEFER to dev_dbg() with dev_err_probe()
  instead of logging it as an error on every retried probe (found by
  sashiko.dev)
- Patch 19: add an err_clean_ctrl_handler label instead of freeing the
  control handler inline on a media_entity_pads_init() failure
- Patch 20: share the control handler's own lock as the subdev state lock
  (sd->state_lock = sd->ctrl_handler->lock) instead of a separate shared
  mutex for both, keep it6625_lock as an independent
  MCU/register-transaction lock, and take the active state's lock
  explicitly at every call site that reaches it outside a core-locked
  path (probe-time setup, an IRQ callback, and the two ioctls the core
  doesn't pre-lock a state for). Make it6625_init_state() seed every
  state -- active and TRY alike -- from the current it6625->timings and
  the default format index (so a TRY state opened after signal detection
  still reflects live detected timings, not frozen boot defaults) instead
  of branching on whether the active state exists yet, and unify the
  TRY/ACTIVE commit tail in it6625_set_fmt(). Trim the commit message and
  move the lock-interleaving trace out of it.
- Patch 21: take it6625_lock explicitly in it6625_enable_streams()/
  it6625_disable_streams(), since the core-held state lock is no longer
  the same mutex after patch 20's locking-model change.
- Rebased onto current origin/next (b38d06ad1e1 -> 2dcdfb625c3);
  it6625_set_fmt() now takes the unused const struct
  v4l2_subdev_client_info *ci parameter added by commit 7eef49c16461
  ("media: v4l2-subdev: Add struct v4l2_subdev_client_info pointer to
  pad ops")
- Link to v1: https://lore.kernel.org/r/20260918-upstream-it6625-follow-up-patch-v1-0-78d72d7886a5@ite.com.tw

---
Hermes Wu (21):
      dt-bindings: media: ite,it6625: document the default CSI-2 bus type
      media: i2c: it6625: propagate initial-setup and control-update errors
      media: i2c: it6625: default the debug module parameter to 0
      media: i2c: it6625: drop unused bus field from struct it6625
      media: i2c: it6625: drop stale GCC < 4.4.6 workaround
      media: i2c: it6625: use unsigned int loop indices in table lookups
      media: i2c: it6625: drop redundant parentheses in status helpers
      media: i2c: it6625: make the audio sampling-rate table static const
      media: i2c: it6625: tidy CEC buffer init and a continuation line
      media: i2c: it6625: clean up it6625_wait_for_status()
      media: i2c: it6625: use unsigned int indices in EDID read/write
      media: i2c: it6625: use unaligned/units helpers to decode pixel clock
      media: i2c: it6625: decode detected timings via typed register structs
      media: i2c: it6625: fix link-frequency reporting for one-/two-trio C-PHY
      media: i2c: it6625: use early returns in it6625_update_timings_if_changed()
      media: i2c: it6625: drop the private CSI-format name table
      media: i2c: it6625: require a DT endpoint and simplify endpoint parsing
      media: i2c: it6625: finish reverse fir-tree declaration order
      media: i2c: it6625: fold subdev initialization into probe
      media: i2c: it6625: use centrally managed active state
      media: i2c: it6625: use enable_streams and disable_streams

 .../devicetree/bindings/media/i2c/ite,it6625.yaml  |   2 +
 drivers/media/i2c/it6625.c                         | 567 ++++++++++++---------
 2 files changed, 318 insertions(+), 251 deletions(-)
---
base-commit: 2dcdfb625c3b8fe87454e19dfbc54b3e3f0ad70e
change-id: 20260917-upstream-it6625-follow-up-patch-b81b34266c43

Best regards,
-- 
Hermes Wu <Hermes.wu@ite.com.tw>



^ permalink raw reply	[flat|nested] 23+ messages in thread
end of thread, other threads:[~2026-09-30 15:32 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-30  6:41 [PATCH v2 00/21] media: i2c: it6625: address review feedback and adopt subdev state Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 01/21] dt-bindings: media: ite,it6625: document the default CSI-2 bus type Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 02/21] media: i2c: it6625: propagate initial-setup and control-update errors Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 03/21] media: i2c: it6625: default the debug module parameter to 0 Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 04/21] media: i2c: it6625: drop unused bus field from struct it6625 Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 05/21] media: i2c: it6625: drop stale GCC < 4.4.6 workaround Hermes Wu via B4 Relay
2026-09-30 15:31   ` Hans Verkuil
2026-09-30  6:41 ` [PATCH v2 06/21] media: i2c: it6625: use unsigned int loop indices in table lookups Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 07/21] media: i2c: it6625: drop redundant parentheses in status helpers Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 08/21] media: i2c: it6625: make the audio sampling-rate table static const Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 09/21] media: i2c: it6625: tidy CEC buffer init and a continuation line Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 10/21] media: i2c: it6625: clean up it6625_wait_for_status() Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 11/21] media: i2c: it6625: use unsigned int indices in EDID read/write Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 12/21] media: i2c: it6625: use unaligned/units helpers to decode pixel clock Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 13/21] media: i2c: it6625: decode detected timings via typed register structs Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 14/21] media: i2c: it6625: fix link-frequency reporting for one-/two-trio C-PHY Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 15/21] media: i2c: it6625: use early returns in it6625_update_timings_if_changed() Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 16/21] media: i2c: it6625: drop the private CSI-format name table Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 17/21] media: i2c: it6625: require a DT endpoint and simplify endpoint parsing Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 18/21] media: i2c: it6625: finish reverse fir-tree declaration order Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 19/21] media: i2c: it6625: fold subdev initialization into probe Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 20/21] media: i2c: it6625: use centrally managed active state Hermes Wu via B4 Relay
2026-09-30  6:41 ` [PATCH v2 21/21] media: i2c: it6625: use enable_streams and disable_streams Hermes Wu via B4 Relay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®