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: <dhbbpogydqclblzpd2qn2tr2cyyh5gq5cgostzbiq6ygsixj46@oopsp75svt4c>
Date: Wed, 14 May 2025 10:25:36 +0200
From: Mehdi Djait <mehdi.djait@...ux.intel.com>
To: Hans de Goede <hdegoede@...hat.com>, 
	Daniel Scally <dan.scally@...asonboard.com>
Cc: sakari.ailus@...ux.intel.com, laurent.pinchart@...asonboard.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

Hello Hans,

thank you for the comment.

On Sat, May 10, 2025 at 04:21:09PM +0200, Hans de Goede wrote:
> Hi Mehdi,
> 
> 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:
> 

is it even possible to get a reference to the clock producer in ACPI
systems or am I missing something here ?

Here is what I gathered online for the discussion leading to this patch:
----------------------------------------------------------------------------------------------------------------------------------------------
ClockInput resource added to ACPI v6.5: https://uefi.org/specs/ACPI/6.5/19_ASL_Reference.html#clockinput-clock-input-resource-descriptor-macro
- commit adding ClockInput resource to acpica: https://github.com/acpica/acpica/commit/661feab5ee01a34af95a389a18c82e79f1aba05a
- commit kernel upstream: 520d4a0ee5b6d9c7a1258ace6caa13a94ac35ef8 "ACPICA: add support
  for ClockInput resource (v6.5)"

this does not mean we can use it: I found this out-of-tree patch to supports fixed clock sources
https://github.com/niyas-sait/linux-acpi/blob/main/0001-acpi-add-clock-bindings-for-fixed-clock-resources.patch
it was not sent to the acpi mailing list. It was mentioned in this
dicussion: https://lore.kernel.org/linux-kernel/78763d69bae04204b2af37201b09f8b5@huawei.com/

Another interesting link: https://linaro.atlassian.net/wiki/spaces/CLIENTPC/pages/28822175758/ACPI+Clock+Input+Resources
----------------------------------------------------------------------------------------------------------------------------------------------

link for the dicussion: https://lore.kernel.org/linux-media/20250220154909.152538-1-mehdi.djait@linux.intel.com/

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

So if I understood the above correctly: in the ov8865 IPU3/ACPI case:

1) sensor->extclk is NULL because the clock producer is not available in
ACPI systems. ClockInput() ACPI resource was introduced to acpica after
the ov8865 patch and the resource is not even being used in the upstream kernel.

2) the sensor->extclk_rate will be set from reading
the clock-frequency _DSD property in:

	ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", &rate);

> 
> IMHO it would be good if the generic helper would handle
> this case too and if there is both a clk-provider and
> a clock-frequency property then try to set the clock-frequency
> value with clk_set_rate(), arguably you could just warn on
> a failure to set the rate though, instead of the error
> the ov8865 driver currently throws.
> 
> Sakari, Laurent any opinions on adding handling this case
> to the generic helper ?
> 
> Regards,
> 
> Hans

--
Kind Regards
Mehdi Djait