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
| ||
|
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
Powered by blists - more mailing lists