[PATCH v4 0/3] Use proper printk format in appletbdrm

[PATCH v4 0/3] Use proper printk format in appletbdrm

[Aditya Garg]

The vsprint patch was originally being sent as a seperate patch [1], and
I was waiting it to be taken up. But as suggested by Petr, I'm sending
them via DRM.

v2:
Remove printf tests, will merge later through Kees' tree

v3:
Re-add printf tests, since 6.15-rc1 has the necessary commits merged now.

v4:
Do changes requested by Andy and add Petr's review to printf patch.

Link: https://lore.kernel.org/lkml/1A03A5B4-93AC-4307-AE6A-4A4C4B7E9472@live.com/ [1]

Aditya Garg (2):
  printf: add tests for generic FourCCs
  drm/appletbdrm: use %p4cl instead of %p4cc

Hector Martin (1):
  lib/vsprintf: Add support for generic FourCCs by extending %p4cc

 Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
 drivers/gpu/drm/tiny/appletbdrm.c         |  4 +--
 lib/tests/printf_kunit.c                  | 39 +++++++++++++++++++----
 lib/vsprintf.c                            | 35 ++++++++++++++++----
 scripts/checkpatch.pl                     |  2 +-
 5 files changed, 96 insertions(+), 16 deletions(-)

-- 
2.43.0

[PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Aditya Garg]

From: Hector Martin <marcan@marcan.st>

%p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
it's useful to be able to print generic 4-character codes formatted as
an integer. Extend it to add format specifiers for printing generic
32-bit FourCCs with various endian semantics:

%p4ch	Host byte order
%p4cn	Network byte order
%p4cl	Little-endian
%p4cb	Big-endian

The endianness determines how bytes are interpreted as a u32, and the
FourCC is then always printed MSByte-first (this is the opposite of
V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
allow printing LSByte-first FourCCs stored in host endian order
(other than the hex form being in character order, not the integer
value).

Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++++
 lib/vsprintf.c                            | 35 +++++++++++++++++++----
 scripts/checkpatch.pl                     |  2 +-
 3 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 4bdc394e8..125fd0397 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -648,6 +648,38 @@ Examples::
 	%p4cc	Y10  little-endian (0x20303159)
 	%p4cc	NV12 big-endian (0xb231564e)
 
+Generic FourCC code
+-------------------
+
+::
+	%p4c[hnlb]	gP00 (0x67503030)
+
+Print a generic FourCC code, as both ASCII characters and its numerical
+value as hexadecimal.
+
+The generic FourCC code is always printed in the big-endian format,
+the most significant byte first. This is the opposite of V4L/DRM FourCCs.
+
+The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
+endianness is used to load the stored bytes. The data might be interpreted
+using the host byte order, network byte order, little-endian, or big-endian.
+
+Passed by reference.
+
+Examples for a little-endian machine, given &(u32)0x67503030::
+
+	%p4ch	gP00 (0x67503030)
+	%p4cn	00Pg (0x30305067)
+	%p4cl	gP00 (0x67503030)
+	%p4cb	00Pg (0x30305067)
+
+Examples for a big-endian machine, given &(u32)0x67503030::
+
+	%p4ch	gP00 (0x67503030)
+	%p4cn	00Pg (0x30305067)
+	%p4cl	00Pg (0x30305067)
+	%p4cb	gP00 (0x67503030)
+
 Rust
 ----
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 01699852f..6bc64ae52 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1793,27 +1793,50 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
 	char output[sizeof("0123 little-endian (0x01234567)")];
 	char *p = output;
 	unsigned int i;
+	bool pixel_fmt = false;
 	u32 orig, val;
 
-	if (fmt[1] != 'c' || fmt[2] != 'c')
+	if (fmt[1] != 'c')
 		return error_string(buf, end, "(%p4?)", spec);
 
 	if (check_pointer(&buf, end, fourcc, spec))
 		return buf;
 
 	orig = get_unaligned(fourcc);
-	val = orig & ~BIT(31);
+	switch (fmt[2]) {
+	case 'h':
+		break;
+	case 'n':
+		orig = swab32(orig);
+		break;
+	case 'l':
+		orig = (__force u32)cpu_to_le32(orig);
+		break;
+	case 'b':
+		orig = (__force u32)cpu_to_be32(orig);
+		break;
+	case 'c':
+		/* Pixel formats are printed LSB-first */
+		pixel_fmt = true;
+		break;
+	default:
+		return error_string(buf, end, "(%p4?)", spec);
+	}
+
+	val = pixel_fmt ? swab32(orig & ~BIT(31)) : orig;
 
 	for (i = 0; i < sizeof(u32); i++) {
-		unsigned char c = val >> (i * 8);
+		unsigned char c = val >> ((3 - i) * 8);
 
 		/* Print non-control ASCII characters as-is, dot otherwise */
 		*p++ = isascii(c) && isprint(c) ? c : '.';
 	}
 
-	*p++ = ' ';
-	strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
-	p += strlen(p);
+	if (pixel_fmt) {
+		*p++ = ' ';
+		strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
+		p += strlen(p);
+	}
 
 	*p++ = ' ';
 	*p++ = '(';
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3d22bf863..44e233b6f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6891,7 +6891,7 @@ sub process {
 					    ($extension eq "f" &&
 					     defined $qualifier && $qualifier !~ /^w/) ||
 					    ($extension eq "4" &&
-					     defined $qualifier && $qualifier !~ /^cc/)) {
+					     defined $qualifier && $qualifier !~ /^c[hnlbc]/)) {
 						$bad_specifier = $specifier;
 						last;
 					}
-- 
2.43.0

[PATCH v4 3/3] drm/appletbdrm: use %p4cl instead of %p4cc

[Aditya Garg]

From: Aditya Garg <gargaditya08@live.com>

Due to lack of a proper format specifier, %p4cc was being used instead
of %p4cl for the purpose of printing FourCCs. But the disadvange was
that they were being printed in a reverse order. %p4cl should correct
this issue.

Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 drivers/gpu/drm/tiny/appletbdrm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
index 703b9a41a..751b05753 100644
--- a/drivers/gpu/drm/tiny/appletbdrm.c
+++ b/drivers/gpu/drm/tiny/appletbdrm.c
@@ -212,7 +212,7 @@ static int appletbdrm_read_response(struct appletbdrm_device *adev,
 	}
 
 	if (response->msg != expected_response) {
-		drm_err(drm, "Unexpected response from device (expected %p4cc found %p4cc)\n",
+		drm_err(drm, "Unexpected response from device (expected %p4cl found %p4cl)\n",
 			&expected_response, &response->msg);
 		return -EIO;
 	}
@@ -286,7 +286,7 @@ static int appletbdrm_get_information(struct appletbdrm_device *adev)
 	}
 
 	if (pixel_format != APPLETBDRM_PIXEL_FORMAT) {
-		drm_err(drm, "Encountered unknown pixel format (%p4cc)\n", &pixel_format);
+		drm_err(drm, "Encountered unknown pixel format (%p4cl)\n", &pixel_format);
 		ret = -EINVAL;
 		goto free_info;
 	}
-- 
2.43.0

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Kees Cook]

On Tue, Apr 08, 2025 at 12:17:57PM +0530, Aditya Garg wrote:
> From: Hector Martin <marcan@marcan.st>
> 
> %p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
> it's useful to be able to print generic 4-character codes formatted as
> an integer. Extend it to add format specifiers for printing generic
> 32-bit FourCCs with various endian semantics:
> 
> %p4ch	Host byte order
> %p4cn	Network byte order
> %p4cl	Little-endian
> %p4cb	Big-endian
> 
> The endianness determines how bytes are interpreted as a u32, and the
> FourCC is then always printed MSByte-first (this is the opposite of
> V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
> allow printing LSByte-first FourCCs stored in host endian order
> (other than the hex form being in character order, not the integer
> value).
> 
> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Geert Uytterhoeven]

Hi Aditya, Hector,

On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
> From: Hector Martin <marcan@marcan.st>
>
> %p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
> it's useful to be able to print generic 4-character codes formatted as
> an integer. Extend it to add format specifiers for printing generic
> 32-bit FourCCs with various endian semantics:
>
> %p4ch   Host byte order
> %p4cn   Network byte order
> %p4cl   Little-endian
> %p4cb   Big-endian
>
> The endianness determines how bytes are interpreted as a u32, and the
> FourCC is then always printed MSByte-first (this is the opposite of
> V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
> allow printing LSByte-first FourCCs stored in host endian order
> (other than the hex form being in character order, not the integer
> value).
>
> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>

Thanks for your patch, which is now commit 1938479b2720ebc0
("lib/vsprintf: Add support for generic FourCCs by extending %p4cc")
in drm-misc-next/

> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -648,6 +648,38 @@ Examples::
>         %p4cc   Y10  little-endian (0x20303159)
>         %p4cc   NV12 big-endian (0xb231564e)
>
> +Generic FourCC code
> +-------------------
> +
> +::
> +       %p4c[hnlb]      gP00 (0x67503030)
> +
> +Print a generic FourCC code, as both ASCII characters and its numerical
> +value as hexadecimal.
> +
> +The generic FourCC code is always printed in the big-endian format,
> +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
> +
> +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
> +endianness is used to load the stored bytes. The data might be interpreted
> +using the host byte order, network byte order, little-endian, or big-endian.
> +
> +Passed by reference.
> +
> +Examples for a little-endian machine, given &(u32)0x67503030::
> +
> +       %p4ch   gP00 (0x67503030)
> +       %p4cn   00Pg (0x30305067)
> +       %p4cl   gP00 (0x67503030)
> +       %p4cb   00Pg (0x30305067)
> +
> +Examples for a big-endian machine, given &(u32)0x67503030::
> +
> +       %p4ch   gP00 (0x67503030)
> +       %p4cn   00Pg (0x30305067)

This doesn't look right to me, as network byte order is big endian?
Note that I didn't check the code.

> +       %p4cl   00Pg (0x30305067)
> +       %p4cb   gP00 (0x67503030)
> +
>  Rust
>  ----
>

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 v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Aditya Garg]

