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: <20250619212630.GQ22102@pendragon.ideasonboard.com>
Date: Fri, 20 Jun 2025 00:26:30 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Mehdi Djait <mehdi.djait@...ux.intel.com>
Cc: sakari.ailus@...ux.intel.com, akinobu.mita@...il.com,
	stanislaw.gruszka@...ux.intel.com, hdegoede@...hat.com,
	arnd@...db.de, alain.volmat@...s.st.com, andrzej.hajda@...el.com,
	benjamin.mugnier@...s.st.com, dave.stevenson@...pberrypi.com,
	hansg@...nel.org, hverkuil@...all.nl, jacopo.mondi@...asonboard.com,
	jonas@...boo.se, kieran.bingham@...asonboard.com, khalasa@...p.pl,
	prabhakar.csengg@...il.com, mani@...nel.org,
	m.felsch@...gutronix.de, martink@...teo.de, mattwmajewski@...il.com,
	matthias.fend@...end.at, mchehab@...nel.org,
	michael.riesch@...labora.com, naush@...pberrypi.com,
	nicholas@...hemail.net, nicolas.dufresne@...labora.com,
	paul.elder@...asonboard.com, dan.scally@...asonboard.com,
	pavel@...nel.org, petrcvekcz@...il.com, rashanmu@...il.com,
	ribalda@...omium.org, rmfrfs@...il.com, zhengsq@...k-chips.com,
	slongerbeam@...il.com, sylvain.petinot@...s.st.com,
	s.nawrocki@...sung.com, tomi.valkeinen@...asonboard.com,
	umang.jain@...asonboard.com, zhi.mao@...iatek.com,
	linux-kernel@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH v1 00/55] media: Add a helper for obtaining the clock
 producer

Hi Mehdi,

Thank you for the patch.

On Thu, Jun 19, 2025 at 07:58:53PM +0200, Mehdi Djait wrote:
> Hello everyone,
> 
> Here is my v1 for the new helper v4l2_devm_sensor_clk_get()
> 
> Any testing of the patches is GREATLY APPRECIATED! Especially the two
> drivers with the special ACPI case:
> 1) OV8865
> 2) OV2680
> 
> 
> Background
> ----------
> 
> A reference to the clock producer is not available to the kernel
> in ACPI-based platforms but the sensor drivers still need them.
> 
> devm_clk_get() will return an error and the probe function will fail.
> 
> 
> Solution
> --------
> 
> Introduce a generic helper for v4l2 sensor drivers on both DT- and ACPI-based
> platforms.
> 
> 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.
> 
> 
> Solution for special ACPI case
> ------------------------------
> 
> This function also handles the special ACPI-based system case where:
> 
> 1) The clock-frequency _DSD property is present.
> 2) A reference to the clock producer is present, where the clock is provided
> by a camera sensor PMIC driver (e.g. int3472/tps68470.c)
> 
> In this case try to set the clock-frequency value to the provided clock.
> 
> 
> RFC History
> -----------
> 
> RFC v4 -> RFC v5:
> Suggested by Arnd Bergmann:
> 	- removed IS_REACHABLE(CONFIG_COMMON_CLK). IS_REACHABLE() is actually
> 	discouraged [1]. COFIG_COMMON_CLK is a bool, so IS_ENABLED() will be the
> 	right solution here
> Suggested by Hans de Goede:
> 	- added handling for the special ACPI-based system case, where
> 	  both a reference to the clock-provider and the _DSD
> 	  clock-frequency are present.
> 	- updated the function's kernel-doc and the commit msg
> 	  to mention this special case.
> Link RFC v4: https://lore.kernel.org/linux-media/20250321130329.342236-1-mehdi.djait@linux.intel.com/
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/Documentation/kbuild/kconfig-language.rst?h=next-20250513&id=700bd25bd4f47a0f4e02e0a25dde05f1a6b16eea
> 
> RFC v3 -> RFC v4:
> Suggested by Laurent:
> 	- removed the #ifdef to use IS_REACHABLE(CONFIG_COMMON_CLK)
> 	- changed to kasprintf() to allocate the clk name when id is NULL and
> 	  used the __free(kfree) scope-based cleanup helper when
> 	  defining the variable to hold the allocated name
> Link v3: https://lore.kernel.org/linux-media/20250321093814.18159-1-mehdi.djait@linux.intel.com/
> 
> RFC v2 -> RFC v3:
> - Added #ifdef CONFIG_COMMON_CLK for the ACPI case
> Link v2: https://lore.kernel.org/linux-media/20250310122305.209534-1-mehdi.djait@linux.intel.com/
> 
> RFC v1 -> RFC v2:
> Suggested by Sakari:
>     - removed clk_name
>     - removed the IS_ERR() check
>     - improved the kernel-doc comment and commit msg
> Link v1: https://lore.kernel.org/linux-media/20250227092643.113939-1-mehdi.djait@linux.intel.com
> 
> Mehdi Djait (55):
>   media: v4l2-common: Add a helper for obtaining the clock producer
>   Documentation: media: camera-sensor: Mention
>     v4l2_devm_sensor_clk_get() for obtaining the clock
>   media: i2c: ar0521: Use the v4l2 helper for obtaining the clock
>   media: i2c: ds90ub913: Use the v4l2 helper for obtaining the clock
>   media: i2c: ds90ub960: Use the v4l2 helper for obtaining the clock

