[PATCH 0/2] drm/bridge: two minor code improvements

[PATCH 0/2] drm/bridge: two minor code improvements

[Luca Ceresoli]

This small series brings small improvements to two bridge drivers.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Luca Ceresoli (2):
      drm/bridge: imx8qxp-ldb: cleanup return value
      drm/bridge: fsl-ldb: make warning message more informative

 drivers/gpu/drm/bridge/fsl-ldb.c         | 6 +++---
 drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
---
base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
change-id: 20250306-drm-two-ldb-improvements-16fa67cc0eaf

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>

[PATCH 1/2] drm/bridge: imx8qxp-ldb: cleanup return value

[Luca Ceresoli]

'ret' can only be 0 at this point, being preceded by a 'if (ret) return
ret;'. So return 0 for clarity.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
@@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
 
 	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
 
-	return ret;
+	return 0;
 }
 
 static void imx8qxp_ldb_remove(struct platform_device *pdev)

-- 
2.48.1

Re: [PATCH 1/2] drm/bridge: imx8qxp-ldb: cleanup return value

[Frank Li]

On Thu, Mar 06, 2025 at 06:28:40PM +0100, Luca Ceresoli wrote:
> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> ret;'. So return 0 for clarity.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
>
>  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
>
> -	return ret;
> +	return 0;

I don't think it is necessary, no difference.

Frank

>  }
>
>  static void imx8qxp_ldb_remove(struct platform_device *pdev)
>
> --
> 2.48.1
>

Re: [PATCH 1/2] drm/bridge: imx8qxp-ldb: cleanup return value

[Liu Ying]

On 03/07/2025, Luca Ceresoli wrote:
> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> ret;'. So return 0 for clarity.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
>  
>  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
>  
> -	return ret;
> +	return 0;

I guess this is not the only place across the kernel tree where this cleanup
could be done.  So, maybe use some tools to cleanup them all?

>  }
>  
>  static void imx8qxp_ldb_remove(struct platform_device *pdev)
> 

-- 
Regards,
Liu Ying

Re: [PATCH 1/2] drm/bridge: imx8qxp-ldb: cleanup return value

[Luca Ceresoli]

Hello Liu,

On Fri, 7 Mar 2025 14:42:12 +0800
Liu Ying <victor.liu@nxp.com> wrote:

> On 03/07/2025, Luca Ceresoli wrote:
> > 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> > ret;'. So return 0 for clarity.
> > 
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > ---
> >  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
> > --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
> >  
> >  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
> >  
> > -	return ret;
> > +	return 0;  
> 
> I guess this is not the only place across the kernel tree where this cleanup
> could be done.  So, maybe use some tools to cleanup them all?

I had stumbled upon this as I was doing some changes to this function,
and needed to understand the code flow. Definitely 'ret 0' would have
made it  immediately clear that all the code between the last 'if (ret)
return ret;' and the final 'return ret' is not allowed to fail.

I think this change would (slightly, but still) improve future readers'
life.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/2] drm/bridge: imx8qxp-ldb: cleanup return value

[Frank Li]

On Fri, Mar 07, 2025 at 12:22:17PM +0100, Luca Ceresoli wrote:
> Hello Liu,
>
> On Fri, 7 Mar 2025 14:42:12 +0800
> Liu Ying <victor.liu@nxp.com> wrote:
>
> > On 03/07/2025, Luca Ceresoli wrote:
> > > 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> > > ret;'. So return 0 for clarity.
> > >
> > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > > ---
> > >  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > > index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
> > > --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > > +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > > @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
> > >
> > >  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
> > >
> > > -	return ret;
> > > +	return 0;
> >
> > I guess this is not the only place across the kernel tree where this cleanup
> > could be done.  So, maybe use some tools to cleanup them all?
>
> I had stumbled upon this as I was doing some changes to this function,
> and needed to understand the code flow. Definitely 'ret 0' would have
> made it  immediately clear that all the code between the last 'if (ret)
> return ret;' and the final 'return ret' is not allowed to fail.
>
> I think this change would (slightly, but still) improve future readers'
> life.

I think "return ret" at probe already become common sense for developer.
No value to take efforts to clean up this.

Frank

>
> Luca
>
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 1/2] drm/bridge: imx8qxp-ldb: cleanup return value

[Liu Ying]

On 03/07/2025, Luca Ceresoli wrote:
> Hello Liu,
> 
> On Fri, 7 Mar 2025 14:42:12 +0800
> Liu Ying <victor.liu@nxp.com> wrote:
> 
>> On 03/07/2025, Luca Ceresoli wrote:
>>> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
>>> ret;'. So return 0 for clarity.
>>>
>>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>>> ---
>>>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
>>> index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
>>> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
>>> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
>>> @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
>>>  
>>>  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
>>>  
>>> -	return ret;
>>> +	return 0;  
>>
>> I guess this is not the only place across the kernel tree where this cleanup
>> could be done.  So, maybe use some tools to cleanup them all?
> 
> I had stumbled upon this as I was doing some changes to this function,
> and needed to understand the code flow. Definitely 'ret 0' would have
> made it  immediately clear that all the code between the last 'if (ret)
> return ret;' and the final 'return ret' is not allowed to fail.
> 
> I think this change would (slightly, but still) improve future readers'
> life.