On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
> Hi Aditya, Hector,
> 
> On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
>> From: Hector Martin <marcan@marcan.st>
>>
>> %p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
>> it's useful to be able to print generic 4-character codes formatted as
>> an integer. Extend it to add format specifiers for printing generic
>> 32-bit FourCCs with various endian semantics:
>>
>> %p4ch   Host byte order
>> %p4cn   Network byte order
>> %p4cl   Little-endian
>> %p4cb   Big-endian
>>
>> The endianness determines how bytes are interpreted as a u32, and the
>> FourCC is then always printed MSByte-first (this is the opposite of
>> V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
>> allow printing LSByte-first FourCCs stored in host endian order
>> (other than the hex form being in character order, not the integer
>> value).
>>
>> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Reviewed-by: Petr Mladek <pmladek@suse.com>
>> Tested-by: Petr Mladek <pmladek@suse.com>
>> Signed-off-by: Hector Martin <marcan@marcan.st>
>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> 
> Thanks for your patch, which is now commit 1938479b2720ebc0
> ("lib/vsprintf: Add support for generic FourCCs by extending %p4cc")
> in drm-misc-next/
> 
>> --- a/Documentation/core-api/printk-formats.rst
>> +++ b/Documentation/core-api/printk-formats.rst
>> @@ -648,6 +648,38 @@ Examples::
>>         %p4cc   Y10  little-endian (0x20303159)
>>         %p4cc   NV12 big-endian (0xb231564e)
>>
>> +Generic FourCC code
>> +-------------------
>> +
>> +::
>> +       %p4c[hnlb]      gP00 (0x67503030)
>> +
>> +Print a generic FourCC code, as both ASCII characters and its numerical
>> +value as hexadecimal.
>> +
>> +The generic FourCC code is always printed in the big-endian format,
>> +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
>> +
>> +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
>> +endianness is used to load the stored bytes. The data might be interpreted
>> +using the host byte order, network byte order, little-endian, or big-endian.
>> +
>> +Passed by reference.
>> +
>> +Examples for a little-endian machine, given &(u32)0x67503030::
>> +
>> +       %p4ch   gP00 (0x67503030)
>> +       %p4cn   00Pg (0x30305067)
>> +       %p4cl   gP00 (0x67503030)
>> +       %p4cb   00Pg (0x30305067)
>> +
>> +Examples for a big-endian machine, given &(u32)0x67503030::
>> +
>> +       %p4ch   gP00 (0x67503030)
>> +       %p4cn   00Pg (0x30305067)
> 
> This doesn't look right to me, as network byte order is big endian?
> Note that I didn't check the code.

Originally, it was %p4cr (reverse-endian), but on the request of the maintainers, it was changed to %p4cn.

So here network means reverse of host, not strictly big-endian.

> 
>> +       %p4cl   00Pg (0x30305067)
>> +       %p4cb   gP00 (0x67503030)
>> +
>>  Rust
>>  ----
>>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Geert Uytterhoeven]

Hi Aditya,

CC netdev

On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
> On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
> > On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
> >> From: Hector Martin <marcan@marcan.st>
> >>
> >> %p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
> >> it's useful to be able to print generic 4-character codes formatted as
> >> an integer. Extend it to add format specifiers for printing generic
> >> 32-bit FourCCs with various endian semantics:
> >>
> >> %p4ch   Host byte order
> >> %p4cn   Network byte order
> >> %p4cl   Little-endian
> >> %p4cb   Big-endian
> >>
> >> The endianness determines how bytes are interpreted as a u32, and the
> >> FourCC is then always printed MSByte-first (this is the opposite of
> >> V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
> >> allow printing LSByte-first FourCCs stored in host endian order
> >> (other than the hex form being in character order, not the integer
> >> value).
> >>
> >> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> Reviewed-by: Petr Mladek <pmladek@suse.com>
> >> Tested-by: Petr Mladek <pmladek@suse.com>
> >> Signed-off-by: Hector Martin <marcan@marcan.st>
> >> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> >
> > Thanks for your patch, which is now commit 1938479b2720ebc0
> > ("lib/vsprintf: Add support for generic FourCCs by extending %p4cc")
> > in drm-misc-next/
> >
> >> --- a/Documentation/core-api/printk-formats.rst
> >> +++ b/Documentation/core-api/printk-formats.rst
> >> @@ -648,6 +648,38 @@ Examples::
> >>         %p4cc   Y10  little-endian (0x20303159)
> >>         %p4cc   NV12 big-endian (0xb231564e)
> >>
> >> +Generic FourCC code
> >> +-------------------
> >> +
> >> +::
> >> +       %p4c[hnlb]      gP00 (0x67503030)
> >> +
> >> +Print a generic FourCC code, as both ASCII characters and its numerical
> >> +value as hexadecimal.
> >> +
> >> +The generic FourCC code is always printed in the big-endian format,
> >> +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
> >> +
> >> +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
> >> +endianness is used to load the stored bytes. The data might be interpreted
> >> +using the host byte order, network byte order, little-endian, or big-endian.
> >> +
> >> +Passed by reference.
> >> +
> >> +Examples for a little-endian machine, given &(u32)0x67503030::
> >> +
> >> +       %p4ch   gP00 (0x67503030)
> >> +       %p4cn   00Pg (0x30305067)
> >> +       %p4cl   gP00 (0x67503030)
> >> +       %p4cb   00Pg (0x30305067)
> >> +
> >> +Examples for a big-endian machine, given &(u32)0x67503030::
> >> +
> >> +       %p4ch   gP00 (0x67503030)
> >> +       %p4cn   00Pg (0x30305067)
> >
> > This doesn't look right to me, as network byte order is big endian?
> > Note that I didn't check the code.
>
> Originally, it was %p4cr (reverse-endian), but on the request of the maintainers, it was changed to %p4cn.

Ah, I found it[1]:

| so, it needs more information that this mimics htonl() / ntohl() for
networking.

IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
while %p4ch and %p4cl yield different results on big-endian.

> So here network means reverse of host, not strictly big-endian.

Please don't call it "network byte order" if that does not have the same
meaning as in the network subsystem.

Personally, I like "%p4r" (reverse) more...
(and "%p4ch" might mean human-readable ;-)

[1] https://lore.kernel.org/all/Z8B6DwcRbV-8D8GB@smile.fi.intel.com

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 v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Aditya Garg]

On 22-04-2025 02:13 pm, Geert Uytterhoeven wrote:
> Hi Aditya,
> 
> CC netdev
> 
> On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
>> On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
>>> On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
>>>> From: Hector Martin <marcan@marcan.st>
>>>>
>>>> %p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
>>>> it's useful to be able to print generic 4-character codes formatted as
>>>> an integer. Extend it to add format specifiers for printing generic
>>>> 32-bit FourCCs with various endian semantics:
>>>>
>>>> %p4ch   Host byte order
>>>> %p4cn   Network byte order
>>>> %p4cl   Little-endian
>>>> %p4cb   Big-endian
>>>>
>>>> The endianness determines how bytes are interpreted as a u32, and the
>>>> FourCC is then always printed MSByte-first (this is the opposite of
>>>> V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
>>>> allow printing LSByte-first FourCCs stored in host endian order
>>>> (other than the hex form being in character order, not the integer
>>>> value).
>>>>
>>>> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>> Reviewed-by: Petr Mladek <pmladek@suse.com>
>>>> Tested-by: Petr Mladek <pmladek@suse.com>
>>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>>
>>> Thanks for your patch, which is now commit 1938479b2720ebc0
>>> ("lib/vsprintf: Add support for generic FourCCs by extending %p4cc")
>>> in drm-misc-next/
>>>
>>>> --- a/Documentation/core-api/printk-formats.rst
>>>> +++ b/Documentation/core-api/printk-formats.rst
>>>> @@ -648,6 +648,38 @@ Examples::
>>>>         %p4cc   Y10  little-endian (0x20303159)
>>>>         %p4cc   NV12 big-endian (0xb231564e)
>>>>
>>>> +Generic FourCC code
>>>> +-------------------
>>>> +
>>>> +::
>>>> +       %p4c[hnlb]      gP00 (0x67503030)
>>>> +
>>>> +Print a generic FourCC code, as both ASCII characters and its numerical
>>>> +value as hexadecimal.
>>>> +
>>>> +The generic FourCC code is always printed in the big-endian format,
>>>> +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
>>>> +
>>>> +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
>>>> +endianness is used to load the stored bytes. The data might be interpreted
>>>> +using the host byte order, network byte order, little-endian, or big-endian.
>>>> +
>>>> +Passed by reference.
>>>> +
>>>> +Examples for a little-endian machine, given &(u32)0x67503030::
>>>> +
>>>> +       %p4ch   gP00 (0x67503030)
>>>> +       %p4cn   00Pg (0x30305067)
>>>> +       %p4cl   gP00 (0x67503030)
>>>> +       %p4cb   00Pg (0x30305067)
>>>> +
>>>> +Examples for a big-endian machine, given &(u32)0x67503030::
>>>> +
>>>> +       %p4ch   gP00 (0x67503030)
>>>> +       %p4cn   00Pg (0x30305067)
>>>
>>> This doesn't look right to me, as network byte order is big endian?
>>> Note that I didn't check the code.
>>
>> Originally, it was %p4cr (reverse-endian), but on the request of the maintainers, it was changed to %p4cn.
> 
> Ah, I found it[1]:
> 
> | so, it needs more information that this mimics htonl() / ntohl() for
> networking.
> 
> IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
> while %p4ch and %p4cl yield different results on big-endian.
> 
>> So here network means reverse of host, not strictly big-endian.
> 
> Please don't call it "network byte order" if that does not have the same
> meaning as in the network subsystem.
> 
> Personally, I like "%p4r" (reverse) more...

I share the same view about this. But, we have to respect the maintainers request as well xD.

Still, feel free to send a patch if you want to make this change.

Cheers
Aditya

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Andy Shevchenko]