Those two are FPD-Link serializer and deserializer drivers. I would
leave them out, and only address camera sensors in this series, as ACPI
integration for serdes or other kind of chips needs to be discussed
separately.

>   media: i2c: et8ek8: Use the v4l2 helper for obtaining the clock
>   media: i2c: gc05a2: Use the v4l2 helper for obtaining the clock
>   media: i2c: gc08a3: Use the v4l2 helper for obtaining the clock
>   media: i2c: gc2145: Use the v4l2 helper for obtaining the clock
>   media: i2c: hi846: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx214: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx219: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx283: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx290: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx296: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx334: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx335: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx412: Use the v4l2 helper for obtaining the clock
>   media: i2c: imx415: Use the v4l2 helper for obtaining the clock
>   media: i2c: max2175: Use the v4l2 helper for obtaining the clock

This is a tuner, I would leave it out too.

>   media: i2c: mt9m001: Use the v4l2 helper for obtaining the clock
>   media: i2c: mt9m111: Use the v4l2 helper for obtaining the clock
>   media: i2c: mt9m114: Use the v4l2 helper for obtaining the clock
>   media: i2c: mt9p031: Use the v4l2 helper for obtaining the clock
>   media: i2c: mt9t112: Use the v4l2 helper for obtaining the clock
>   media: i2c: mt9v032: Use the v4l2 helper for obtaining the clock
>   media: i2c: mt9v111: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov02a10: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov2659: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov2685: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov5640: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov5645: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov5647: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov5648: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov5695: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov64a40: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov6650: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov7740: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov8856: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov8858: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov8865: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov9282: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov9640: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov9650: Use the v4l2 helper for obtaining the clock
>   media: i2c: s5c73m3: Use the v4l2 helper for obtaining the clock
>   media: i2c: s5k5baf: Use the v4l2 helper for obtaining the clock
>   media: i2c: s5k6a3: Use the v4l2 helper for obtaining the clock
>   media: i2c: st-mipid02: Use the v4l2 helper for obtaining the clock
>   media: i2c: tc358743: Use the v4l2 helper for obtaining the clock
>   media: i2c: tc358746: Use the v4l2 helper for obtaining the clock

Those three drivers are for a CSI-2 to parallel or HDMI to CSI-2
bridges, I would leave them out too.

>   media: i2c: thp7312: Use the v4l2 helper for obtaining the clock

And this is an external ISP, that I would also leave out for now.

>   media: i2c: vd55g1: Use the v4l2 helper for obtaining the clock
>   media: i2c: vd56g3: Use the v4l2 helper for obtaining the clock
>   media: i2c: vgxy61: Use the v4l2 helper for obtaining the clock
>   media: i2c: ov2680: Use the v4l2 helper for obtaining the clock

All the rest should be sensors, but please double-check.

