| Message ID | 20250321130329.342236-1-mehdi.djait@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,v4] media: v4l2-common: Add a helper for obtaining the clock producer | expand |
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@linux.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. 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 > --- > v1 -> 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 > > v2 -> 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/ > > v3 -> 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/ > > > drivers/media/v4l2-core/v4l2-common.c | 40 +++++++++++++++++++++++++++ > include/media/v4l2-common.h | 18 ++++++++++++ > 2 files changed, 58 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > index 0a2f4f0d0a07..b33152e2c3af 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,40 @@ 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) > +{ > + const char *clk_id __free(kfree) = NULL; > + 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_REACHABLE(CONFIG_COMMON_CLK)) > + return ERR_PTR(-ENOENT); > + > + 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) { > + clk_id = kasprintf(GFP_KERNEL, "clk-%s", dev_name(dev)); > + if (!clk_id) > + return ERR_PTR(-ENOMEM); > + id = clk_id; > + } > + > + 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) > { > /*
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@linux.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
Hi Laurent, On Thu, May 15, 2025 at 10:44:03AM +0200, Laurent Pinchart wrote: > Hi Hans, > > 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@linux.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) > > 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 ? > > We really need to standardize the clock-frequency property, and document > it accordingly. Some drivers use it to set the external clock rate, > while others use it to inform themselves about the clock rate, without > changing it, for platforms that have no CCF clock providers. Some > drivers also set the clock rate to a fixed value, or to a value that > depends on the link frequency selected by userspace. I don't like this > situation much. > > > > --- > > > v1 -> 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 > > > > > > v2 -> 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/ > > > > > > v3 -> 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/ > > > > > > > > > drivers/media/v4l2-core/v4l2-common.c | 40 +++++++++++++++++++++++++++ > > > include/media/v4l2-common.h | 18 ++++++++++++ > > > 2 files changed, 58 insertions(+) > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > > > index 0a2f4f0d0a07..b33152e2c3af 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,40 @@ 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) > > > +{ > > > + const char *clk_id __free(kfree) = NULL; > > > + 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_REACHABLE(CONFIG_COMMON_CLK)) > > > + return ERR_PTR(-ENOENT); > > > + > > > + 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) { > > > + clk_id = kasprintf(GFP_KERNEL, "clk-%s", dev_name(dev)); > > > + if (!clk_id) > > > + return ERR_PTR(-ENOMEM); > > > + id = clk_id; > > > + } > > > + > > > + 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) > > > { > > > /* > > -- > Regards, > > Laurent Pinchart -- Kind Regards Mehdi Djait
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@linux.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. Could we document how this is supposed to work on DT and ACPI? 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. > > 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. > > i2c { > #address-cells = <1>; > #size-cells = <0>; > > sensor@6c { > compatible = "sony,imx258"; > reg = <0x6c>; > clocks = <&imx258_clk>; > > port { > endpoint { > remote-endpoint = <&csi1_ep>; > data-lanes = <1 2 3 4>; > link-frequencies = /bits/ 64 <320000000>; > }; > }; > }; > }; > > /* Oscillator on the camera board */ > imx258_clk: clk { > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <19200000>; > }; >
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index 0a2f4f0d0a07..b33152e2c3af 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,40 @@ 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) +{ + const char *clk_id __free(kfree) = NULL; + 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_REACHABLE(CONFIG_COMMON_CLK)) + return ERR_PTR(-ENOENT); + + 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) { + clk_id = kasprintf(GFP_KERNEL, "clk-%s", dev_name(dev)); + if (!clk_id) + return ERR_PTR(-ENOMEM); + id = clk_id; + } + + 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) { /*
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@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 v1: https://lore.kernel.org/linux-media/20250227092643.113939-1-mehdi.djait@linux.intel.com v2 -> 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/ v3 -> 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/ drivers/media/v4l2-core/v4l2-common.c | 40 +++++++++++++++++++++++++++ include/media/v4l2-common.h | 18 ++++++++++++ 2 files changed, 58 insertions(+)