On Tue, Apr 22, 2025 at 10:43:59AM +0200, Geert Uytterhoeven wrote:
> On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
> > On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
> > > On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:

...

> > Originally, it was %p4cr (reverse-endian), but on the request of the
> > maintainers, it was changed to %p4cn.
> 
> Ah, I found it[1]:
> 
> | so, it needs more information that this mimics htonl() / ntohl() for
> networking.
> 
> IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
> while %p4ch and %p4cl yield different results on big-endian.
> 
> > So here network means reverse of host, not strictly big-endian.
> 
> Please don't call it "network byte order" if that does not have the same
> meaning as in the network subsystem.
> 
> Personally, I like "%p4r" (reverse) more...
> (and "%p4ch" might mean human-readable ;-)

It will confuse the reader. h/r is not very established pair. If you really
wont see h/n, better to drop them completely for now then. Because I'm against
h/r pair.

> [1] https://lore.kernel.org/all/Z8B6DwcRbV-8D8GB@smile.fi.intel.com


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Geert Uytterhoeven]

Hi Andy,

On Tue, 22 Apr 2025 at 12:16, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Tue, Apr 22, 2025 at 10:43:59AM +0200, Geert Uytterhoeven wrote:
> > On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
> > > On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
> > > > On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
>
> ...
>
> > > Originally, it was %p4cr (reverse-endian), but on the request of the
> > > maintainers, it was changed to %p4cn.
> >
> > Ah, I found it[1]:
> >
> > | so, it needs more information that this mimics htonl() / ntohl() for
> > networking.
> >
> > IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
> > while %p4ch and %p4cl yield different results on big-endian.
> >
> > > So here network means reverse of host, not strictly big-endian.
> >
> > Please don't call it "network byte order" if that does not have the same
> > meaning as in the network subsystem.
> >
> > Personally, I like "%p4r" (reverse) more...
> > (and "%p4ch" might mean human-readable ;-)
>
> It will confuse the reader. h/r is not very established pair. If you really
> wont see h/n, better to drop them completely for now then. Because I'm against
> h/r pair.

I am not against h/n in se, but I am against bad/confusing naming.
The big question is: should it print
  (A) the value in network byte order, or
  (B) the reverse of host byte order?

If the answer is (A), I see no real reason to have %p4n, as %p4b prints
the exact same thing.  Moreover, it leaves us without a portable
way to print values in reverse without the caller doing an explicit
__swab32() (which is not compatible with the %p pass-by-pointer
calling convention).

If the answer is (B), "%p4n using network byte order" is bad/confusing
naming.

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 v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Aditya Garg]

On 22-04-2025 04:02 pm, Geert Uytterhoeven wrote:
> Hi Andy,
> 
> On Tue, 22 Apr 2025 at 12:16, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> On Tue, Apr 22, 2025 at 10:43:59AM +0200, Geert Uytterhoeven wrote:
>>> On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
>>>> On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
>>>>> On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
>>
>> ...
>>
>>>> Originally, it was %p4cr (reverse-endian), but on the request of the
>>>> maintainers, it was changed to %p4cn.
>>>
>>> Ah, I found it[1]:
>>>
>>> | so, it needs more information that this mimics htonl() / ntohl() for
>>> networking.
>>>
>>> IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
>>> while %p4ch and %p4cl yield different results on big-endian.
>>>
>>>> So here network means reverse of host, not strictly big-endian.
>>>
>>> Please don't call it "network byte order" if that does not have the same
>>> meaning as in the network subsystem.
>>>
>>> Personally, I like "%p4r" (reverse) more...
>>> (and "%p4ch" might mean human-readable ;-)
>>
>> It will confuse the reader. h/r is not very established pair. If you really
>> wont see h/n, better to drop them completely for now then. Because I'm against
>> h/r pair.
> 
> I am not against h/n in se, but I am against bad/confusing naming.
> The big question is: should it print
>   (A) the value in network byte order, or
>   (B) the reverse of host byte order?
> 
> If the answer is (A), I see no real reason to have %p4n, as %p4b prints
> the exact same thing.  Moreover, it leaves us without a portable
> way to print values in reverse without the caller doing an explicit
> __swab32() (which is not compatible with the %p pass-by-pointer
> calling convention).
> 
> If the answer is (B), "%p4n using network byte order" is bad/confusing
> naming.

The answer is definitely (B). As far as bad/confusing naming is concerned,
I'll let vsprintf maintainers decide. As far as usage is concerned, %p4cl
is used in appletbdrm and %p4ch is used in to be upstreamed soon smc driver
by Asahi Linux.

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Andy Shevchenko]

On Tue, Apr 22, 2025 at 08:45:31PM +0530, Aditya Garg wrote:
> On 22-04-2025 04:02 pm, Geert Uytterhoeven wrote:
> > On Tue, 22 Apr 2025 at 12:16, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:

...

> > I am not against h/n in se, but I am against bad/confusing naming.
> > The big question is: should it print
> >   (A) the value in network byte order, or
> >   (B) the reverse of host byte order?
> > 
> > If the answer is (A), I see no real reason to have %p4n, as %p4b prints
> > the exact same thing.  Moreover, it leaves us without a portable
> > way to print values in reverse without the caller doing an explicit
> > __swab32() (which is not compatible with the %p pass-by-pointer
> > calling convention).
> > 
> > If the answer is (B), "%p4n using network byte order" is bad/confusing
> > naming.
> 
> The answer is definitely (B). As far as bad/confusing naming is concerned,
> I'll let vsprintf maintainers decide. As far as usage is concerned, %p4cl
> is used in appletbdrm and %p4ch is used in to be upstreamed soon smc driver
> by Asahi Linux.

Can it use %p4cb? Or in another word,
why does it require "host" representation?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Aditya Garg]

On 22-04-2025 08:52 pm, Andy Shevchenko wrote:
> On Tue, Apr 22, 2025 at 08:45:31PM +0530, Aditya Garg wrote:
>> On 22-04-2025 04:02 pm, Geert Uytterhoeven wrote:
>>> On Tue, 22 Apr 2025 at 12:16, Andy Shevchenko
>>> <andriy.shevchenko@linux.intel.com> wrote:
> 
> ...
> 
>>> I am not against h/n in se, but I am against bad/confusing naming.
>>> The big question is: should it print
>>>   (A) the value in network byte order, or
>>>   (B) the reverse of host byte order?
>>>
>>> If the answer is (A), I see no real reason to have %p4n, as %p4b prints
>>> the exact same thing.  Moreover, it leaves us without a portable
>>> way to print values in reverse without the caller doing an explicit
>>> __swab32() (which is not compatible with the %p pass-by-pointer
>>> calling convention).
>>>
>>> If the answer is (B), "%p4n using network byte order" is bad/confusing
>>> naming.
>>
>> The answer is definitely (B). As far as bad/confusing naming is concerned,
>> I'll let vsprintf maintainers decide. As far as usage is concerned, %p4cl
>> is used in appletbdrm and %p4ch is used in to be upstreamed soon smc driver
>> by Asahi Linux.
> 
> Can it use %p4cb? Or in another word,
> why does it require "host" representation?

Sven might know why. He is already CCed.

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Andy Shevchenko]

On Tue, Apr 22, 2025 at 12:32:42PM +0200, Geert Uytterhoeven wrote:
> On Tue, 22 Apr 2025 at 12:16, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Apr 22, 2025 at 10:43:59AM +0200, Geert Uytterhoeven wrote:
> > > On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
> > > > On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
> > > > > On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:

...