For the sensor driver patches *except* ov8865 and ov2680,

Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>

> 
>  .../driver-api/media/camera-sensor.rst        |  3 +-
>  drivers/media/i2c/ar0521.c                    |  2 +-
>  drivers/media/i2c/ds90ub913.c                 |  2 +-
>  drivers/media/i2c/ds90ub960.c                 |  2 +-
>  drivers/media/i2c/et8ek8/et8ek8_driver.c      |  2 +-
>  drivers/media/i2c/gc05a2.c                    |  2 +-
>  drivers/media/i2c/gc08a3.c                    |  2 +-
>  drivers/media/i2c/gc2145.c                    |  2 +-
>  drivers/media/i2c/hi846.c                     |  2 +-
>  drivers/media/i2c/imx214.c                    |  2 +-
>  drivers/media/i2c/imx219.c                    |  2 +-
>  drivers/media/i2c/imx283.c                    |  2 +-
>  drivers/media/i2c/imx290.c                    |  2 +-
>  drivers/media/i2c/imx296.c                    |  2 +-
>  drivers/media/i2c/imx334.c                    |  2 +-
>  drivers/media/i2c/imx335.c                    |  2 +-
>  drivers/media/i2c/imx412.c                    |  2 +-
>  drivers/media/i2c/imx415.c                    |  2 +-
>  drivers/media/i2c/max2175.c                   |  2 +-
>  drivers/media/i2c/mt9m001.c                   |  2 +-
>  drivers/media/i2c/mt9m111.c                   |  2 +-
>  drivers/media/i2c/mt9m114.c                   |  2 +-
>  drivers/media/i2c/mt9p031.c                   |  2 +-
>  drivers/media/i2c/mt9t112.c                   |  2 +-
>  drivers/media/i2c/mt9v032.c                   |  2 +-
>  drivers/media/i2c/mt9v111.c                   |  2 +-
>  drivers/media/i2c/ov02a10.c                   |  2 +-
>  drivers/media/i2c/ov2659.c                    |  2 +-
>  drivers/media/i2c/ov2680.c                    | 27 +++-------
>  drivers/media/i2c/ov2685.c                    |  2 +-
>  drivers/media/i2c/ov5640.c                    |  2 +-
>  drivers/media/i2c/ov5645.c                    |  2 +-
>  drivers/media/i2c/ov5647.c                    |  2 +-
>  drivers/media/i2c/ov5648.c                    |  2 +-
>  drivers/media/i2c/ov5695.c                    |  2 +-
>  drivers/media/i2c/ov64a40.c                   |  2 +-
>  drivers/media/i2c/ov6650.c                    |  2 +-
>  drivers/media/i2c/ov7740.c                    |  2 +-
>  drivers/media/i2c/ov8856.c                    |  2 +-
>  drivers/media/i2c/ov8858.c                    |  2 +-
>  drivers/media/i2c/ov8865.c                    | 32 ++----------
>  drivers/media/i2c/ov9282.c                    |  2 +-
>  drivers/media/i2c/ov9640.c                    |  2 +-
>  drivers/media/i2c/ov9650.c                    |  2 +-
>  drivers/media/i2c/s5c73m3/s5c73m3-core.c      |  2 +-
>  drivers/media/i2c/s5k5baf.c                   |  2 +-
>  drivers/media/i2c/s5k6a3.c                    |  2 +-
>  drivers/media/i2c/st-mipid02.c                |  2 +-
>  drivers/media/i2c/tc358743.c                  |  2 +-
>  drivers/media/i2c/tc358746.c                  |  2 +-
>  drivers/media/i2c/thp7312.c                   |  2 +-
>  drivers/media/i2c/vd55g1.c                    |  2 +-
>  drivers/media/i2c/vd56g3.c                    |  2 +-
>  drivers/media/i2c/vgxy61.c                    |  2 +-
>  drivers/media/v4l2-core/v4l2-common.c         | 49 +++++++++++++++++++
>  include/media/v4l2-common.h                   | 25 ++++++++++
>  56 files changed, 136 insertions(+), 102 deletions(-)

-- 
Regards,

Laurent Pinchart

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