Reviewed-by: Liu Ying <victor.liu@nxp.com>

> 
> Luca
>

Re: [PATCH 1/2] drm/bridge: imx8qxp-ldb: cleanup return value

[Liu Ying]

On 03/07/2025, Luca Ceresoli wrote:
> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> ret;'. So return 0 for clarity.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to misc/kernel.git (drm-misc-next).
Thanks!

-- 
Regards,
Liu Ying

[PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

[Luca Ceresoli]

This warning notifies a clock was set to an inaccurate value. Modify the
string to also show the clock name.

While doing that also rewrap the entire function call.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
index 0fc8a14fd80062248a43b8b93272101a7ca6158a..c7c899a9644bb827845fb3fe96a9695d79a91474 100644
--- a/drivers/gpu/drm/bridge/fsl-ldb.c
+++ b/drivers/gpu/drm/bridge/fsl-ldb.c
@@ -181,9 +181,9 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
 
 	configured_link_freq = clk_get_rate(fsl_ldb->clk);
 	if (configured_link_freq != requested_link_freq)
-		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
-			 configured_link_freq,
-			 requested_link_freq);
+		dev_warn(fsl_ldb->dev,
+			 "Configured %pC clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
+			 fsl_ldb->clk, configured_link_freq, requested_link_freq);
 
 	clk_prepare_enable(fsl_ldb->clk);
 

-- 
2.48.1

Re: [PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

[Frank Li]

On Thu, Mar 06, 2025 at 06:28:41PM +0100, Luca Ceresoli wrote:
> This warning notifies a clock was set to an inaccurate value. Modify the
> string to also show the clock name.
>
> While doing that also rewrap the entire function call.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index 0fc8a14fd80062248a43b8b93272101a7ca6158a..c7c899a9644bb827845fb3fe96a9695d79a91474 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -181,9 +181,9 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
>
>  	configured_link_freq = clk_get_rate(fsl_ldb->clk);
>  	if (configured_link_freq != requested_link_freq)
> -		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
> -			 configured_link_freq,
> -			 requested_link_freq);
> +		dev_warn(fsl_ldb->dev,
> +			 "Configured %pC clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
> +			 fsl_ldb->clk, configured_link_freq, requested_link_freq);

commit message said show clock name, but %p is for pointer value. Are sure
it show clock name?

Frank

>
>  	clk_prepare_enable(fsl_ldb->clk);
>
>
> --
> 2.48.1
>

Re: [PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

[Liu Ying]

On 03/07/2025, Frank Li wrote:
> On Thu, Mar 06, 2025 at 06:28:41PM +0100, Luca Ceresoli wrote:
>> This warning notifies a clock was set to an inaccurate value. Modify the
>> string to also show the clock name.
>>
>> While doing that also rewrap the entire function call.
>>
>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> ---
>>  drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
>> index 0fc8a14fd80062248a43b8b93272101a7ca6158a..c7c899a9644bb827845fb3fe96a9695d79a91474 100644
>> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
>> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
>> @@ -181,9 +181,9 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
>>
>>  	configured_link_freq = clk_get_rate(fsl_ldb->clk);
>>  	if (configured_link_freq != requested_link_freq)
>> -		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
>> -			 configured_link_freq,
>> -			 requested_link_freq);
>> +		dev_warn(fsl_ldb->dev,
>> +			 "Configured %pC clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
>> +			 fsl_ldb->clk, configured_link_freq, requested_link_freq);
> 
> commit message said show clock name, but %p is for pointer value. Are sure
> it show clock name?

%pC prints clock name.  Please see Documentation/core-api/printk-formats.rst.

> 
> Frank
> 
>>
>>  	clk_prepare_enable(fsl_ldb->clk);
>>
>>
>> --
>> 2.48.1
>>

-- 
Regards,
Liu Ying

Re: [PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

[Liu Ying]

On 03/07/2025, Luca Ceresoli wrote:
> This warning notifies a clock was set to an inaccurate value. Modify the
> string to also show the clock name.
> 
> While doing that also rewrap the entire function call.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index 0fc8a14fd80062248a43b8b93272101a7ca6158a..c7c899a9644bb827845fb3fe96a9695d79a91474 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -181,9 +181,9 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
>  
>  	configured_link_freq = clk_get_rate(fsl_ldb->clk);
>  	if (configured_link_freq != requested_link_freq)
> -		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
> -			 configured_link_freq,
> -			 requested_link_freq);
> +		dev_warn(fsl_ldb->dev,
> +			 "Configured %pC clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
> +			 fsl_ldb->clk, configured_link_freq, requested_link_freq);

Though this slightly changes the warning message userspace sees, I guess it is
acceptable.

