[PATCH v2 0/2] vsprintf: remove redundant %pCn format specifier

[PATCH v2 0/2] vsprintf: remove redundant %pCn format specifier

[Luca Ceresoli]

There are two printk format specifiers for clocks: %pC and %pCn, and they
print exactly the same string. Geert confirmed the intended reason for
having two was that %pC would act as a default, like some (but not all)
formats do. However there seem to be no advantage in having two instead of
one, especially now that there is no other %pC specifier since commit
666902e42fd8 ("lib/vsprintf: Remove atomic-unsafe support for %pCr").

Definitely having two without properly documenting they do the same creates
misunderstandings [0].

Since %pCn is used in a single place, replace it with %pC and remove %pCn
to simplify such format specifiers implementation and avoid
misunderstandings.

[0] https://lore.kernel.org/dri-devel/f8df2b5e-b005-4ada-8108-159b2b94a72e@nxp.com/

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v2:
- Add Chinese translation
- Link to v1: https://lore.kernel.org/r/20250307-vsprintf-pcn-v1-0-df0b2ccf610f@bootlin.com

---
Luca Ceresoli (2):
      thermal: bcm2835: use %pC instead of %pCn
      vsprintf: remove redundant and unused %pCn format specifier

 Documentation/core-api/printk-formats.rst                    |  3 +--
 Documentation/translations/zh_CN/core-api/printk-formats.rst |  3 +--
 drivers/thermal/broadcom/bcm2835_thermal.c                   |  2 +-
 lib/vsprintf.c                                               | 10 ++--------
 4 files changed, 5 insertions(+), 13 deletions(-)
---
base-commit: 4d872d51bc9d7b899c1f61534e3dbde72613f627
change-id: 20250307-vsprintf-pcn-8a43e3b0d43e

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

[PATCH v2 1/2] thermal: bcm2835: use %pC instead of %pCn

[Luca Ceresoli]

The %pC and %pCn printk format specifiers produce the exact same string. In
preparation for removing %pCn, use %pC.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2: none.
---
 drivers/thermal/broadcom/bcm2835_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/broadcom/bcm2835_thermal.c b/drivers/thermal/broadcom/bcm2835_thermal.c
index 7fbba2233c4c12e14090832a62784e1500c0ab79..685a5aee5e0dfb113638313a9086721beae09db3 100644
--- a/drivers/thermal/broadcom/bcm2835_thermal.c
+++ b/drivers/thermal/broadcom/bcm2835_thermal.c
@@ -192,7 +192,7 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
 	rate = clk_get_rate(data->clk);
 	if ((rate < 1920000) || (rate > 5000000))
 		dev_warn(dev,
-			 "Clock %pCn running at %lu Hz is outside of the recommended range: 1.92 to 5MHz\n",
+			 "Clock %pC running at %lu Hz is outside of the recommended range: 1.92 to 5MHz\n",
 			 data->clk, rate);
 
 	/* register of thermal sensor and get info from DT */

-- 
2.48.1

[PATCH v2 2/2] vsprintf: remove redundant and unused %pCn format specifier

[Luca Ceresoli]