> > > > Originally, it was %p4cr (reverse-endian), but on the request of the
> > > > maintainers, it was changed to %p4cn.
> > >
> > > Ah, I found it[1]:
> > >
> > > | so, it needs more information that this mimics htonl() / ntohl() for
> > > networking.
> > >
> > > IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
> > > while %p4ch and %p4cl yield different results on big-endian.
> > >
> > > > So here network means reverse of host, not strictly big-endian.
> > >
> > > Please don't call it "network byte order" if that does not have the same
> > > meaning as in the network subsystem.
> > >
> > > Personally, I like "%p4r" (reverse) more...
> > > (and "%p4ch" might mean human-readable ;-)
> >
> > It will confuse the reader. h/r is not very established pair. If you really
> > wont see h/n, better to drop them completely for now then. Because I'm against
> > h/r pair.
> 
> I am not against h/n in se, but I am against bad/confusing naming.
> The big question is: should it print
>   (A) the value in network byte order, or
>   (B) the reverse of host byte order?
> 
> If the answer is (A), I see no real reason to have %p4n, as %p4b prints
> the exact same thing.  Moreover, it leaves us without a portable
> way to print values in reverse without the caller doing an explicit
> __swab32() (which is not compatible with the %p pass-by-pointer
> calling convention).
> 
> If the answer is (B), "%p4n using network byte order" is bad/confusing
> naming.

Other %p extensions that have R/r for "reversed" do not have any H/h part for
"host". That's why if we want reversed, than don't use the host, it should be
default. As I said, I think the best is to remove these for now,

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Petr Mladek]

On Tue 2025-04-22 10:43:59, Geert Uytterhoeven wrote:
> Hi Aditya,
> 
> CC netdev
> 
> On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
> > On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
> > > On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
> > >> From: Hector Martin <marcan@marcan.st>
> > >>
> > >> %p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
> > >> it's useful to be able to print generic 4-character codes formatted as
> > >> an integer. Extend it to add format specifiers for printing generic
> > >> 32-bit FourCCs with various endian semantics:
> > >>
> > >> %p4ch   Host byte order
> > >> %p4cn   Network byte order
> > >> %p4cl   Little-endian
> > >> %p4cb   Big-endian
> > >>
> > >> The endianness determines how bytes are interpreted as a u32, and the
> > >> FourCC is then always printed MSByte-first (this is the opposite of
> > >> V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
> > >> allow printing LSByte-first FourCCs stored in host endian order
> > >> (other than the hex form being in character order, not the integer
> > >> value).
> > >>
> > >> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >> Reviewed-by: Petr Mladek <pmladek@suse.com>
> > >> Tested-by: Petr Mladek <pmladek@suse.com>
> > >> Signed-off-by: Hector Martin <marcan@marcan.st>
> > >> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> > >
> > > Thanks for your patch, which is now commit 1938479b2720ebc0
> > > ("lib/vsprintf: Add support for generic FourCCs by extending %p4cc")
> > > in drm-misc-next/
> > >
> > >> --- a/Documentation/core-api/printk-formats.rst
> > >> +++ b/Documentation/core-api/printk-formats.rst
> > >> @@ -648,6 +648,38 @@ Examples::
> > >>         %p4cc   Y10  little-endian (0x20303159)
> > >>         %p4cc   NV12 big-endian (0xb231564e)
> > >>
> > >> +Generic FourCC code
> > >> +-------------------
> > >> +
> > >> +::
> > >> +       %p4c[hnlb]      gP00 (0x67503030)
> > >> +
> > >> +Print a generic FourCC code, as both ASCII characters and its numerical
> > >> +value as hexadecimal.
> > >> +
> > >> +The generic FourCC code is always printed in the big-endian format,
> > >> +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
> > >> +
> > >> +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
> > >> +endianness is used to load the stored bytes. The data might be interpreted
> > >> +using the host byte order, network byte order, little-endian, or big-endian.
> > >> +
> > >> +Passed by reference.
> > >> +
> > >> +Examples for a little-endian machine, given &(u32)0x67503030::
> > >> +
> > >> +       %p4ch   gP00 (0x67503030)
> > >> +       %p4cn   00Pg (0x30305067)
> > >> +       %p4cl   gP00 (0x67503030)
> > >> +       %p4cb   00Pg (0x30305067)
> > >> +
> > >> +Examples for a big-endian machine, given &(u32)0x67503030::
> > >> +
> > >> +       %p4ch   gP00 (0x67503030)
> > >> +       %p4cn   00Pg (0x30305067)
> > >
> > > This doesn't look right to me, as network byte order is big endian?
> > > Note that I didn't check the code.
> >
> > Originally, it was %p4cr (reverse-endian), but on the request of the maintainers, it was changed to %p4cn.
> 
> Ah, I found it[1]:
> 
> | so, it needs more information that this mimics htonl() / ntohl() for
> networking.
> 
> IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
> while %p4ch and %p4cl yield different results on big-endian.
> 
> > So here network means reverse of host, not strictly big-endian.
> 
> Please don't call it "network byte order" if that does not have the same
> meaning as in the network subsystem.
> 
> Personally, I like "%p4r" (reverse) more...
> (and "%p4ch" might mean human-readable ;-)
> 
> [1] https://lore.kernel.org/all/Z8B6DwcRbV-8D8GB@smile.fi.intel.com

I have to admit that I was always a bit confused by the meaning of the
new modifiers. And I did give up at some point and decided to do not
block the patch when it made sense to others.

But I have to agree with Geert here. The current behavior of %p4ch
is confusing on big endian system. I would expect that it does not
revert the ordering.

Well, I still think that people might find all 4 variants useful.
Andy does not like "r". What about "hR"? It is inspired by
the existing %pmR.

I tried to implement it and the complexity of the code is similar:

From f6aa2213cec9b9d25c0506e3112f32e90a18aa7f Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Wed, 23 Apr 2025 15:02:10 +0200
Subject: [PATCH] vsprintf: Use %p4chR instead of %p4cn for reading data in
 reversed host ordering

The generic FourCC format always prints the data using the big endian
order. It is generic because it allows to read the data using a custom
ordering.

The current code uses "n" for reading data in the reverse host ordering.
It makes the 4 variants [hnbl] consistent with the generic printing
of IPv4 addresses.

Unfortunately, it creates confusion on big endian systems. For example,
it shows the data &(u32)0x67503030 as

	%p4cn	00Pg (0x30305067)

But people expect that the ordering stays the same. The network ordering
is a big-endian ordering.

The problem is that the semantic is not the same. The modifiers affect
the output ordering of IPv4 addresses while they affect the reading order
in case of FourCC code.

Avoid the confusion by replacing the "n" modifier with "hR", aka
reverse host ordering.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 Documentation/core-api/printk-formats.rst | 10 +++++-----
 lib/tests/printf_kunit.c                  |  2 +-
 lib/vsprintf.c                            | 11 ++++++++---
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 125fd0397510..f531873bb3c9 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -652,7 +652,7 @@ Generic FourCC code
 -------------------
 
 ::
-	%p4c[hnlb]	gP00 (0x67503030)
+	%p4c[h[R]lb]	gP00 (0x67503030)
 
 Print a generic FourCC code, as both ASCII characters and its numerical
 value as hexadecimal.
@@ -660,23 +660,23 @@ value as hexadecimal.
 The generic FourCC code is always printed in the big-endian format,
 the most significant byte first. This is the opposite of V4L/DRM FourCCs.
 
-The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
+The additional ``h``, ``hR``, ``l``, and ``b`` specifiers define what
 endianness is used to load the stored bytes. The data might be interpreted
-using the host byte order, network byte order, little-endian, or big-endian.
+using the host, reversed host byte order, little-endian, or big-endian.
 
 Passed by reference.
 
 Examples for a little-endian machine, given &(u32)0x67503030::
 
 	%p4ch	gP00 (0x67503030)
-	%p4cn	00Pg (0x30305067)
+	%p4chR	00Pg (0x30305067)
 	%p4cl	gP00 (0x67503030)
 	%p4cb	00Pg (0x30305067)
 
 Examples for a big-endian machine, given &(u32)0x67503030::
 
 	%p4ch	gP00 (0x67503030)
-	%p4cn	00Pg (0x30305067)
+	%p4chR	00Pg (0x30305067)
 	%p4cl	00Pg (0x30305067)
 	%p4cb	gP00 (0x67503030)
 
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
index b1fa0dcea52f..b8a4b5006f9c 100644
--- a/lib/tests/printf_kunit.c
+++ b/lib/tests/printf_kunit.c
@@ -738,7 +738,7 @@ static void fourcc_pointer(struct kunit *kunittest)
 
 	fourcc_pointer_test(kunittest, try_cc, ARRAY_SIZE(try_cc), "%p4cc");
 	fourcc_pointer_test(kunittest, try_ch, ARRAY_SIZE(try_ch), "%p4ch");
-	fourcc_pointer_test(kunittest, try_cn, ARRAY_SIZE(try_cn), "%p4cn");
+	fourcc_pointer_test(kunittest, try_cn, ARRAY_SIZE(try_cn), "%p4chR");
 	fourcc_pointer_test(kunittest, try_cl, ARRAY_SIZE(try_cl), "%p4cl");
 	fourcc_pointer_test(kunittest, try_cb, ARRAY_SIZE(try_cb), "%p4cb");
 }
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 2c5de4216415..34587b2dbdb1 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1804,9 +1804,8 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
 	orig = get_unaligned(fourcc);
 	switch (fmt[2]) {
 	case 'h':
-		break;
-	case 'n':
-		orig = swab32(orig);
+		if (fmt[3] == 'R')
+			orig = swab32(orig);
 		break;
 	case 'l':
 		orig = (__force u32)cpu_to_le32(orig);
@@ -2396,6 +2395,12 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
  *       read the documentation (path below) first.
  * - 'NF' For a netdev_features_t
  * - '4cc' V4L2 or DRM FourCC code, with endianness and raw numerical value.
+ * - '4c[h[R]lb]' For generic FourCC code with raw numerical value. Both are
+ *	 displayed in the big-endian format. This is the opposite of V4L2 or
+ *	 DRM FourCCs.
+ *	 The additional specifiers define what endianness is used to load
+ *	 the stored bytes. The data might be interpreted using the host,
+ *	 reversed host byte order, little-endian, or big-endian.
  * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
  *            a certain separator (' ' by default):
  *              C colon
-- 
2.49.0

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Aditya Garg]

