From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yw1-f179.google.com (mail-yw1-f179.google.com [209.85.128.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DE520333422 for ; Mon, 1 Dec 2025 16:07:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764605230; cv=none; b=qViXdvEKUoGMn7nfYp5d6S7ZIh//YnTSf8idbxbh8Izd8u1V8C/iCHRyfGOgkzAvbtK9rBdANdSnJwtVnQoqD7sRcReqwhZewkDuDUbCp0+oH99DqrsJgWHwji0O1FfMiXopudC8z6FnXCi7X6Zhl/0N4xPBe9SL1Yj0I00rc7g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764605230; c=relaxed/simple; bh=c0s1Bpv9SIJtLgO+MFj7pYgKnWbXZmC//uAC5eGqd1A=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=YShHW66BEd3lMLW0PFwBr0wUFt6IjLG0whsD99Qc767aEbmyxOb0xx3Zvr7+v+2662PGwr+6AML92BV7wsk/c4msDxBY+4hEUV4WAOW49RDL1evXuq1C/k7tvv9gVpYEBbbphR+w08v5vHn/vY8erMvaQmsmkKlvYTzByM2Li9A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=raspberrypi.com; spf=pass smtp.mailfrom=raspberrypi.com; dkim=pass (2048-bit key) header.d=raspberrypi.com header.i=@raspberrypi.com header.b=GNABcPxj; arc=none smtp.client-ip=209.85.128.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=raspberrypi.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=raspberrypi.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="GNABcPxj" Received: by mail-yw1-f179.google.com with SMTP id 00721157ae682-78802ac22abso44383747b3.3 for ; Mon, 01 Dec 2025 08:07:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1764605226; x=1765210026; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=e+GUj4l2rAJUYnUE8POrzf1i4JPBTbLojun0fOHCQLk=; b=GNABcPxjsX8pKJBHspOZ5Qg9LAmQnT0e3AiYIx8DQrQs5bhFi1pn6+jcgDHbNzbGfQ HUKsMdzYCLD9U2qFvZYv46pvg7Ht4GQg3S6PKdd29L//FC7hSzIxeU7mWbCk6rytMgxD kWp62/qfU1o6MxdpAXQCwbhe8w/EDkv62ELbSs46REffPE+lgmq0QilErzExHTgUtWdC iKdLEyQeCYDtvCFZzAQQ8asec/MQl2vh7qjYe+7lcR4oD1BnLlpjaAakZxaWWHqJ8h9M FYU1xipX6hIsUWmWbD42wMKpKYzkJ+lXxeI2uO+rrLlf0nDTKzCL08+YCo+ecOc4qZIu y58Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764605226; x=1765210026; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=e+GUj4l2rAJUYnUE8POrzf1i4JPBTbLojun0fOHCQLk=; b=T4/ymAYhOc40JmsVXRfNfKWtmn/T6wMDqByqAPCEun/Ocvog8aGaBQjVfHI4Aozfpm t+4aI7kuqT5NW6zKdDBHoUzgbdUQACAX1b5hduoEAXT4ujBTpe74eoCocq0AbK5jxJNJ 34/LFCEscjnEvuqJ549RhW/8qkgP08g3QWpAE5/AMbKEt1rmUu0HWui+FPNfe8UolHiS GNW4Vgab9cg7w7Llu6c3yc41MtJsJsXTZwdHy/GLLOqboDGzFaQ3qT82pxVlxNzzcZPl ZM1gtCXqkJ4DD00yREKhYR+IwFG7YkFupU7o55GnsJtNpkpWB36PbUxt02uJx/YA3Ldp Zl7Q== X-Forwarded-Encrypted: i=1; AJvYcCU4uHDmJzH3qyuUo0kSEWL+/DOOis08gIZniAAgS6RLR4C9HfUY/vkuSXTxm/dCXYixmEzB9Dtqd76TVxs=@vger.kernel.org X-Gm-Message-State: AOJu0YxLzALk6JsaPKMrtglVCjGUiusQTJaOeWQITTVEq8UstOM2VdBP AoiIQqDSXG+IkzCP0H6HfiieuMQo1Vo1nsYb5XGV32u+c1rwJGRkXb7b/Rl8vc57e9O7PWTjdMP 56gV5riHFiAb2dkLDxU/xrbO/QXQMfzKNwUK23p2vmQ== X-Gm-Gg: ASbGncujQbR03hMbXYHAfmt+44A9XPvgKcEs4aEe9Yzvu43SXrs3JmZqOUI3/djYPqh YjkEH2anmtPZKhsGRRUHdbYJCbqgnCvRFjIho3zpwsV5fTfQexsn2Hsav0HtkM1ReCNzHsM6ru8 dQHrHDHPuNolxWrqHmQZ1jWlDqmpyUEhVmlA8yFZwwzSfzPT/XtIkFI7EPUazwSwpHsWhZTuccG iX/gVPnxHOw9/Uz6g4fSZFsds39MQhulHWNnEpa9DD5OEhnZWmGpMfxH0t4xs+P2gDqCKA1TFWz LuBELGxx7NgLgjq33ENrCed7Hkk= X-Google-Smtp-Source: AGHT+IFJLCIBjC3GyN+zfktvsURYsuAMKMhR7lJGonUfr2PybOCbYOMQXw9BWaix5ZQ9kWj4E9hU6vquSCi7u7o3X8E= X-Received: by 2002:a53:b10a:0:b0:640:d303:3523 with SMTP id 956f58d0204a3-6432934817cmr17176653d50.50.1764605226195; Mon, 01 Dec 2025 08:07:06 -0800 (PST) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20251201000026.690298-1-xiaolei.wang@windriver.com> <4553d9ed-ba4e-4f83-b48e-e819e7979293@oss.qualcomm.com> In-Reply-To: From: Dave Stevenson Date: Mon, 1 Dec 2025 16:06:49 +0000 X-Gm-Features: AWmQ_blNg8KbEQdKP9_fgV3XmVwk3gwUAM2iKX1JWAnczZnCzRNWWSohnJ5iWp8 Message-ID: Subject: Re: [PATCH] media: i2c: ov5647: use our own mutex for the ctrl lock To: Jacopo Mondi Cc: Sakari Ailus , johannes.goede@oss.qualcomm.com, Xiaolei Wang , jacopo@jmondi.org, mchehab@kernel.org, prabhakar.mahadev-lad.rj@bp.renesas.com, laurent.pinchart@ideasonboard.com, hverkuil+cisco@kernel.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Jai Luthra Content-Type: text/plain; charset="UTF-8" Hi Sakari On Mon, 1 Dec 2025 at 13:58, Jacopo Mondi wrote: > > 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@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 > > > > > > 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. > > 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. Raspberry Pi stopped selling OV5647 in about 2016 after Omnivision gave a last-time-buy date in 2014/5, and we brought out the v2 camera module based on imx219. However there are still modules being sold even now - stick "OV5647" into eBay or Amazon and you'll get loads of hits. We still support the modules, but have little enthusiasm for investing significant development effort into it whilst it remains functional. As this is a bug fix for a genuine issue and has minimal impact, I'd be tempted to accept it. Reworking the driver to use the same mutex and all the subdev state can be done at a separate time (unless Xiaolei is really keen to do it now). Dave > 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@ideasonboard.com/ > > I'll cc him as well > > > > > -- > > Regards, > > > > Sakari Ailus > >