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

[Jacopo Mondi]

Hello

On Mon, Dec 01, 2025 at 03:02:11PM +0200, Sakari Ailus wrote:
> Hi Hans, Xiaolei,
>
> On Mon, Dec 01, 2025 at 10:31:59AM +0100, johannes.goede@xxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxx/
> >
> > 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.

ov5647 is the rpi camera module v1, so I guess it's still around even
if a bit old. Dave in cc is the expert and maintainer of this driver.

Jai has a series in review to upstream all the remaining BSP patches
for this driver.
https://lore.kernel.org/all/20251118-b4-rpi-ov5647-v2-0-5e78e7cb7f9b@xxxxxxxxxxxxxxxx/

I'll cc him as well

>
> --
> Regards,
>
> Sakari Ailus
>