%pC and %pCn print the same string, and commit 900cca294425 ("lib/vsprintf:
add %pC{,n,r} format specifiers for clocks") introducing them does not
clarify any intended difference. It can be assumed %pC is a default for
%pCn as some other specifiers do, but not all are consistent with this
policy. Moreover there is now no other suffix other than 'n', which makes a
default not really useful.

All users in the kernel were using %pC except for one which has been
converted. So now remove %pCn and all the unnecessary extra code and
documentation.

Acked-by: Stephen Boyd <sboyd@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- update Chinese documentation (change acked by Yanteng Si <si.yanteng@linux.dev>)
---
 Documentation/core-api/printk-formats.rst                    |  3 +--
 Documentation/translations/zh_CN/core-api/printk-formats.rst |  3 +--
 lib/vsprintf.c                                               | 10 ++--------
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index ecccc0473da9c10f45f2464566f690472c61401e..f3009e6ec80a864c330c8812efcd82c12f4066b3 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -571,9 +571,8 @@ struct clk
 ::
 
 	%pC	pll1
-	%pCn	pll1
 
-For printing struct clk structures. %pC and %pCn print the name of the clock
+For printing struct clk structures. %pC prints the name of the clock
 (Common Clock Framework) or a unique 32-bit ID (legacy clock framework).
 
 Passed by reference.
diff --git a/Documentation/translations/zh_CN/core-api/printk-formats.rst b/Documentation/translations/zh_CN/core-api/printk-formats.rst
index bd36d35eba4eb124be43a66227059a30429e4135..96a917ecc93f2a4872784b6d8e3f98bcb9f5f737 100644
--- a/Documentation/translations/zh_CN/core-api/printk-formats.rst
+++ b/Documentation/translations/zh_CN/core-api/printk-formats.rst
@@ -523,9 +523,8 @@ clk结构体
 ::
 
 	%pC	pll1
-	%pCn	pll1
 
-用于打印clk结构。%pC 和 %pCn 打印时钟的名称（通用时钟框架）或唯一的32位
+用于打印clk结构。%pC 打印时钟的名称（通用时钟框架）或唯一的32位
 ID（传统时钟框架）。
 
 通过引用传递。
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 56fe96319292674c9f79559cf78dd0d99d1a1f06..143d55cb1c12acac21a6c6bafd255437e878f280 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1969,15 +1969,11 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
 	if (check_pointer(&buf, end, clk, spec))
 		return buf;
 
-	switch (fmt[1]) {
-	case 'n':
-	default:
 #ifdef CONFIG_COMMON_CLK
-		return string(buf, end, __clk_get_name(clk), spec);
+	return string(buf, end, __clk_get_name(clk), spec);
 #else
-		return ptr_to_id(buf, end, clk, spec);
+	return ptr_to_id(buf, end, clk, spec);
 #endif
-	}
 }
 
 static
@@ -2382,8 +2378,6 @@ char *rust_fmt_argument(char *buf, char *end, void *ptr);
  *      T    time64_t
  * - 'C' For a clock, it prints the name (Common Clock Framework) or address
  *       (legacy clock framework) of the clock
- * - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
- *        (legacy clock framework) of the clock
  * - 'G' For flags to be printed as a collection of symbolic strings that would
  *       construct the specific value. Supported flags given by option:
  *       p page flags (see struct page) given as pointer to unsigned long

-- 
2.48.1

Re: [PATCH v2 2/2] vsprintf: remove redundant and unused %pCn format specifier

[Yanteng Si]

在 3/11/25 5:21 PM, Luca Ceresoli 写道:
> %pC and %pCn print the same string, and commit 900cca294425 ("lib/vsprintf:
> add %pC{,n,r} format specifiers for clocks") introducing them does not
> clarify any intended difference. It can be assumed %pC is a default for
> %pCn as some other specifiers do, but not all are consistent with this
> policy. Moreover there is now no other suffix other than 'n', which makes a
> default not really useful.
>
> All users in the kernel were using %pC except for one which has been
> converted. So now remove %pCn and all the unnecessary extra code and
> documentation.
>
> Acked-by: Stephen Boyd <sboyd@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Reviewed-by: Yanteng Si <si.yanteng@linux.dev>


Thanks,

Yanteng

>
> ---
>
> Changes in v2:
> - update Chinese documentation (change acked by Yanteng Si <si.yanteng@linux.dev>)
> ---
>   Documentation/core-api/printk-formats.rst                    |  3 +--
>   Documentation/translations/zh_CN/core-api/printk-formats.rst |  3 +--
>   lib/vsprintf.c                                               | 10 ++--------
>   3 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index ecccc0473da9c10f45f2464566f690472c61401e..f3009e6ec80a864c330c8812efcd82c12f4066b3 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -571,9 +571,8 @@ struct clk
>   ::
>   
>   	%pC	pll1
> -	%pCn	pll1
>   
> -For printing struct clk structures. %pC and %pCn print the name of the clock
> +For printing struct clk structures. %pC prints the name of the clock
>   (Common Clock Framework) or a unique 32-bit ID (legacy clock framework).
>   
>   Passed by reference.
> diff --git a/Documentation/translations/zh_CN/core-api/printk-formats.rst b/Documentation/translations/zh_CN/core-api/printk-formats.rst
> index bd36d35eba4eb124be43a66227059a30429e4135..96a917ecc93f2a4872784b6d8e3f98bcb9f5f737 100644
> --- a/Documentation/translations/zh_CN/core-api/printk-formats.rst
> +++ b/Documentation/translations/zh_CN/core-api/printk-formats.rst
> @@ -523,9 +523,8 @@ clk结构体
>   ::
>   
>   	%pC	pll1
> -	%pCn	pll1
>   
> -用于打印clk结构。%pC 和 %pCn 打印时钟的名称（通用时钟框架）或唯一的32位
> +用于打印clk结构。%pC 打印时钟的名称（通用时钟框架）或唯一的32位
>   ID（传统时钟框架）。
>   
>   通过引用传递。
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 56fe96319292674c9f79559cf78dd0d99d1a1f06..143d55cb1c12acac21a6c6bafd255437e878f280 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1969,15 +1969,11 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
>   	if (check_pointer(&buf, end, clk, spec))
>   		return buf;
>   
> -	switch (fmt[1]) {
> -	case 'n':
> -	default:
>   #ifdef CONFIG_COMMON_CLK
> -		return string(buf, end, __clk_get_name(clk), spec);
> +	return string(buf, end, __clk_get_name(clk), spec);
>   #else
> -		return ptr_to_id(buf, end, clk, spec);
> +	return ptr_to_id(buf, end, clk, spec);
>   #endif
> -	}
>   }
>   
>   static
> @@ -2382,8 +2378,6 @@ char *rust_fmt_argument(char *buf, char *end, void *ptr);
>    *      T    time64_t
>    * - 'C' For a clock, it prints the name (Common Clock Framework) or address
>    *       (legacy clock framework) of the clock
> - * - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
> - *        (legacy clock framework) of the clock
>    * - 'G' For flags to be printed as a collection of symbolic strings that would
>    *       construct the specific value. Supported flags given by option:
>    *       p page flags (see struct page) given as pointer to unsigned long
>