> On 23 Apr 2025, at 7:09 PM, Petr Mladek <pmladek@suse.com> wrote:
> 
> On Tue 2025-04-22 10:43:59, Geert Uytterhoeven wrote:
>> Hi Aditya,
>> 
>> CC netdev
>> 
>>> On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
>>> On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
>>>> On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
>>>>> From: Hector Martin <marcan@marcan.st>
>>>>> 
>>>>> %p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
>>>>> it's useful to be able to print generic 4-character codes formatted as
>>>>> an integer. Extend it to add format specifiers for printing generic
>>>>> 32-bit FourCCs with various endian semantics:
>>>>> 
>>>>> %p4ch   Host byte order
>>>>> %p4cn   Network byte order
>>>>> %p4cl   Little-endian
>>>>> %p4cb   Big-endian
>>>>> 
>>>>> The endianness determines how bytes are interpreted as a u32, and the
>>>>> FourCC is then always printed MSByte-first (this is the opposite of
>>>>> V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
>>>>> allow printing LSByte-first FourCCs stored in host endian order
>>>>> (other than the hex form being in character order, not the integer
>>>>> value).
>>>>> 
>>>>> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>>>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>>> Reviewed-by: Petr Mladek <pmladek@suse.com>
>>>>> Tested-by: Petr Mladek <pmladek@suse.com>
>>>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>>> 
>>>> Thanks for your patch, which is now commit 1938479b2720ebc0
>>>> ("lib/vsprintf: Add support for generic FourCCs by extending %p4cc")
>>>> in drm-misc-next/
>>>> 
>>>>> --- a/Documentation/core-api/printk-formats.rst
>>>>> +++ b/Documentation/core-api/printk-formats.rst
>>>>> @@ -648,6 +648,38 @@ Examples::
>>>>>        %p4cc   Y10  little-endian (0x20303159)
>>>>>        %p4cc   NV12 big-endian (0xb231564e)
>>>>> 
>>>>> +Generic FourCC code
>>>>> +-------------------
>>>>> +
>>>>> +::
>>>>> +       %p4c[hnlb]      gP00 (0x67503030)
>>>>> +
>>>>> +Print a generic FourCC code, as both ASCII characters and its numerical
>>>>> +value as hexadecimal.
>>>>> +
>>>>> +The generic FourCC code is always printed in the big-endian format,
>>>>> +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
>>>>> +
>>>>> +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
>>>>> +endianness is used to load the stored bytes. The data might be interpreted
>>>>> +using the host byte order, network byte order, little-endian, or big-endian.
>>>>> +
>>>>> +Passed by reference.
>>>>> +
>>>>> +Examples for a little-endian machine, given &(u32)0x67503030::
>>>>> +
>>>>> +       %p4ch   gP00 (0x67503030)
>>>>> +       %p4cn   00Pg (0x30305067)
>>>>> +       %p4cl   gP00 (0x67503030)
>>>>> +       %p4cb   00Pg (0x30305067)
>>>>> +
>>>>> +Examples for a big-endian machine, given &(u32)0x67503030::
>>>>> +
>>>>> +       %p4ch   gP00 (0x67503030)
>>>>> +       %p4cn   00Pg (0x30305067)
>>>> 
>>>> This doesn't look right to me, as network byte order is big endian?
>>>> Note that I didn't check the code.
>>> 
>>> Originally, it was %p4cr (reverse-endian), but on the request of the maintainers, it was changed to %p4cn.
>> 
>> Ah, I found it[1]:
>> 
>> | so, it needs more information that this mimics htonl() / ntohl() for
>> networking.
>> 
>> IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
>> while %p4ch and %p4cl yield different results on big-endian.
>> 
>>> So here network means reverse of host, not strictly big-endian.
>> 
>> Please don't call it "network byte order" if that does not have the same
>> meaning as in the network subsystem.
>> 
>> Personally, I like "%p4r" (reverse) more...
>> (and "%p4ch" might mean human-readable ;-)
>> 
>> [1] https://lore.kernel.org/all/Z8B6DwcRbV-8D8GB@smile.fi.intel.com
> 
> I have to admit that I was always a bit confused by the meaning of the
> new modifiers. And I did give up at some point and decided to do not
> block the patch when it made sense to others.
> 
> But I have to agree with Geert here. The current behavior of %p4ch
> is confusing on big endian system. I would expect that it does not
> revert the ordering.
> 
> Well, I still think that people might find all 4 variants useful.
> Andy does not like "r". What about "hR"? It is inspired by
> the existing %pmR.
> 
> I tried to implement it and the complexity of the code is similar:
> 
> From f6aa2213cec9b9d25c0506e3112f32e90a18aa7f Mon Sep 17 00:00:00 2001
> From: Petr Mladek <pmladek@suse.com>
> Date: Wed, 23 Apr 2025 15:02:10 +0200
> Subject: [PATCH] vsprintf: Use %p4chR instead of %p4cn for reading data in
> reversed host ordering
> 
> The generic FourCC format always prints the data using the big endian
> order. It is generic because it allows to read the data using a custom
> ordering.
> 
> The current code uses "n" for reading data in the reverse host ordering.
> It makes the 4 variants [hnbl] consistent with the generic printing
> of IPv4 addresses.
> 
> Unfortunately, it creates confusion on big endian systems. For example,
> it shows the data &(u32)0x67503030 as
> 
>    %p4cn    00Pg (0x30305067)
> 
> But people expect that the ordering stays the same. The network ordering
> is a big-endian ordering.
> 
> The problem is that the semantic is not the same. The modifiers affect
> the output ordering of IPv4 addresses while they affect the reading order
> in case of FourCC code.
> 
> Avoid the confusion by replacing the "n" modifier with "hR", aka
> reverse host ordering.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
> Documentation/core-api/printk-formats.rst | 10 +++++-----
> lib/tests/printf_kunit.c                  |  2 +-
> lib/vsprintf.c                            | 11 ++++++++---
> 3 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 125fd0397510..f531873bb3c9 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -652,7 +652,7 @@ Generic FourCC code
> -------------------
> 
> ::
> -    %p4c[hnlb]    gP00 (0x67503030)
> +    %p4c[h[R]lb]    gP00 (0x67503030)
> 
> Print a generic FourCC code, as both ASCII characters and its numerical
> value as hexadecimal.
> @@ -660,23 +660,23 @@ value as hexadecimal.
> The generic FourCC code is always printed in the big-endian format,
> the most significant byte first. This is the opposite of V4L/DRM FourCCs.
> 
> -The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
> +The additional ``h``, ``hR``, ``l``, and ``b`` specifiers define what
> endianness is used to load the stored bytes. The data might be interpreted
> -using the host byte order, network byte order, little-endian, or big-endian.
> +using the host, reversed host byte order, little-endian, or big-endian.
> 
> Passed by reference.
> 
> Examples for a little-endian machine, given &(u32)0x67503030::
> 
>    %p4ch    gP00 (0x67503030)
> -    %p4cn    00Pg (0x30305067)
> +    %p4chR    00Pg (0x30305067)
>    %p4cl    gP00 (0x67503030)
>    %p4cb    00Pg (0x30305067)
> 
> Examples for a big-endian machine, given &(u32)0x67503030::
> 
>    %p4ch    gP00 (0x67503030)
> -    %p4cn    00Pg (0x30305067)
> +    %p4chR    00Pg (0x30305067)
>    %p4cl    00Pg (0x30305067)
>    %p4cb    gP00 (0x67503030)
> 
> diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
> index b1fa0dcea52f..b8a4b5006f9c 100644
> --- a/lib/tests/printf_kunit.c
> +++ b/lib/tests/printf_kunit.c
> @@ -738,7 +738,7 @@ static void fourcc_pointer(struct kunit *kunittest)
> 
>    fourcc_pointer_test(kunittest, try_cc, ARRAY_SIZE(try_cc), "%p4cc");
>    fourcc_pointer_test(kunittest, try_ch, ARRAY_SIZE(try_ch), "%p4ch");
> -    fourcc_pointer_test(kunittest, try_cn, ARRAY_SIZE(try_cn), "%p4cn");
> +    fourcc_pointer_test(kunittest, try_cn, ARRAY_SIZE(try_cn), "%p4chR");

Maybe rename try_cn to try_chR?

In any case, it looks good to me.

>    fourcc_pointer_test(kunittest, try_cl, ARRAY_SIZE(try_cl), "%p4cl");
>    fourcc_pointer_test(kunittest, try_cb, ARRAY_SIZE(try_cb), "%p4cb");
> }
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 2c5de4216415..34587b2dbdb1 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1804,9 +1804,8 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
>    orig = get_unaligned(fourcc);
>    switch (fmt[2]) {
>    case 'h':
> -        break;
> -    case 'n':
> -        orig = swab32(orig);
> +        if (fmt[3] == 'R')
> +            orig = swab32(orig);
>        break;
>    case 'l':
>        orig = (__force u32)cpu_to_le32(orig);
> @@ -2396,6 +2395,12 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
>  *       read the documentation (path below) first.
>  * - 'NF' For a netdev_features_t
>  * - '4cc' V4L2 or DRM FourCC code, with endianness and raw numerical value.
> + * - '4c[h[R]lb]' For generic FourCC code with raw numerical value. Both are
> + *     displayed in the big-endian format. This is the opposite of V4L2 or
> + *     DRM FourCCs.
> + *     The additional specifiers define what endianness is used to load
> + *     the stored bytes. The data might be interpreted using the host,
> + *     reversed host byte order, little-endian, or big-endian.
>  * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
>  *            a certain separator (' ' by default):
>  *              C colon
> --
> 2.49.0
> 

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Geert Uytterhoeven]

