[PATCH v2] media: v4l2-common: Always register clock with device-specific name

[PATCH v2] media: v4l2-common: Always register clock with device-specific name

[Paul Cercueil]

If we need to register a dummy fixed-frequency clock, always register it
using a device-specific name.

This supports the use case where a system has two of the same sensor,
meaning two instances of the same driver, which previously both tried
(and failed) to create a clock with the same name.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2: include requested clock id in fixed clock name

 drivers/media/v4l2-core/v4l2-common.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 554c591e1113..f32263ba96c4 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -792,14 +792,11 @@ struct clk *__devm_v4l2_sensor_clk_get(struct device *dev, const char *id,
 	if (ret)
 		return ERR_PTR(ret == -EINVAL ? -EPROBE_DEFER : ret);
 
-	if (!id) {
-		clk_id = kasprintf(GFP_KERNEL, "clk-%s", dev_name(dev));
-		if (!clk_id)
-			return ERR_PTR(-ENOMEM);
-		id = clk_id;
-	}
+	clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s", dev_name(dev), id);
+	if (!clk_id)
+		return ERR_PTR(-ENOMEM);
 
-	clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
+	clk_hw = devm_clk_hw_register_fixed_rate(dev, clk_id, NULL, 0, rate);
 	if (IS_ERR(clk_hw))
 		return ERR_CAST(clk_hw);
 
-- 
2.53.0

Re: [PATCH v2] media: v4l2-common: Always register clock with device-specific name

[Mehdi Djait]

Hi Paul,

Thank you for the patch!

On Fri, Mar 27, 2026 at 11:57:50PM +0100, Paul Cercueil wrote:
> If we need to register a dummy fixed-frequency clock, always register it
> using a device-specific name.
> 
> This supports the use case where a system has two of the same sensor,
> meaning two instances of the same driver, which previously both tried
> (and failed) to create a clock with the same name.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Notes:
>     v2: include requested clock id in fixed clock name
> 
>  drivers/media/v4l2-core/v4l2-common.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 554c591e1113..f32263ba96c4 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -792,14 +792,11 @@ struct clk *__devm_v4l2_sensor_clk_get(struct device *dev, const char *id,
>  	if (ret)
>  		return ERR_PTR(ret == -EINVAL ? -EPROBE_DEFER : ret);
>  
> -	if (!id) {
> -		clk_id = kasprintf(GFP_KERNEL, "clk-%s", dev_name(dev));
> -		if (!clk_id)
> -			return ERR_PTR(-ENOMEM);
> -		id = clk_id;
> -	}
> +	clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s", dev_name(dev), id);

How about this, for drivers calling this without id:

	if (id)
		clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s", dev_name(dev), id);
	else
		clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s", dev_name(dev));

	if (!clk_id)
		return ERR_PTR(-ENOMEM);
>  
> -	clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
> +	clk_hw = devm_clk_hw_register_fixed_rate(dev, clk_id, NULL, 0, rate);
>  	if (IS_ERR(clk_hw))
>  		return ERR_CAST(clk_hw);

--
Kind Regards
Mehdi Djait

Re: [PATCH v2] media: v4l2-common: Always register clock with device-specific name

[Mehdi Djait]

On Mon, Mar 30, 2026 at 06:21:56PM +0200, Mehdi Djait wrote:

[..]

> 
> How about this, for drivers calling this without id:
> 
> 	if (id)
> 		clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s", dev_name(dev), id);
> 	else
> 		clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s", dev_name(dev));
							|
							+-> of course
							without this %s
> 
> 	if (!clk_id)
> 		return ERR_PTR(-ENOMEM);

--
Kind Regards
Mehdi Djait

Re: [PATCH v2] media: v4l2-common: Always register clock with device-specific name

[Paul Cercueil]

Hi Mehdi,

Le lundi 30 mars 2026 à 18:21 +0200, Mehdi Djait a écrit :
> Hi Paul,
> 
> Thank you for the patch!
> 
> On Fri, Mar 27, 2026 at 11:57:50PM +0100, Paul Cercueil wrote:
> > If we need to register a dummy fixed-frequency clock, always
> > register it
> > using a device-specific name.
> > 
> > This supports the use case where a system has two of the same
> > sensor,
> > meaning two instances of the same driver, which previously both
> > tried
> > (and failed) to create a clock with the same name.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > ---
> > 
> > Notes:
> >     v2: include requested clock id in fixed clock name
> > 
> >  drivers/media/v4l2-core/v4l2-common.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c
> > b/drivers/media/v4l2-core/v4l2-common.c
> > index 554c591e1113..f32263ba96c4 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -792,14 +792,11 @@ struct clk *__devm_v4l2_sensor_clk_get(struct
> > device *dev, const char *id,
> >  	if (ret)
> >  		return ERR_PTR(ret == -EINVAL ? -EPROBE_DEFER :
> > ret);
> >  
> > -	if (!id) {
> > -		clk_id = kasprintf(GFP_KERNEL, "clk-%s",
> > dev_name(dev));
> > -		if (!clk_id)
> > -			return ERR_PTR(-ENOMEM);
> > -		id = clk_id;
> > -	}
> > +	clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s", dev_name(dev),
> > id);
> 
> How about this, for drivers calling this without id:
> 
> 	if (id)
> 		clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s",
> dev_name(dev), id);
> 	else
> 		clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s",
> dev_name(dev));
> 
> 	if (!clk_id)
> 		return ERR_PTR(-ENOMEM);

In the case where "id" is NULL then the name will be e.g.
"clk-ov2680@1c-(null)" which I assumed was fine - even with a valid
"id" the clock name wouldn't be very pretty anyway.

But I can't update it to your suggestion and send a v3.

Cheers,
-Paul Cercueil

Re: [PATCH v2] media: v4l2-common: Always register clock with device-specific name

[Mehdi Djait]

Hi Paul,


[..]

> > 	if (id)
> > 		clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s",
> > dev_name(dev), id);
> > 	else
> > 		clk_id = kasprintf(GFP_KERNEL, "clk-%s-%s",
> > dev_name(dev));
> > 
> > 	if (!clk_id)
> > 		return ERR_PTR(-ENOMEM);
> 
> In the case where "id" is NULL then the name will be e.g.
> "clk-ov2680@1c-(null)" which I assumed was fine - even with a valid
> "id" the clock name wouldn't be very pretty anyway.
> 
> But I can't update it to your suggestion and send a v3.

Yes, please.

--
Kind Regards
Mehdi Djait