Does it make sense to s/%pC/%pCn/ so that the clock name is printed in lower
case instead of upper case, since it seems that all i.MX specific clock names
are in lower case?

>  
>  	clk_prepare_enable(fsl_ldb->clk);
>  
> 

-- 
Regards,
Liu Ying

Re: [PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

[Luca Ceresoli]

Hello Liu,

thanks for your reviews.

On Fri, 7 Mar 2025 14:33:37 +0800
Liu Ying <victor.liu@nxp.com> wrote:

> On 03/07/2025, Luca Ceresoli wrote:
> > This warning notifies a clock was set to an inaccurate value. Modify the
> > string to also show the clock name.
> > 
> > While doing that also rewrap the entire function call.
> > 
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > ---
> >  drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> > index 0fc8a14fd80062248a43b8b93272101a7ca6158a..c7c899a9644bb827845fb3fe96a9695d79a91474 100644
> > --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> > +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> > @@ -181,9 +181,9 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
> >  
> >  	configured_link_freq = clk_get_rate(fsl_ldb->clk);
> >  	if (configured_link_freq != requested_link_freq)
> > -		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
> > -			 configured_link_freq,
> > -			 requested_link_freq);
> > +		dev_warn(fsl_ldb->dev,
> > +			 "Configured %pC clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
> > +			 fsl_ldb->clk, configured_link_freq, requested_link_freq);  
> 
> Though this slightly changes the warning message userspace sees, I guess it is
> acceptable.
> 
> Does it make sense to s/%pC/%pCn/ so that the clock name is printed in lower
> case instead of upper case, since it seems that all i.MX specific clock names
> are in lower case?

%pC and %pCn print the same string, as I just discovered at
https://elixir.bootlin.com/linux/v6.14-rc5/source/lib/vsprintf.c#L1972

I've pondering for a moment about whether we should document %pC and
%pCn produce the same output or rather %pCn. I decided to try the
latter and just sent a patch to do it [0].

FYI in my case the printed value is (with both %pC and %pCn)
"32ec0000.blk-ctrl:bridge@5c".

[0] https://lore.kernel.org/20250307-vsprintf-pcn-v1-0-df0b2ccf610f@bootlin.com

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

[Liu Ying]

On 03/07/2025, Luca Ceresoli wrote:
> Hello Liu,

Hello Luca,

> 
> thanks for your reviews.
> 
> On Fri, 7 Mar 2025 14:33:37 +0800
> Liu Ying <victor.liu@nxp.com> wrote:
> 
>> On 03/07/2025, Luca Ceresoli wrote:
>>> This warning notifies a clock was set to an inaccurate value. Modify the
>>> string to also show the clock name.
>>>
>>> While doing that also rewrap the entire function call.
>>>
>>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>>> ---
>>>  drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
>>> index 0fc8a14fd80062248a43b8b93272101a7ca6158a..c7c899a9644bb827845fb3fe96a9695d79a91474 100644
>>> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
>>> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
>>> @@ -181,9 +181,9 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
>>>  
>>>  	configured_link_freq = clk_get_rate(fsl_ldb->clk);
>>>  	if (configured_link_freq != requested_link_freq)
>>> -		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
>>> -			 configured_link_freq,
>>> -			 requested_link_freq);
>>> +		dev_warn(fsl_ldb->dev,
>>> +			 "Configured %pC clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n",
>>> +			 fsl_ldb->clk, configured_link_freq, requested_link_freq);  
>>
>> Though this slightly changes the warning message userspace sees, I guess it is
>> acceptable.
>>
>> Does it make sense to s/%pC/%pCn/ so that the clock name is printed in lower
>> case instead of upper case, since it seems that all i.MX specific clock names
>> are in lower case?
> 
> %pC and %pCn print the same string, as I just discovered at
> https://elixir.bootlin.com/linux/v6.14-rc5/source/lib/vsprintf.c#L1972

Ack.

> 
> I've pondering for a moment about whether we should document %pC and
> %pCn produce the same output or rather %pCn. I decided to try the
> latter and just sent a patch to do it [0].
> 
> FYI in my case the printed value is (with both %pC and %pCn)
> "32ec0000.blk-ctrl:bridge@5c".

This is the LDB device name. The i.MX8MP LDB clock name is "media_ldb_root_clk".

Acked-by: Liu Ying <victor.liu@nxp.com>

> 
> [0] https://lore.kernel.org/20250307-vsprintf-pcn-v1-0-df0b2ccf610f@bootlin.com
> 
> Luca
> 

-- 
Regards,
Liu Ying

Re: [PATCH 2/2] drm/bridge: fsl-ldb: make warning message more informative

[Liu Ying]

On 03/07/2025, Luca Ceresoli wrote:
> This warning notifies a clock was set to an inaccurate value. Modify the
> string to also show the clock name.
> 
> While doing that also rewrap the entire function call.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/fsl-ldb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Applied to misc/kernel.git (drm-misc-next).
Thanks!

-- 
Regards,
Liu Ying