Hi Petr,

On Wed, 23 Apr 2025 at 15:39, Petr Mladek <pmladek@suse.com> wrote:
> On Tue 2025-04-22 10:43:59, Geert Uytterhoeven wrote:
> > On Tue, 22 Apr 2025 at 10:30, Aditya Garg <gargaditya08@live.com> wrote:
> > > On 22-04-2025 01:37 pm, Geert Uytterhoeven wrote:
> > > > On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
> > > >> From: Hector Martin <marcan@marcan.st>
> > > >>
> > > >> %p4cc is designed for DRM/V4L2 FourCCs with their specific quirks, but
> > > >> it's useful to be able to print generic 4-character codes formatted as
> > > >> an integer. Extend it to add format specifiers for printing generic
> > > >> 32-bit FourCCs with various endian semantics:
> > > >>
> > > >> %p4ch   Host byte order
> > > >> %p4cn   Network byte order
> > > >> %p4cl   Little-endian
> > > >> %p4cb   Big-endian
> > > >>
> > > >> The endianness determines how bytes are interpreted as a u32, and the
> > > >> FourCC is then always printed MSByte-first (this is the opposite of
> > > >> V4L/DRM FourCCs). This covers most practical cases, e.g. %p4cn would
> > > >> allow printing LSByte-first FourCCs stored in host endian order
> > > >> (other than the hex form being in character order, not the integer
> > > >> value).
> > > >>
> > > >> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > > >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > >> Reviewed-by: Petr Mladek <pmladek@suse.com>
> > > >> Tested-by: Petr Mladek <pmladek@suse.com>
> > > >> Signed-off-by: Hector Martin <marcan@marcan.st>
> > > >> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> > > >
> > > > Thanks for your patch, which is now commit 1938479b2720ebc0
> > > > ("lib/vsprintf: Add support for generic FourCCs by extending %p4cc")
> > > > in drm-misc-next/
> > > >
> > > >> --- a/Documentation/core-api/printk-formats.rst
> > > >> +++ b/Documentation/core-api/printk-formats.rst
> > > >> @@ -648,6 +648,38 @@ Examples::
> > > >>         %p4cc   Y10  little-endian (0x20303159)
> > > >>         %p4cc   NV12 big-endian (0xb231564e)
> > > >>
> > > >> +Generic FourCC code
> > > >> +-------------------
> > > >> +
> > > >> +::
> > > >> +       %p4c[hnlb]      gP00 (0x67503030)
> > > >> +
> > > >> +Print a generic FourCC code, as both ASCII characters and its numerical
> > > >> +value as hexadecimal.
> > > >> +
> > > >> +The generic FourCC code is always printed in the big-endian format,
> > > >> +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
> > > >> +
> > > >> +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
> > > >> +endianness is used to load the stored bytes. The data might be interpreted
> > > >> +using the host byte order, network byte order, little-endian, or big-endian.
> > > >> +
> > > >> +Passed by reference.
> > > >> +
> > > >> +Examples for a little-endian machine, given &(u32)0x67503030::
> > > >> +
> > > >> +       %p4ch   gP00 (0x67503030)
> > > >> +       %p4cn   00Pg (0x30305067)
> > > >> +       %p4cl   gP00 (0x67503030)
> > > >> +       %p4cb   00Pg (0x30305067)
> > > >> +
> > > >> +Examples for a big-endian machine, given &(u32)0x67503030::
> > > >> +
> > > >> +       %p4ch   gP00 (0x67503030)
> > > >> +       %p4cn   00Pg (0x30305067)
> > > >
> > > > This doesn't look right to me, as network byte order is big endian?
> > > > Note that I didn't check the code.
> > >
> > > Originally, it was %p4cr (reverse-endian), but on the request of the maintainers, it was changed to %p4cn.
> >
> > Ah, I found it[1]:
> >
> > | so, it needs more information that this mimics htonl() / ntohl() for
> > networking.
> >
> > IMHO this does not mimic htonl(), as htonl() is a no-op on big-endian.
> > while %p4ch and %p4cl yield different results on big-endian.
> >
> > > So here network means reverse of host, not strictly big-endian.
> >
> > Please don't call it "network byte order" if that does not have the same
> > meaning as in the network subsystem.
> >
> > Personally, I like "%p4r" (reverse) more...
> > (and "%p4ch" might mean human-readable ;-)
> >
> > [1] https://lore.kernel.org/all/Z8B6DwcRbV-8D8GB@smile.fi.intel.com
>
> I have to admit that I was always a bit confused by the meaning of the
> new modifiers. And I did give up at some point and decided to do not
> block the patch when it made sense to others.
>
> But I have to agree with Geert here. The current behavior of %p4ch
> is confusing on big endian system. I would expect that it does not
> revert the ordering.
>
> Well, I still think that people might find all 4 variants useful.
> Andy does not like "r". What about "hR"? It is inspired by
> the existing %pmR.

I am not a fan of complicating the format specifier even more by adding
more characters...  But seeing %pmR, I have to admit it does make sense.

> The problem is that the semantic is not the same. The modifiers affect
> the output ordering of IPv4 addresses while they affect the reading order
> in case of FourCC code.

Note that for IPv4 addresses we have %pI4, which BTW also takes [hnbl]
modifiers.

> Avoid the confusion by replacing the "n" modifier with "hR", aka
> reverse host ordering.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Thanks, LGTM!

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 v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Andy Shevchenko]

On Wed, Apr 23, 2025 at 04:50:02PM +0200, Geert Uytterhoeven wrote:
> On Wed, 23 Apr 2025 at 15:39, Petr Mladek <pmladek@suse.com> wrote:
> > On Tue 2025-04-22 10:43:59, Geert Uytterhoeven wrote:

...

> > The problem is that the semantic is not the same. The modifiers affect
> > the output ordering of IPv4 addresses while they affect the reading order
> > in case of FourCC code.
> 
> Note that for IPv4 addresses we have %pI4, which BTW also takes [hnbl]
> modifiers.

Ouch, now I think I understand your complain. You mean that the behaviour of
h/n here is different to what it is for IPv4 case?

> > Avoid the confusion by replacing the "n" modifier with "hR", aka
> > reverse host ordering.

Not ideal, but better than 'h'ost / 'r'everse pair. Not giving a tag and not
objecting either if there is a consensus.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Geert Uytterhoeven]

Hi Andy,

On Wed, 23 Apr 2025 at 18:30, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Wed, Apr 23, 2025 at 04:50:02PM +0200, Geert Uytterhoeven wrote:
> > On Wed, 23 Apr 2025 at 15:39, Petr Mladek <pmladek@suse.com> wrote:
> > > On Tue 2025-04-22 10:43:59, Geert Uytterhoeven wrote:
>
> ...
>
> > > The problem is that the semantic is not the same. The modifiers affect
> > > the output ordering of IPv4 addresses while they affect the reading order
> > > in case of FourCC code.
> >
> > Note that for IPv4 addresses we have %pI4, which BTW also takes [hnbl]
> > modifiers.
>
> Ouch, now I think I understand your complain. You mean that the behaviour of
> h/n here is different to what it is for IPv4 case?

Indeed. "%pI4n" byte-swaps on little-endian, but not on big-endian
(remember, network byte-order _is_ big-endian), while "%p4cn" swaps
everywhere.

> > > Avoid the confusion by replacing the "n" modifier with "hR", aka
> > > reverse host ordering.
>
> Not ideal, but better than 'h'ost / 'r'everse pair. Not giving a tag and not
> objecting either if there is a consensus.

That is worth as much as my LGTM ;-)

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 v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Andy Shevchenko]

On Tue, Apr 22, 2025 at 10:07:33AM +0200, Geert Uytterhoeven wrote:
> On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:

...

> > +Generic FourCC code
> > +-------------------
> > +
> > +::
> > +       %p4c[hnlb]      gP00 (0x67503030)
> > +
> > +Print a generic FourCC code, as both ASCII characters and its numerical
> > +value as hexadecimal.
> > +
> > +The generic FourCC code is always printed in the big-endian format,
> > +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
> > +
> > +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
> > +endianness is used to load the stored bytes. The data might be interpreted
> > +using the host byte order, network byte order, little-endian, or big-endian.
> > +
> > +Passed by reference.
> > +
> > +Examples for a little-endian machine, given &(u32)0x67503030::
> > +
> > +       %p4ch   gP00 (0x67503030)
> > +       %p4cn   00Pg (0x30305067)
> > +       %p4cl   gP00 (0x67503030)
> > +       %p4cb   00Pg (0x30305067)
> > +
> > +Examples for a big-endian machine, given &(u32)0x67503030::
> > +
> > +       %p4ch   gP00 (0x67503030)
> > +       %p4cn   00Pg (0x30305067)
> 
> This doesn't look right to me, as network byte order is big endian?
> Note that I didn't check the code.