Re: [PATCH v2 2/2] vsprintf: remove redundant and unused %pCn format specifier

[Petr Mladek]

On Tue 2025-03-11 10:21:23, Luca Ceresoli wrote:
> %pC and %pCn print the same string, and commit 900cca294425 ("lib/vsprintf:
> add %pC{,n,r} format specifiers for clocks") introducing them does not
> clarify any intended difference. It can be assumed %pC is a default for
> %pCn as some other specifiers do, but not all are consistent with this
> policy. Moreover there is now no other suffix other than 'n', which makes a
> default not really useful.
> 
> All users in the kernel were using %pC except for one which has been
> converted. So now remove %pCn and all the unnecessary extra code and
> documentation.
> 
> Acked-by: Stephen Boyd <sboyd@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Makes sense. Looks and works well, so:

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

Daniel, if I get it correctly, you have already taken the 1st patch.
Would you mind to take also this patch using the same tree, please?
Otherwise, we would need to coordinate pull requests in the upcoming
merge window ;-)

Best Regards,
Petr

Re: [PATCH v2 2/2] vsprintf: remove redundant and unused %pCn format specifier

[Luca Ceresoli]

Hello Petr, Daniel,

On Tue, 11 Mar 2025 15:36:11 +0100
Petr Mladek <pmladek@suse.com> wrote:

> On Tue 2025-03-11 10:21:23, Luca Ceresoli wrote:
> > %pC and %pCn print the same string, and commit 900cca294425 ("lib/vsprintf:
> > add %pC{,n,r} format specifiers for clocks") introducing them does not
> > clarify any intended difference. It can be assumed %pC is a default for
> > %pCn as some other specifiers do, but not all are consistent with this
> > policy. Moreover there is now no other suffix other than 'n', which makes a
> > default not really useful.
> > 
> > All users in the kernel were using %pC except for one which has been
> > converted. So now remove %pCn and all the unnecessary extra code and
> > documentation.
> > 
> > Acked-by: Stephen Boyd <sboyd@kernel.org>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>  
> 
> Makes sense. Looks and works well, so:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>
> 
> Daniel, if I get it correctly, you have already taken the 1st patch.
> Would you mind to take also this patch using the same tree, please?
> Otherwise, we would need to coordinate pull requests in the upcoming
> merge window ;-)

I see none of these two patches in linux-next.

Anything I should do? Resend? Or just wait a bit more?

Best regards,
Luca

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

Re: [PATCH v2 2/2] vsprintf: remove redundant and unused %pCn format specifier

[Petr Mladek]

