* [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
@ 2025-03-10 12:23 Mehdi Djait
2025-03-11 14:51 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Mehdi Djait @ 2025-03-10 12:23 UTC (permalink / raw)
To: sakari.ailus, laurent.pinchart
Cc: tomi.valkeinen, jacopo.mondi, hverkuil, kieran.bingham, naush,
mchehab, hdegoede, dave.stevenson, linux-media, linux-kernel,
Mehdi Djait
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 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@linux.intel.com>
---
Link for discussion (where this patch was proposed): https://lore.kernel.org/linux-media/20250220154909.152538-1-mehdi.djait@linux.intel.com/
v1 -> v2:
Suggested by Sakari:
- removed clk_name
- removed the IS_ERR() check
- improved the kernel-doc comment and commit msg
Link for v1: https://lore.kernel.org/linux-media/20250227092643.113939-1-mehdi.djait@linux.intel.com
drivers/media/v4l2-core/v4l2-common.c | 35 +++++++++++++++++++++++++++
include/media/v4l2-common.h | 18 ++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 0a2f4f0d0a07..99d826acb213 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -34,6 +34,9 @@
* Added Gerd Knorrs v4l1 enhancements (Justin Schoeman)
*/
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/kernel.h>
@@ -636,3 +639,35 @@ int v4l2_link_freq_to_bitmap(struct device *dev, const u64 *fw_link_freqs,
return 0;
}
EXPORT_SYMBOL_GPL(v4l2_link_freq_to_bitmap);
+
+struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id)
+{
+ struct clk_hw *clk_hw;
+ struct clk *clk;
+ u32 rate;
+ int ret;
+
+ clk = devm_clk_get_optional(dev, id);
+ if (clk)
+ return clk;
+
+ if (!is_acpi_node(dev_fwnode(dev)))
+ return ERR_PTR(-ENOENT);
+
+ ret = device_property_read_u32(dev, "clock-frequency", &rate);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (!id) {
+ id = devm_kasprintf(dev, GFP_KERNEL, "clk-%s", dev_name(dev));
+ if (!id)
+ return ERR_PTR(-ENOMEM);
+ }
+
+ clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
+ if (IS_ERR(clk_hw))
+ return ERR_CAST(clk_hw);
+
+ return clk_hw->clk;
+}
+EXPORT_SYMBOL_GPL(devm_v4l2_sensor_clk_get);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 63ad36f04f72..35b9ac698e8a 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -573,6 +573,24 @@ int v4l2_link_freq_to_bitmap(struct device *dev, const u64 *fw_link_freqs,
unsigned int num_of_driver_link_freqs,
unsigned long *bitmap);
+/**
+ * devm_v4l2_sensor_clk_get - lookup and obtain a reference to an optional clock
+ * producer for a camera sensor.
+ *
+ * @dev: device for v4l2 sensor clock "consumer"
+ * @id: clock consumer ID
+ *
+ * This function behaves the same way 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-clock with the frequency indicated
+ * in the property.
+ *
+ * Return:
+ * * pointer to a struct clk on success or an error code on failure.
+ */
+struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id);
+
static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf)
{
/*
--
2.48.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
2025-03-10 12:23 [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer Mehdi Djait
@ 2025-03-11 14:51 ` kernel test robot
2025-03-11 16:46 ` kernel test robot
2025-05-10 12:56 ` Sakari Ailus
2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-03-11 14:51 UTC (permalink / raw)
To: Mehdi Djait; +Cc: oe-kbuild-all
Hi Mehdi,
[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on sailus-media-tree/master v6.14-rc6 next-20250311]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mehdi-Djait/media-v4l2-common-Add-a-helper-for-obtaining-the-clock-producer/20250310-202543
base: linus/master
patch link: https://lore.kernel.org/r/20250310122305.209534-1-mehdi.djait%40linux.intel.com
patch subject: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
config: i386-randconfig-054-20250311 (https://download.01.org/0day-ci/archive/20250311/202503112248.4yP7WmdR-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250311/202503112248.4yP7WmdR-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503112248.4yP7WmdR-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: drivers/media/v4l2-core/v4l2-common.o: in function `devm_v4l2_sensor_clk_get':
>> drivers/media/v4l2-core/v4l2-common.c:667: undefined reference to `__clk_hw_register_fixed_rate'
vim +667 drivers/media/v4l2-core/v4l2-common.c
642
643 struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id)
644 {
645 struct clk_hw *clk_hw;
646 struct clk *clk;
647 u32 rate;
648 int ret;
649
650 clk = devm_clk_get_optional(dev, id);
651 if (clk)
652 return clk;
653
654 if (!is_acpi_node(dev_fwnode(dev)))
655 return ERR_PTR(-ENOENT);
656
657 ret = device_property_read_u32(dev, "clock-frequency", &rate);
658 if (ret)
659 return ERR_PTR(ret);
660
661 if (!id) {
662 id = devm_kasprintf(dev, GFP_KERNEL, "clk-%s", dev_name(dev));
663 if (!id)
664 return ERR_PTR(-ENOMEM);
665 }
666
> 667 clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
2025-03-10 12:23 [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer Mehdi Djait
2025-03-11 14:51 ` kernel test robot
@ 2025-03-11 16:46 ` kernel test robot
2025-05-10 12:56 ` Sakari Ailus
2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-03-11 16:46 UTC (permalink / raw)
To: Mehdi Djait; +Cc: oe-kbuild-all
Hi Mehdi,
[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on sailus-media-tree/master v6.14-rc6 next-20250311]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mehdi-Djait/media-v4l2-common-Add-a-helper-for-obtaining-the-clock-producer/20250310-202543
base: linus/master
patch link: https://lore.kernel.org/r/20250310122305.209534-1-mehdi.djait%40linux.intel.com
patch subject: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
config: i386-randconfig-053-20250311 (https://download.01.org/0day-ci/archive/20250312/202503120028.JHoKfjCI-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250312/202503120028.JHoKfjCI-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503120028.JHoKfjCI-lkp@intel.com/
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: __clk_hw_register_fixed_rate
>>> referenced by v4l2-common.c:667 (drivers/media/v4l2-core/v4l2-common.c:667)
>>> drivers/media/v4l2-core/v4l2-common.o:(devm_v4l2_sensor_clk_get) in archive vmlinux.a
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
2025-03-10 12:23 [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer Mehdi Djait
2025-03-11 14:51 ` kernel test robot
2025-03-11 16:46 ` kernel test robot
@ 2025-05-10 12:56 ` Sakari Ailus
2025-05-12 8:21 ` Mehdi Djait
2 siblings, 1 reply; 7+ messages in thread
From: Sakari Ailus @ 2025-05-10 12:56 UTC (permalink / raw)
To: Mehdi Djait
Cc: laurent.pinchart, tomi.valkeinen, jacopo.mondi, hverkuil,
kieran.bingham, naush, mchehab, hdegoede, dave.stevenson,
linux-media, linux-kernel
Hi Mehdi,
On Mon, Mar 10, 2025 at 01:23:05PM +0100, 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 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@linux.intel.com>
> ---
> Link for discussion (where this patch was proposed): https://lore.kernel.org/linux-media/20250220154909.152538-1-mehdi.djait@linux.intel.com/
>
> v1 -> v2:
> Suggested by Sakari:
> - removed clk_name
> - removed the IS_ERR() check
> - improved the kernel-doc comment and commit msg
> Link for v1: https://lore.kernel.org/linux-media/20250227092643.113939-1-mehdi.djait@linux.intel.com
>
> drivers/media/v4l2-core/v4l2-common.c | 35 +++++++++++++++++++++++++++
> include/media/v4l2-common.h | 18 ++++++++++++++
> 2 files changed, 53 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 0a2f4f0d0a07..99d826acb213 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -34,6 +34,9 @@
> * Added Gerd Knorrs v4l1 enhancements (Justin Schoeman)
> */
>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> #include <linux/module.h>
> #include <linux/types.h>
> #include <linux/kernel.h>
> @@ -636,3 +639,35 @@ int v4l2_link_freq_to_bitmap(struct device *dev, const u64 *fw_link_freqs,
> return 0;
> }
> EXPORT_SYMBOL_GPL(v4l2_link_freq_to_bitmap);
> +
> +struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id)
> +{
> + struct clk_hw *clk_hw;
> + struct clk *clk;
> + u32 rate;
> + int ret;
> +
> + clk = devm_clk_get_optional(dev, id);
> + if (clk)
> + return clk;
> +
> + if (!is_acpi_node(dev_fwnode(dev)))
> + return ERR_PTR(-ENOENT);
> +
> + ret = device_property_read_u32(dev, "clock-frequency", &rate);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + if (!id) {
> + id = devm_kasprintf(dev, GFP_KERNEL, "clk-%s", dev_name(dev));
> + if (!id)
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
devm_clk_hw_register_fixed_rate() is only available when COMMON_CLK is
enabled. You need #ifdefs here. In practice without CCF only
devm_clk_get_optional() is useful I guess.
Another question is then how commonly COMMON_CLK is enabled e.g. on x86
systems. At least Debian kernel has it. Presumably it's common elsewhere,
too.
> + if (IS_ERR(clk_hw))
> + return ERR_CAST(clk_hw);
> +
> + return clk_hw->clk;
> +}
> +EXPORT_SYMBOL_GPL(devm_v4l2_sensor_clk_get);
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 63ad36f04f72..35b9ac698e8a 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -573,6 +573,24 @@ int v4l2_link_freq_to_bitmap(struct device *dev, const u64 *fw_link_freqs,
> unsigned int num_of_driver_link_freqs,
> unsigned long *bitmap);
>
> +/**
> + * devm_v4l2_sensor_clk_get - lookup and obtain a reference to an optional clock
> + * producer for a camera sensor.
> + *
> + * @dev: device for v4l2 sensor clock "consumer"
> + * @id: clock consumer ID
> + *
> + * This function behaves the same way 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-clock with the frequency indicated
> + * in the property.
> + *
> + * Return:
> + * * pointer to a struct clk on success or an error code on failure.
> + */
> +struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id);
> +
> static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf)
> {
> /*
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
2025-05-10 12:56 ` Sakari Ailus
@ 2025-05-12 8:21 ` Mehdi Djait
2025-05-12 15:41 ` Sakari Ailus
0 siblings, 1 reply; 7+ messages in thread
From: Mehdi Djait @ 2025-05-12 8:21 UTC (permalink / raw)
To: Sakari Ailus
Cc: laurent.pinchart, tomi.valkeinen, jacopo.mondi, hverkuil,
kieran.bingham, naush, mchehab, hdegoede, dave.stevenson,
linux-media, linux-kernel
Hi Sakari,
On Sat, May 10, 2025 at 12:56:02PM +0000, Sakari Ailus wrote:
> Hi Mehdi,
>
> On Mon, Mar 10, 2025 at 01:23:05PM +0100, 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 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@linux.intel.com>
SNIP
> > +struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id)
> > +{
> > + struct clk_hw *clk_hw;
> > + struct clk *clk;
> > + u32 rate;
> > + int ret;
> > +
> > + clk = devm_clk_get_optional(dev, id);
> > + if (clk)
> > + return clk;
> > +
> > + if (!is_acpi_node(dev_fwnode(dev)))
> > + return ERR_PTR(-ENOENT);
> > +
> > + ret = device_property_read_u32(dev, "clock-frequency", &rate);
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + if (!id) {
> > + id = devm_kasprintf(dev, GFP_KERNEL, "clk-%s", dev_name(dev));
> > + if (!id)
> > + return ERR_PTR(-ENOMEM);
> > + }
> > +
> > + clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
>
> devm_clk_hw_register_fixed_rate() is only available when COMMON_CLK is
> enabled. You need #ifdefs here. In practice without CCF only
> devm_clk_get_optional() is useful I guess.
>
I added a call to IS_REACHABLE(CONFIG_COMMON_CLK) in the v4 of this patch:
https://lore.kernel.org/linux-media/20250321130329.342236-1-mehdi.djait@linux.intel.com/
> Another question is then how commonly COMMON_CLK is enabled e.g. on x86
> systems. At least Debian kernel has it. Presumably it's common elsewhere,
> too.
on Arch linux it is also enabled and Fedora also. I would also assume it
is also enabled in the other linux distros.
--
Kind Regards
Mehdi Djait
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
2025-05-12 8:21 ` Mehdi Djait
@ 2025-05-12 15:41 ` Sakari Ailus
2025-05-14 11:43 ` Mehdi Djait
0 siblings, 1 reply; 7+ messages in thread
From: Sakari Ailus @ 2025-05-12 15:41 UTC (permalink / raw)
To: Mehdi Djait
Cc: laurent.pinchart, tomi.valkeinen, jacopo.mondi, hverkuil,
kieran.bingham, naush, mchehab, hdegoede, dave.stevenson,
linux-media, linux-kernel
Hi Mehdi,
On Mon, May 12, 2025 at 10:21:21AM +0200, Mehdi Djait wrote:
> Hi Sakari,
>
> On Sat, May 10, 2025 at 12:56:02PM +0000, Sakari Ailus wrote:
> > Hi Mehdi,
> >
> > On Mon, Mar 10, 2025 at 01:23:05PM +0100, 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 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@linux.intel.com>
>
> SNIP
>
> > > +struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id)
> > > +{
> > > + struct clk_hw *clk_hw;
> > > + struct clk *clk;
> > > + u32 rate;
> > > + int ret;
> > > +
> > > + clk = devm_clk_get_optional(dev, id);
> > > + if (clk)
> > > + return clk;
> > > +
> > > + if (!is_acpi_node(dev_fwnode(dev)))
> > > + return ERR_PTR(-ENOENT);
> > > +
> > > + ret = device_property_read_u32(dev, "clock-frequency", &rate);
> > > + if (ret)
> > > + return ERR_PTR(ret);
> > > +
> > > + if (!id) {
> > > + id = devm_kasprintf(dev, GFP_KERNEL, "clk-%s", dev_name(dev));
> > > + if (!id)
> > > + return ERR_PTR(-ENOMEM);
> > > + }
> > > +
> > > + clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
> >
> > devm_clk_hw_register_fixed_rate() is only available when COMMON_CLK is
> > enabled. You need #ifdefs here. In practice without CCF only
> > devm_clk_get_optional() is useful I guess.
> >
>
> I added a call to IS_REACHABLE(CONFIG_COMMON_CLK) in the v4 of this patch:
> https://lore.kernel.org/linux-media/20250321130329.342236-1-mehdi.djait@linux.intel.com/
I wonder if this approach works. Depending on the compiler implementation,
the compiler could (or even should) still run into issues in finding an
unresolvable symbol, even if the symbol is not reachable and can be
optimised away.
>
> > Another question is then how commonly COMMON_CLK is enabled e.g. on x86
> > systems. At least Debian kernel has it. Presumably it's common elsewhere,
> > too.
>
> on Arch linux it is also enabled and Fedora also. I would also assume it
> is also enabled in the other linux distros.
Ack, thanks for checking.
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer
2025-05-12 15:41 ` Sakari Ailus
@ 2025-05-14 11:43 ` Mehdi Djait
0 siblings, 0 replies; 7+ messages in thread
From: Mehdi Djait @ 2025-05-14 11:43 UTC (permalink / raw)
To: Sakari Ailus
Cc: laurent.pinchart, tomi.valkeinen, jacopo.mondi, hverkuil,
kieran.bingham, naush, mchehab, hdegoede, dave.stevenson,
linux-media, linux-kernel, Arnd Bergmann
Hi Sakari,
On Mon, May 12, 2025 at 06:41:07PM +0300, Sakari Ailus wrote:
> Hi Mehdi,
>
> On Mon, May 12, 2025 at 10:21:21AM +0200, Mehdi Djait wrote:
> > Hi Sakari,
> >
> > On Sat, May 10, 2025 at 12:56:02PM +0000, Sakari Ailus wrote:
> > > Hi Mehdi,
> > >
> > > On Mon, Mar 10, 2025 at 01:23:05PM +0100, 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 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@linux.intel.com>
> >
> > SNIP
> >
> > > > +struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id)
> > > > +{
> > > > + struct clk_hw *clk_hw;
> > > > + struct clk *clk;
> > > > + u32 rate;
> > > > + int ret;
> > > > +
> > > > + clk = devm_clk_get_optional(dev, id);
> > > > + if (clk)
> > > > + return clk;
> > > > +
> > > > + if (!is_acpi_node(dev_fwnode(dev)))
> > > > + return ERR_PTR(-ENOENT);
> > > > +
> > > > + ret = device_property_read_u32(dev, "clock-frequency", &rate);
> > > > + if (ret)
> > > > + return ERR_PTR(ret);
> > > > +
> > > > + if (!id) {
> > > > + id = devm_kasprintf(dev, GFP_KERNEL, "clk-%s", dev_name(dev));
> > > > + if (!id)
> > > > + return ERR_PTR(-ENOMEM);
> > > > + }
> > > > +
> > > > + clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
> > >
> > > devm_clk_hw_register_fixed_rate() is only available when COMMON_CLK is
> > > enabled. You need #ifdefs here. In practice without CCF only
> > > devm_clk_get_optional() is useful I guess.
> > >
> >
> > I added a call to IS_REACHABLE(CONFIG_COMMON_CLK) in the v4 of this patch:
> > https://lore.kernel.org/linux-media/20250321130329.342236-1-mehdi.djait@linux.intel.com/
>
> I wonder if this approach works. Depending on the compiler implementation,
> the compiler could (or even should) still run into issues in finding an
> unresolvable symbol, even if the symbol is not reachable and can be
> optimised away.
>
So I discussed with Arnd about this (He introduced IS_REACHABLE()):
- IS_REACHABLE() is actually discouraged [1]
- COFIG_COMMON_CLK is a bool, so IS_ENABLED() will be the right solution here
- usage of IS_ENABLED() is also encouraged in the coding style [2]
- we will not face compiler issues because the kernel relies on
dead code elimination in the compiler. (Actually I remember this is
one of the reasons why you cannot compile the kernel with optimization
turned off. I don't know how much this argument holds today [3]
[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
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n1178
[3] https://lore.kernel.org/all/20080909160452.GB30160@cs181140183.pp.htv.fi/
> >
> > > Another question is then how commonly COMMON_CLK is enabled e.g. on x86
> > > systems. At least Debian kernel has it. Presumably it's common elsewhere,
> > > too.
> >
> > on Arch linux it is also enabled and Fedora also. I would also assume it
> > is also enabled in the other linux distros.
>
> Ack, thanks for checking.
>
> --
> Regards,
>
> Sakari Ailus
--
Kind Regards
Mehdi Djait
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-05-14 11:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-10 12:23 [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer Mehdi Djait
2025-03-11 14:51 ` kernel test robot
2025-03-11 16:46 ` kernel test robot
2025-05-10 12:56 ` Sakari Ailus
2025-05-12 8:21 ` Mehdi Djait
2025-05-12 15:41 ` Sakari Ailus
2025-05-14 11:43 ` Mehdi Djait
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.