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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <bmvv2cc75baovi5hxfyuubl332btxxq7h6w447dwngzmizykoq@ly6lonub3qxy>
Date: Mon, 19 May 2025 11:45:49 +0200
From: Mehdi Djait <mehdi.djait@...ux.intel.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.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 Sakari,

On Thu, May 15, 2025 at 11:50:19PM +0300, Sakari Ailus wrote:
> Hi Mehdi, Laurent,
> 
> On Thu, May 15, 2025 at 03:51:33PM +0200, Mehdi Djait wrote:
> > Hi Laurent,
> > 
> > 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.

Ack.

> 
> Could we document how this is supposed to work on DT and ACPI?

I can include a documentation patch in the next version.

> 
> 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.
> 

makes sense.

> > 
> > 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.
> 

Ack.

--
Kind Regards
Mehdi Djait