On Mon 2025-04-07 18:46:47, Luca Ceresoli wrote:
> Hello Petr, Daniel,
> 
> On Tue, 11 Mar 2025 15:36:11 +0100
> Petr Mladek <pmladek@suse.com> wrote:
> 
> > On Tue 2025-03-11 10:21:23, Luca Ceresoli wrote:
> > > %pC and %pCn print the same string, and commit 900cca294425 ("lib/vsprintf:
> > > add %pC{,n,r} format specifiers for clocks") introducing them does not
> > > clarify any intended difference. It can be assumed %pC is a default for
> > > %pCn as some other specifiers do, but not all are consistent with this
> > > policy. Moreover there is now no other suffix other than 'n', which makes a
> > > default not really useful.
> > > 
> > > All users in the kernel were using %pC except for one which has been
> > > converted. So now remove %pCn and all the unnecessary extra code and
> > > documentation.
> > > 
> > > Acked-by: Stephen Boyd <sboyd@kernel.org>
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>  
> > 
> > Makes sense. Looks and works well, so:
> > 
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> > Tested-by: Petr Mladek <pmladek@suse.com>
> > 
> > Daniel, if I get it correctly, you have already taken the 1st patch.
> > Would you mind to take also this patch using the same tree, please?
> > Otherwise, we would need to coordinate pull requests in the upcoming
> > merge window ;-)
> 
> I see none of these two patches in linux-next.

I see.

> Anything I should do? Resend? Or just wait a bit more?

Daniel,	Rafael, Zhang, Lukasz,

would you like to take both patches via the linux-pm.git thermal tree?
Or should I take both patches via the printk tree?
Both ways work for me.

Best Regards,
Petr

Re: [PATCH v2 2/2] vsprintf: remove redundant and unused %pCn format specifier

[Daniel Lezcano]

On Tue, Apr 08, 2025 at 09:33:57AM +0200, Petr Mladek wrote:
> On Mon 2025-04-07 18:46:47, Luca Ceresoli wrote:
> > Hello Petr, Daniel,
> > 
> > On Tue, 11 Mar 2025 15:36:11 +0100
> > Petr Mladek <pmladek@suse.com> wrote:
> > 
> > > On Tue 2025-03-11 10:21:23, Luca Ceresoli wrote:
> > > > %pC and %pCn print the same string, and commit 900cca294425 ("lib/vsprintf:
> > > > add %pC{,n,r} format specifiers for clocks") introducing them does not
> > > > clarify any intended difference. It can be assumed %pC is a default for
> > > > %pCn as some other specifiers do, but not all are consistent with this
> > > > policy. Moreover there is now no other suffix other than 'n', which makes a
> > > > default not really useful.
> > > > 
> > > > All users in the kernel were using %pC except for one which has been
> > > > converted. So now remove %pCn and all the unnecessary extra code and
> > > > documentation.
> > > > 
> > > > Acked-by: Stephen Boyd <sboyd@kernel.org>
> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>  
> > > 
> > > Makes sense. Looks and works well, so:
> > > 
> > > Reviewed-by: Petr Mladek <pmladek@suse.com>
> > > Tested-by: Petr Mladek <pmladek@suse.com>
> > > 
> > > Daniel, if I get it correctly, you have already taken the 1st patch.
> > > Would you mind to take also this patch using the same tree, please?
> > > Otherwise, we would need to coordinate pull requests in the upcoming
> > > merge window ;-)
> > 
> > I see none of these two patches in linux-next.
> 
> I see.
> 
> > Anything I should do? Resend? Or just wait a bit more?
> 
> Daniel,	Rafael, Zhang, Lukasz,
> 
> would you like to take both patches via the linux-pm.git thermal tree?
> Or should I take both patches via the printk tree?
> Both ways work for me.

I'll take care of them now

Thanks for the heads up

  -- Daniel

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2 0/2] vsprintf: remove redundant %pCn format specifier

[Daniel Lezcano]

On Tue, Mar 11, 2025 at 10:21:21AM +0100, Luca Ceresoli wrote:
> There are two printk format specifiers for clocks: %pC and %pCn, and they
> print exactly the same string. Geert confirmed the intended reason for
> having two was that %pC would act as a default, like some (but not all)
> formats do. However there seem to be no advantage in having two instead of
> one, especially now that there is no other %pC specifier since commit
> 666902e42fd8 ("lib/vsprintf: Remove atomic-unsafe support for %pCr").
> 
> Definitely having two without properly documenting they do the same creates
> misunderstandings [0].
> 
> Since %pCn is used in a single place, replace it with %pC and remove %pCn
> to simplify such format specifiers implementation and avoid
> misunderstandings.
> 
> [0] https://lore.kernel.org/dri-devel/f8df2b5e-b005-4ada-8108-159b2b94a72e@nxp.com/
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---

Applied, thanks