Re: [PATCH] media: i2c: ov5647: use our own mutex for the ctrl lock

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Hans, Xiaolei,

On Mon, Dec 01, 2025 at 10:31:59AM +0100, johannes.goede@oss.qualcomm.com wrote:
> Hi,
> 
> On 1-Dec-25 1:00 AM, Xiaolei Wang wrote:
> > __v4l2_ctrl_handler_setup() and __v4l2_ctrl_modify_range()
> > contains an assertion to verify that the v4l2_ctrl_handler::lock
> > is held, as it should only be called when the lock has already
> > been acquired. Therefore use our own mutex for the ctrl lock,
> > otherwise a warning will be  reported.
> > 
> > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> 
> Generally speaking as a default locking setup for sensor
> drivers we are moving in the direction of removing driver
> specific locks and instead using the control-handler
> lock everywhere, including using it as the active state
> lock, see e.g. :
> 
> https://lore.kernel.org/linux-media/20250313184314.91410-14-hdegoede@redhat.com/
> 
> which sets ov02c10->sd.state_lock = ov02c10->ctrl_handler.lock
> and then removes a bunch of manual mutex_lock / unlock calls
> since all ops which get called with a sd_state will already
> have the lock called when operating on the active_state
> (and when called in try mode they should not touch anything
> needing locking).
> 
> Note if you also want to make the ctrl_handler lock
> the active state lock then you need to add calls to
> v4l2_subdev_init_finalize() / v4l2_subdev_cleanup()
> to allocate the active-state to probe().

I agree with the above, but the driver is old and it uses its own lock to
serialise access to its data structures while it uses the control lock
separately. So this looks like a bugfix that could be backported.

I wonder if anyone still has a system with this sensor.

-- 
Regards,

Sakari Ailus