linux-kernel - Re: [RFC PATCH v4] media: v4l2-common: Add a helper for obtaining the clock producer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aC2wzFD_bPgESZpR@kekkonen.localdomain>
Date: Wed, 21 May 2025 10:54:04 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Mehdi Djait <mehdi.djait@...ux.intel.com>,
	Hans de Goede <hdegoede@...hat.com>,
	tomi.valkeinen@...asonboard.com, jacopo.mondi@...asonboard.com,
	hverkuil@...all.nl, kieran.bingham@...asonboard.com,
	naush@...pberrypi.com, mchehab@...nel.org,
	dave.stevenson@...pberrypi.com, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v4] media: v4l2-common: Add a helper for obtaining
 the clock producer

Hi Laurent,

On Wed, May 21, 2025 at 12:51:41PM +0200, Laurent Pinchart wrote:
> On Thu, May 15, 2025 at 11:50:19PM +0300, Sakari Ailus wrote:
> > On Thu, May 15, 2025 at 03:51:33PM +0200, Mehdi Djait wrote:
> > > On Thu, May 15, 2025 at 02:40:50PM +0200, Laurent Pinchart wrote:
> > > > On Thu, May 15, 2025 at 11:17:37AM +0200, Mehdi Djait wrote:
> > > > > On Thu, May 15, 2025 at 10:44:03AM +0200, Laurent Pinchart wrote:
> > > > > > On Sat, May 10, 2025 at 04:21:09PM +0200, Hans de Goede wrote:
> > > > > > > On 21-Mar-25 2:03 PM, Mehdi Djait wrote:
> > > > > > > > Introduce a helper for v4l2 sensor drivers on both DT- and ACPI-based
> > > > > > > > platforms to retrieve a reference to the clock producer from firmware.
> > > > > > > > 
> > > > > > > > This helper behaves the same as clk_get_optional() except where there is
> > > > > > > > no clock producer like in ACPI-based platforms.
> > > > > > > > 
> > > > > > > > For ACPI-based platforms the function will read the "clock-frequency"
> > > > > > > > ACPI _DSD property and register a fixed frequency clock with the frequency
> > > > > > > > indicated in the property.
> > > > > > > > 
> > > > > > > > Signed-off-by: Mehdi Djait <mehdi.djait@...ux.intel.com>
> > > > > > > 
> > > > > > > This certainly looks quite useful, thank you for working
> > > > > > > on this.
> > > > > > > 
> > > > > > > Note on some IPU3 platforms where the clk is provided by
> > > > > > > a clk-generator which is part of a special sensor-PMIC
> > > > > > > the situation is a bit more complicated.
> > > > > > > 
> > > > > > > Basically if there is both a clk provider and a clock-frequency
> > > > > > > property then the clock-frequency value should be set as freq
> > > > > > > to the clk-provider, see:
> > > > > > > 
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/ov8865.c#n3020
> > > > > > > 
> > > > > > > for an example of a driver which handles this case.
> > > > > > 
> > > > > > On a side note, the DT bindings for the OV8865 doesn't specify the
> > > > > > clock-frequency property...
> > > > > 
> > > > > Is this wrong ?
> > > > > 
> > > > > The OV8865 driver was introduced for DT-based systems, where you will
> > > > > get a reference to the "struct clk corresponding to the clock producer"
> > > > > and then get the clock-rate/frequency with a call to:
> > > > > 
> > > > > 	rate = clk_get_rate(sensor->extclk);
> > > > > 
> > > > > The patch "73dcffeb2ff9 media: i2c: Support 19.2MHz input clock in ov8865"
> > > > > adding support for clock-frequency came later to support ACPI-based
> > > > > systems (IPU3 here)
> > > > 
> > > > I'd expect all device properties to be documented in DT bindings. Is
> > > > that an incorrect assumption ?
> > > > 
> > > 
> > > I am actually genuinely asking, is the clock-frequency a device property
> > > of the ov8865 camera sensor or the clock source, which is a separate device ?
> > 
> > The sensor's.
> >
> > Could we document how this is supposed to work on DT and ACPI?
> 
> Yes please. Would you like to send a patch ? :-)

I'd add this to the helper's documentation. We'll work this out with Mehdi.

> 
> > I think we should also select COMMON_CLK on ACPI systems for sensor
> > drivers (in a separate patch maybe?), instead of relying on distributions
> > enabling it.
> > 
> > > Example the imx258 with a fixed-clock, which has its own compatible
> > > and DT bindings under bindings/clock/fixed-clock.yaml
> > > 
> > > So when adding support for ACPI-based systems, the DT bindings should
> > > not be changed because getting the clock-frequency from the ACPI _DSD
> > > property is a workaround only needed on ACPI-based systems.
> > 
> > I wouldn't say it's a workaround, but something that's only needed on ACPI
> > systems.
> 
> Does that mean that the clock-frequency property should be deprecated on
> DT-based systems, and not used in any new sensor bindings ?

I don't think we've added any "clock-frequency" properties in DT bindings
for camera sensors since around 2020 or so.

-- 
Regards,

Sakari Ailus