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

[PATCH 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. The reason for having two is not totally
clear (see discussion in patch 2), but there seem to be no advantage in
having two instead of one.

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/71c44221-b18b-4928-8faf-00893ec4a109@nxp.com/

Signed-off-by: Luca Ceresoli <luca.ceresoli@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 +--
 drivers/thermal/broadcom/bcm2835_thermal.c |  2 +-
 lib/vsprintf.c                             | 10 ++--------
 3 files changed, 4 insertions(+), 11 deletions(-)
---
base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
change-id: 20250307-vsprintf-pcn-8a43e3b0d43e

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

[PATCH 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.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 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

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

[Geert Uytterhoeven]

On Fri, 7 Mar 2025 at 12:19, Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> The %pC and %pCn printk format specifiers produce the exact same string. In
> preparation for removing %pCn, use %pC.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Daniel Lezcano]

On 07/03/2025 12:19, Luca Ceresoli wrote:
> The %pC and %pCn printk format specifiers produce the exact same string. In
> preparation for removing %pCn, use %pC.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---

Applied patch 1/2

Thanks!

-- 
<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

[PATCH 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.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/core-api/printk-formats.rst |  3 +--
 lib/vsprintf.c                            | 10 ++--------
 2 files changed, 3 insertions(+), 10 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/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 2/2] vsprintf: remove redundant and unused %pCn format specifier

[Geert Uytterhoeven]

Hi Luca,

On Fri, 7 Mar 2025 at 12:19, Luca Ceresoli <luca.ceresoli@bootlin.com> 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.

The original intention was indeed to serve as a default, like some
other formats also have ("%pa[p]", %pOF[f]").

> 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.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

I guess this is a worthwhile cleanup.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Andy Shevchenko]

On Fri, Mar 07, 2025 at 12:19:08PM +0100, 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.

You seem forgot to update translation(s) of the documentation.

-- 
With Best Regards,
Andy Shevchenko

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

[Luca Ceresoli]

Hello Andy,

On Fri, 7 Mar 2025 19:17:26 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Fri, Mar 07, 2025 at 12:19:08PM +0100, 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.  
> 
> You seem forgot to update translation(s) of the documentation.

I'm afraid I don't speak Chinese. :-)

For this specific change I think I could come up with an approximation
of it, but the both the docs and git log suggest this is not expected.

Luca

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

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

[Andy Shevchenko]

On Sat, Mar 08, 2025 at 12:34:25AM +0100, Luca Ceresoli wrote:
> On Fri, 7 Mar 2025 19:17:26 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Mar 07, 2025 at 12:19:08PM +0100, 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.  
> > 
> > You seem forgot to update translation(s) of the documentation.
> 
> I'm afraid I don't speak Chinese. :-)

At bare minimum we can drop the same line in the list.
Also in such a case we may ask a Chinese speaking person to review / correct /
suggest the changes. I would not leave a leftover as it will be forgotten so
easily and documentation becomes not in sync.

> For this specific change I think I could come up with an approximation
> of it, but the both the docs and git log suggest this is not expected.


-- 
With Best Regards,
Andy Shevchenko

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

[Luca Ceresoli]

Hello,

+To: Alex Shi, Yanteng Si (Chinese docs maintainers)
+Cc: Binbin Zhou (contributor to English version of printk-formats.rst)

On Mon, 10 Mar 2025 10:17:14 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sat, Mar 08, 2025 at 12:34:25AM +0100, Luca Ceresoli wrote:
> > On Fri, 7 Mar 2025 19:17:26 +0200
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:  
> > > On Fri, Mar 07, 2025 at 12:19:08PM +0100, 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.    
> > > 
> > > You seem forgot to update translation(s) of the documentation.  
> > 
> > I'm afraid I don't speak Chinese. :-)  
> 
> At bare minimum we can drop the same line in the list.
> Also in such a case we may ask a Chinese speaking person to review / correct /
> suggest the changes. I would not leave a leftover as it will be forgotten so
> easily and documentation becomes not in sync.

Alex Shi, Yanteng Si: what is the process to update the Chinese
documentation after changes to the English one, considering I don't
speak Chinese?

The documentation [0] seems to imply it will be done by you after the
English documentation update, is it the case?

Otherwise, for this simple change, I can add a patch that looks like a
somewhat correct one for Chinese:

@@ -523,9 +523,8 @@ clk结构体
 ::
 
 	%pC	pll1
-	%pCn	pll1
 
-用于打印clk结构。%pC 和 %pCn 打印时钟的名称（通用时钟框架）或唯一的32位
+用于打印clk结构。%pC 打印时钟的名称（通用时钟框架）或唯一的32位
 ID（传统时钟框架）。
 
 通过引用传递。

Can you confirm the above is a correct change?

Luca

[0] https://docs.kernel.org/translations/index.html

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

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

[Stephen Boyd]

Quoting Luca Ceresoli (2025-03-07 03:19:08)
> %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.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>

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

[Geert Uytterhoeven]

Hi Luca,

On Fri, 7 Mar 2025 at 12:19, Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> There are two printk format specifiers for clocks: %pC and %pCn, and they
> print exactly the same string. The reason for having two is not totally
> clear (see discussion in patch 2), but there seem to be no advantage in
> having two instead of one.
>
> 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/71c44221-b18b-4928-8faf-00893ec4a109@nxp.com/

The link looks unrelated?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Luca Ceresoli]

On Fri, 7 Mar 2025 13:13:19 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Luca,
> 
> On Fri, 7 Mar 2025 at 12:19, Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> > There are two printk format specifiers for clocks: %pC and %pCn, and they
> > print exactly the same string. The reason for having two is not totally
> > clear (see discussion in patch 2), but there seem to be no advantage in
> > having two instead of one.
> >
> > 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/71c44221-b18b-4928-8faf-00893ec4a109@nxp.com/  
> 
> The link looks unrelated?

Wrong link, sorry. The correct one
is: https://lore.kernel.org/dri-devel/f8df2b5e-b005-4ada-8108-159b2b94a72e@nxp.com/

Luca

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