RE: [PATCH 20/21] media: i2c: it6625: use centrally managed active state

[Hermes.Wu]

Hi Sakari,

Thank you for the clarification.

> Is this AI-generated?

I used AI assistance to help understand this review comment and draft parts
of patches 20/21 because I was not certain about the expected locking model.
I reviewed the result before sending it, but my conclusion about the state
lock was incorrect. I should have asked for clarification instead of making
the claim about what most drivers do. Sorry about that.

> Please use the control handler's lock as the state lock, too. This is
> what most modern drivers do, too.

Do you mean that I should leave the control handler using its own internal
lock and assign that lock to the subdev state before calling
v4l2_subdev_init_finalize(), as follows?

	sd->state_lock = sd->ctrl_handler->lock;

I am also still unsure about the separate it6625_lock. The chip's MCU
processes one register-driven command at a time, and operations such as
issuing a command followed by polling for completion must not be
interleaved. This lock also serializes CEC, IRQ/polling, EDID and debugfs
paths that do not run under the control/state lock.

Should it6625_lock therefore remain as a separate MCU transaction lock, with
the control/state lock acquired first whenever both locks are needed?

Regards,
Hermes