Yes, network is big endian and this seems right to me. What is the confusion?

> > +       %p4cl   00Pg (0x30305067)
> > +       %p4cb   gP00 (0x67503030)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc

[Geert Uytterhoeven]

Hi Andy,

On Tue, 22 Apr 2025 at 12:12, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Tue, Apr 22, 2025 at 10:07:33AM +0200, Geert Uytterhoeven wrote:
> > On Tue, 8 Apr 2025 at 08:48, Aditya Garg <gargaditya08@live.com> wrote:
>
> ...
>
> > > +Generic FourCC code
> > > +-------------------
> > > +
> > > +::
> > > +       %p4c[hnlb]      gP00 (0x67503030)
> > > +
> > > +Print a generic FourCC code, as both ASCII characters and its numerical
> > > +value as hexadecimal.
> > > +
> > > +The generic FourCC code is always printed in the big-endian format,
> > > +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
> > > +
> > > +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
> > > +endianness is used to load the stored bytes. The data might be interpreted
> > > +using the host byte order, network byte order, little-endian, or big-endian.
> > > +
> > > +Passed by reference.
> > > +
> > > +Examples for a little-endian machine, given &(u32)0x67503030::
> > > +
> > > +       %p4ch   gP00 (0x67503030)
> > > +       %p4cn   00Pg (0x30305067)
> > > +       %p4cl   gP00 (0x67503030)
> > > +       %p4cb   00Pg (0x30305067)
> > > +
> > > +Examples for a big-endian machine, given &(u32)0x67503030::
> > > +
> > > +       %p4ch   gP00 (0x67503030)
> > > +       %p4cn   00Pg (0x30305067)
> >
> > This doesn't look right to me, as network byte order is big endian?
> > Note that I didn't check the code.
>
> Yes, network is big endian and this seems right to me. What is the confusion?

On a big-endian machine, it should print 0x67503030, like the host
or explicit big-endian output.

> > > +       %p4cl   00Pg (0x30305067)
> > > +       %p4cb   gP00 (0x67503030)

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

[PATCH v4 2/3] printf: add tests for generic FourCCs

[Aditya Garg]

From: Aditya Garg <gargaditya08@live.com>

This patch adds support for kunit tests of generic 32-bit FourCCs added to
vsprintf.

Acked-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 lib/tests/printf_kunit.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
index 2c9f6170b..b1fa0dcea 100644
--- a/lib/tests/printf_kunit.c
+++ b/lib/tests/printf_kunit.c
@@ -701,21 +701,46 @@ static void fwnode_pointer(struct kunit *kunittest)
 	software_node_unregister_node_group(group);
 }
 
+struct fourcc_struct {
+	u32 code;
+	const char *str;
+};
+
+static void fourcc_pointer_test(struct kunit *kunittest, const struct fourcc_struct *fc,
+				size_t n, const char *fmt)
+{
+	size_t i;
+
+	for (i = 0; i < n; i++)
+		test(fc[i].str, fmt, &fc[i].code);
+}
+
 static void fourcc_pointer(struct kunit *kunittest)
 {
-	struct {
-		u32 code;
-		char *str;
-	} const try[] = {
+	static const struct fourcc_struct try_cc[] = {
 		{ 0x3231564e, "NV12 little-endian (0x3231564e)", },
 		{ 0xb231564e, "NV12 big-endian (0xb231564e)", },
 		{ 0x10111213, ".... little-endian (0x10111213)", },
 		{ 0x20303159, "Y10  little-endian (0x20303159)", },
 	};
-	unsigned int i;
+	static const struct fourcc_struct try_ch[] = {
+		{ 0x41424344, "ABCD (0x41424344)", },
+	};
+	static const struct fourcc_struct try_cn[] = {
+		{ 0x41424344, "DCBA (0x44434241)", },
+	};
+	static const struct fourcc_struct try_cl[] = {
+		{ (__force u32)cpu_to_le32(0x41424344), "ABCD (0x41424344)", },
+	};
+	static const struct fourcc_struct try_cb[] = {
+		{ (__force u32)cpu_to_be32(0x41424344), "ABCD (0x41424344)", },
+	};
 
-	for (i = 0; i < ARRAY_SIZE(try); i++)
-		test(try[i].str, "%p4cc", &try[i].code);
+	fourcc_pointer_test(kunittest, try_cc, ARRAY_SIZE(try_cc), "%p4cc");
+	fourcc_pointer_test(kunittest, try_ch, ARRAY_SIZE(try_ch), "%p4ch");
+	fourcc_pointer_test(kunittest, try_cn, ARRAY_SIZE(try_cn), "%p4cn");
+	fourcc_pointer_test(kunittest, try_cl, ARRAY_SIZE(try_cl), "%p4cl");
+	fourcc_pointer_test(kunittest, try_cb, ARRAY_SIZE(try_cb), "%p4cb");
 }
 
 static void
-- 
2.43.0

Re: [PATCH v4 2/3] printf: add tests for generic FourCCs

[Kees Cook]

On Tue, Apr 08, 2025 at 12:18:32PM +0530, Aditya Garg wrote:
> From: Aditya Garg <gargaditya08@live.com>
> 
> This patch adds support for kunit tests of generic 32-bit FourCCs added to
> vsprintf.
> 
> Acked-by: Tamir Duberstein <tamird@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Andy Shevchenko]

On Tue, Apr 08, 2025 at 12:17:13PM +0530, Aditya Garg wrote:
> The vsprint patch was originally being sent as a seperate patch [1], and
> I was waiting it to be taken up. But as suggested by Petr, I'm sending
> them via DRM.

You need to do something about your tools, really.
Now it's patch 3 threaded to patch 1, while the rest, including cover letter,
seems okay.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Aditya Garg]

> On 8 Apr 2025, at 2:11 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> On Tue, Apr 08, 2025 at 12:17:13PM +0530, Aditya Garg wrote:
>> The vsprint patch was originally being sent as a seperate patch [1], and
>> I was waiting it to be taken up. But as suggested by Petr, I'm sending
>> them via DRM.
> 
> You need to do something about your tools, really.

Uhh, I'll just revert to the tried and tested macOS mail.

Although I don't think a resend is necessary here now.
> Now it's patch 3 threaded to patch 1, while the rest, including cover letter,
> seems okay.
> 
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Andy Shevchenko]

On Tue, Apr 08, 2025 at 08:52:10AM +0000, Aditya Garg wrote:
> > On 8 Apr 2025, at 2:11 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Apr 08, 2025 at 12:17:13PM +0530, Aditya Garg wrote:
> >> The vsprint patch was originally being sent as a seperate patch [1], and
> >> I was waiting it to be taken up. But as suggested by Petr, I'm sending
> >> them via DRM.
> > 
> > You need to do something about your tools, really.
> 
> Uhh, I'll just revert to the tried and tested macOS mail.
> 
> Although I don't think a resend is necessary here now.

I dunno. If people are using `b4`, it might mess up the patch ordering,
I haven't checked this myself (it depends if it takes [PATCH x/y] or message
threading into account first).

It seems not a big deal with _this_ series, but for the future it may be really
a problem (esp. from [runtime] bisectability point of view).

> > Now it's patch 3 threaded to patch 1, while the rest, including cover letter,
> > seems okay.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Petr Mladek]

On Tue 2025-04-08 12:38:50, Andy Shevchenko wrote:
> On Tue, Apr 08, 2025 at 08:52:10AM +0000, Aditya Garg wrote:
> > > On 8 Apr 2025, at 2:11 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Apr 08, 2025 at 12:17:13PM +0530, Aditya Garg wrote:
> > >> The vsprint patch was originally being sent as a seperate patch [1], and
> > >> I was waiting it to be taken up. But as suggested by Petr, I'm sending
> > >> them via DRM.
> > > 
> > > You need to do something about your tools, really.
> > 
> > Uhh, I'll just revert to the tried and tested macOS mail.
> > 
> > Although I don't think a resend is necessary here now.
> 
> I dunno. If people are using `b4`, it might mess up the patch ordering,
> I haven't checked this myself (it depends if it takes [PATCH x/y] or message
> threading into account first).

JFYI, it seems that b4 handles this correctly. AFAIK, it checks the
[PATCH x/y] in subjects.

But I am not sure what DRM guys are using. I guess that they are using
patchwork. I am not sure how it handles this...

Best Regards,
Petr

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Aditya Garg]

Hi

On 08-04-2025 12:17 pm, Aditya Garg wrote:
> The vsprint patch was originally being sent as a seperate patch [1], and
> I was waiting it to be taken up. But as suggested by Petr, I'm sending
> them via DRM.
> 
> v2:
> Remove printf tests, will merge later through Kees' tree
> 
> v3:
> Re-add printf tests, since 6.15-rc1 has the necessary commits merged now.
> 
> v4:
> Do changes requested by Andy and add Petr's review to printf patch.
> 
> Link: https://lore.kernel.org/lkml/1A03A5B4-93AC-4307-AE6A-4A4C4B7E9472@live.com/ [1]
> 
> Aditya Garg (2):
>   printf: add tests for generic FourCCs
>   drm/appletbdrm: use %p4cl instead of %p4cc
> 
> Hector Martin (1):
>   lib/vsprintf: Add support for generic FourCCs by extending %p4cc
> 
>  Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
>  drivers/gpu/drm/tiny/appletbdrm.c         |  4 +--
>  lib/tests/printf_kunit.c                  | 39 +++++++++++++++++++----
>  lib/vsprintf.c                            | 35 ++++++++++++++++----
>  scripts/checkpatch.pl                     |  2 +-
>  5 files changed, 96 insertions(+), 16 deletions(-)
> 

Can I have a feedback from some DRM maintainer on this? AFAIK merge window is over for some time now. It's been more than a week and last time when I submitted, it just stayed in the mailing list without any feedback.

Thanks
Aditya

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Alyssa Rosenzweig]

> Can I have a feedback from some DRM maintainer on this? AFAIK merge window is over for some time now. It's been more than a week and last time when I submitted, it just stayed in the mailing list without any feedback.

DRM hides the merge window from committers so that's not super relevant.

I am a DRM committer and can pick this up if necessary but it's not
clear to me what's going thru with DRM vs elsewhere.

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Aditya Garg]

On 21-04-2025 05:35 pm, Alyssa Rosenzweig wrote:
>> Can I have a feedback from some DRM maintainer on this? AFAIK merge window is over for some time now. It's been more than a week and last time when I submitted, it just stayed in the mailing list without any feedback.
> 
> DRM hides the merge window from committers so that's not super relevant.
> 
> I am a DRM committer and can pick this up if necessary but it's not
> clear to me what's going thru with DRM vs elsewhere.

All the three patches are intended to go through DRM. IIRC Petr, the vsprintf maintainers had requested for that to be done.

The relevant patches have been Reviewed-by Petr as well.

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Alyssa Rosenzweig]

> On 21-04-2025 05:35 pm, Alyssa Rosenzweig wrote:
> >> Can I have a feedback from some DRM maintainer on this? AFAIK merge window is over for some time now. It's been more than a week and last time when I submitted, it just stayed in the mailing list without any feedback.
> > 
> > DRM hides the merge window from committers so that's not super relevant.
> > 
> > I am a DRM committer and can pick this up if necessary but it's not
> > clear to me what's going thru with DRM vs elsewhere.
> 
> All the three patches are intended to go through DRM. IIRC Petr, the vsprintf maintainers had requested for that to be done.
> 
> The relevant patches have been Reviewed-by Petr as well.

OK, will queue this today.

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Aditya Garg]

On 21-04-2025 06:37 pm, Alyssa Rosenzweig wrote:
>> On 21-04-2025 05:35 pm, Alyssa Rosenzweig wrote:
>>>> Can I have a feedback from some DRM maintainer on this? AFAIK merge window is over for some time now. It's been more than a week and last time when I submitted, it just stayed in the mailing list without any feedback.
>>>
>>> DRM hides the merge window from committers so that's not super relevant.
>>>
>>> I am a DRM committer and can pick this up if necessary but it's not
>>> clear to me what's going thru with DRM vs elsewhere.
>>
>> All the three patches are intended to go through DRM. IIRC Petr, the vsprintf maintainers had requested for that to be done.
>>
>> The relevant patches have been Reviewed-by Petr as well.
> 
> OK, will queue this today.

Thanks!

Also, Petr has requested them to be backported for 6.15

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Aditya Garg]

On 21-04-2025 06:38 pm, Aditya Garg wrote:
> 
> 
> On 21-04-2025 06:37 pm, Alyssa Rosenzweig wrote:
>>> On 21-04-2025 05:35 pm, Alyssa Rosenzweig wrote:
>>>>> Can I have a feedback from some DRM maintainer on this? AFAIK merge window is over for some time now. It's been more than a week and last time when I submitted, it just stayed in the mailing list without any feedback.
>>>>
>>>> DRM hides the merge window from committers so that's not super relevant.
>>>>
>>>> I am a DRM committer and can pick this up if necessary but it's not
>>>> clear to me what's going thru with DRM vs elsewhere.
>>>
>>> All the three patches are intended to go through DRM. IIRC Petr, the vsprintf maintainers had requested for that to be done.
>>>
>>> The relevant patches have been Reviewed-by Petr as well.
>>
>> OK, will queue this today.
> 
> Thanks!
> 
> Also, Petr has requested them to be backported for 6.15

To be more clear, he does not have objection for backporting to 6.15. Although its your call. 

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Alyssa Rosenzweig]

I didn't realize this was so subtle with the backporting. I don't think
I can take this on, sorry.

Le Mon, Apr 21, 2025 at 06:40:23PM +0530, Aditya Garg a écrit :
> 
> 
> On 21-04-2025 06:38 pm, Aditya Garg wrote:
> > 
> > 
> > On 21-04-2025 06:37 pm, Alyssa Rosenzweig wrote:
> >>> On 21-04-2025 05:35 pm, Alyssa Rosenzweig wrote:
> >>>>> Can I have a feedback from some DRM maintainer on this? AFAIK merge window is over for some time now. It's been more than a week and last time when I submitted, it just stayed in the mailing list without any feedback.
> >>>>
> >>>> DRM hides the merge window from committers so that's not super relevant.
> >>>>
> >>>> I am a DRM committer and can pick this up if necessary but it's not
> >>>> clear to me what's going thru with DRM vs elsewhere.
> >>>
> >>> All the three patches are intended to go through DRM. IIRC Petr, the vsprintf maintainers had requested for that to be done.
> >>>
> >>> The relevant patches have been Reviewed-by Petr as well.
> >>
> >> OK, will queue this today.
> > 
> > Thanks!
> > 
> > Also, Petr has requested them to be backported for 6.15
> 
> To be more clear, he does not have objection for backporting to 6.15. Although its your call. 
> 

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Aditya Garg]

On 21-04-2025 07:19 pm, Alyssa Rosenzweig wrote:
> I didn't realize this was so subtle with the backporting. I don't think
> I can take this on, sorry.

Any change needed or just because some other maintainer manages this? Although Tbh, I really don't care about backporting since T2 Mac users are still using patched kernels provided by us. I'd rather free my mind in getting this done.
> 
> Le Mon, Apr 21, 2025 at 06:40:23PM +0530, Aditya Garg a écrit :
>>
>>
>> On 21-04-2025 06:38 pm, Aditya Garg wrote:
>>>
>>>
>>> On 21-04-2025 06:37 pm, Alyssa Rosenzweig wrote:
>>>>> On 21-04-2025 05:35 pm, Alyssa Rosenzweig wrote:
>>>>>>> Can I have a feedback from some DRM maintainer on this? AFAIK merge window is over for some time now. It's been more than a week and last time when I submitted, it just stayed in the mailing list without any feedback.
>>>>>>
>>>>>> DRM hides the merge window from committers so that's not super relevant.
>>>>>>
>>>>>> I am a DRM committer and can pick this up if necessary but it's not
>>>>>> clear to me what's going thru with DRM vs elsewhere.
>>>>>
>>>>> All the three patches are intended to go through DRM. IIRC Petr, the vsprintf maintainers had requested for that to be done.
>>>>>
>>>>> The relevant patches have been Reviewed-by Petr as well.
>>>>
>>>> OK, will queue this today.
>>>
>>> Thanks!
>>>
>>> Also, Petr has requested them to be backported for 6.15
>>
>> To be more clear, he does not have objection for backporting to 6.15. Although its your call. 
>>

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Alyssa Rosenzweig]

> Any change needed or just because some other maintainer manages this? Although Tbh, I really don't care about backporting since T2 Mac users are still using patched kernels provided by us. I'd rather free my mind in getting this done.

I'm just too new to kernel to do nontrivial merges. Either I can queue
this series to drm-misc-next today (no backport no changes) or we can
wait for someone more experienced. Let me know which one.

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Aditya Garg]

On 21-04-2025 07:26 pm, Alyssa Rosenzweig wrote:
>> Any change needed or just because some other maintainer manages this? Although Tbh, I really don't care about backporting since T2 Mac users are still using patched kernels provided by us. I'd rather free my mind in getting this done.
> 
> I'm just too new to kernel to do nontrivial merges. Either I can queue
> this series to drm-misc-next today (no backport no changes) or we can
> wait for someone more experienced. Let me know which one.

Merge it to drm-misc-next. I am totally ok with that.

Re: [PATCH v4 0/3] Use proper printk format in appletbdrm

[Alyssa Rosenzweig]

On Tue, 08 Apr 2025 12:17:13 +0530, Aditya Garg wrote:
> The vsprint patch was originally being sent as a seperate patch [1], and
> I was waiting it to be taken up. But as suggested by Petr, I'm sending
> them via DRM.
> 
> v2:
> Remove printf tests, will merge later through Kees' tree
> 
> [...]

Applied, thanks!

[1/3] lib/vsprintf: Add support for generic FourCCs by extending %p4cc
      commit: 1938479b2720ebc05aab349c7dc0a53921ff7c87
[2/3] printf: add tests for generic FourCCs
      commit: 403ff8fd2dbf5066128af4d1fde76c35a800369d
[3/3] drm/appletbdrm: use %p4cl instead of %p4cc
      commit: a49ce9cc85a82d5c5d65186f5a8fda0ebfcff571

Best regards,
-- 
Alyssa Rosenzweig <alyssa@rosenzweig.io>