[PATCH 0/2] Use proper printk format in appletbdrm

[PATCH 0/2] 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 since 6.15 merge window is near, a
potential delay between the drm and vsprintf patch might make the vsprint
patch as an unused addition in 6.15. So, I am sending it together. From
what I have observed in LKML, vsprintf patches are being preferred to be
taken upon by trees which are actually using them.

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

Aditya Garg (1):
  drm/appletbdm: 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/test_printf.c                         | 39 +++++++++++++++++++----
 lib/vsprintf.c                            | 35 ++++++++++++++++----
 scripts/checkpatch.pl                     |  2 +-
 5 files changed, 96 insertions(+), 16 deletions(-)

-- 
2.47.1

[PATCH 1/2] 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>
Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
 lib/test_printf.c                         | 39 +++++++++++++++++++----
 lib/vsprintf.c                            | 35 ++++++++++++++++----
 scripts/checkpatch.pl                     |  2 +-
 4 files changed, 94 insertions(+), 14 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index ecccc0473..bd420e8aa 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/test_printf.c b/lib/test_printf.c
index 59dbe4f9a..b9e8afc01 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
 	software_node_unregister_node_group(group);
 }
 
+struct fourcc_struct {
+	u32 code;
+	const char *str;
+};
+
+static void __init fourcc_pointer_test(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 __init fourcc_pointer(void)
 {
-	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(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
+	fourcc_pointer_test(try_ch, ARRAY_SIZE(try_ch), "%p4ch");
+	fourcc_pointer_test(try_cn, ARRAY_SIZE(try_cn), "%p4cn");
+	fourcc_pointer_test(try_cl, ARRAY_SIZE(try_cl), "%p4cl");
+	fourcc_pointer_test(try_cb, ARRAY_SIZE(try_cb), "%p4cb");
 }
 
 static void __init
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 56fe96319..56511a994 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1781,27 +1781,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 7b28ad331..5595a0898 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6904,7 +6904,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.47.1

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

[Thomas Zimmermann]

Hi

Am 12.03.25 um 10:05 schrieb 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

That looks like someone trying to be too clever for their own good. Just 
my 2 cts.

Best regards
Thomas

>
> 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>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---
>   Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
>   lib/test_printf.c                         | 39 +++++++++++++++++++----
>   lib/vsprintf.c                            | 35 ++++++++++++++++----
>   scripts/checkpatch.pl                     |  2 +-
>   4 files changed, 94 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index ecccc0473..bd420e8aa 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/test_printf.c b/lib/test_printf.c
> index 59dbe4f9a..b9e8afc01 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>   	software_node_unregister_node_group(group);
>   }
>   
> +struct fourcc_struct {
> +	u32 code;
> +	const char *str;
> +};
> +
> +static void __init fourcc_pointer_test(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 __init fourcc_pointer(void)
>   {
> -	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(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
> +	fourcc_pointer_test(try_ch, ARRAY_SIZE(try_ch), "%p4ch");
> +	fourcc_pointer_test(try_cn, ARRAY_SIZE(try_cn), "%p4cn");
> +	fourcc_pointer_test(try_cl, ARRAY_SIZE(try_cl), "%p4cl");
> +	fourcc_pointer_test(try_cb, ARRAY_SIZE(try_cb), "%p4cb");
>   }
>   
>   static void __init
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 56fe96319..56511a994 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1781,27 +1781,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 7b28ad331..5595a0898 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6904,7 +6904,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;
>   					}

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

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

[Aditya Garg]

> On 12 Mar 2025, at 5:16 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
> Hi
> 
>> Am 12.03.25 um 10:05 schrieb 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
> 
> That looks like someone trying to be too clever for their own good. Just my 2 cts.

I don't understand what you are trying to say. Anyways, I thought it's obvious, but Petr's Ack is still left and thus cannot be merged into DRM for now unless he says so in this thread.
> 
> Best regards
> Thomas
> 
>> 
>> 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>
>> Signed-off-by: Hector Martin <marcan@marcan.st>
>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>> ---
>>  Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
>>  lib/test_printf.c                         | 39 +++++++++++++++++++----
>>  lib/vsprintf.c                            | 35 ++++++++++++++++----
>>  scripts/checkpatch.pl                     |  2 +-
>>  4 files changed, 94 insertions(+), 14 deletions(-)
>> 
>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>> index ecccc0473..bd420e8aa 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/test_printf.c b/lib/test_printf.c
>> index 59dbe4f9a..b9e8afc01 100644
>> --- a/lib/test_printf.c
>> +++ b/lib/test_printf.c
>> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>>      software_node_unregister_node_group(group);
>>  }
>>  +struct fourcc_struct {
>> +    u32 code;
>> +    const char *str;
>> +};
>> +
>> +static void __init fourcc_pointer_test(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 __init fourcc_pointer(void)
>>  {
>> -    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(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
>> +    fourcc_pointer_test(try_ch, ARRAY_SIZE(try_ch), "%p4ch");
>> +    fourcc_pointer_test(try_cn, ARRAY_SIZE(try_cn), "%p4cn");
>> +    fourcc_pointer_test(try_cl, ARRAY_SIZE(try_cl), "%p4cl");
>> +    fourcc_pointer_test(try_cb, ARRAY_SIZE(try_cb), "%p4cb");
>>  }
>>    static void __init
>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>> index 56fe96319..56511a994 100644
>> --- a/lib/vsprintf.c
>> +++ b/lib/vsprintf.c
>> @@ -1781,27 +1781,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 7b28ad331..5595a0898 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -6904,7 +6904,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;
>>                      }
> 
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
> 

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

[Thomas Zimmermann]

Hi

Am 12.03.25 um 12:49 schrieb Aditya Garg:
>
>> On 12 Mar 2025, at 5:16 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>>> Am 12.03.25 um 10:05 schrieb 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
>> That looks like someone trying to be too clever for their own good. Just my 2 cts.
> I don't understand what you are trying to say. Anyways, I thought it's obvious, but Petr's Ack is still left and thus cannot be merged into DRM for now unless he says so in this thread.

I'm trying to say that the author of this patch found the %p4cc 
functionality and over-generalized the feature. Source code should 
express the idea of what it's doing in clear terms. %p4ch somehow 
doesn't do that for me. Printing 4 bytes in various orders without 
context seems arbitrary and confusing.

(I don't really have a say here. I'm just asking to reconsider this change.)

Best regards
Thomas

>> Best regards
>> Thomas
>>
>>> 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>
>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>> ---
>>>   Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
>>>   lib/test_printf.c                         | 39 +++++++++++++++++++----
>>>   lib/vsprintf.c                            | 35 ++++++++++++++++----
>>>   scripts/checkpatch.pl                     |  2 +-
>>>   4 files changed, 94 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>>> index ecccc0473..bd420e8aa 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/test_printf.c b/lib/test_printf.c
>>> index 59dbe4f9a..b9e8afc01 100644
>>> --- a/lib/test_printf.c
>>> +++ b/lib/test_printf.c
>>> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>>>       software_node_unregister_node_group(group);
>>>   }
>>>   +struct fourcc_struct {
>>> +    u32 code;
>>> +    const char *str;
>>> +};
>>> +
>>> +static void __init fourcc_pointer_test(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 __init fourcc_pointer(void)
>>>   {
>>> -    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(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
>>> +    fourcc_pointer_test(try_ch, ARRAY_SIZE(try_ch), "%p4ch");
>>> +    fourcc_pointer_test(try_cn, ARRAY_SIZE(try_cn), "%p4cn");
>>> +    fourcc_pointer_test(try_cl, ARRAY_SIZE(try_cl), "%p4cl");
>>> +    fourcc_pointer_test(try_cb, ARRAY_SIZE(try_cb), "%p4cb");
>>>   }
>>>     static void __init
>>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>>> index 56fe96319..56511a994 100644
>>> --- a/lib/vsprintf.c
>>> +++ b/lib/vsprintf.c
>>> @@ -1781,27 +1781,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 7b28ad331..5595a0898 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -6904,7 +6904,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;
>>>                       }
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

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

[Aditya Garg]

> On 12 Mar 2025, at 5:29 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
> Hi
> 
>> Am 12.03.25 um 12:49 schrieb Aditya Garg:
>> 
>>>> On 12 Mar 2025, at 5:16 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>> 
>>> Hi
>>> 
>>>> Am 12.03.25 um 10:05 schrieb 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
>>> That looks like someone trying to be too clever for their own good. Just my 2 cts.
>> I don't understand what you are trying to say. Anyways, I thought it's obvious, but Petr's Ack is still left and thus cannot be merged into DRM for now unless he says so in this thread.
> 
> I'm trying to say that the author of this patch found the %p4cc functionality and over-generalized the feature. Source code should express the idea of what it's doing in clear terms. %p4ch somehow doesn't do that for me. Printing 4 bytes in various orders without context seems arbitrary and confusing.
> 
> (I don't really have a say here. I'm just asking to reconsider this change.)

Ah I see. I'll checkout the macros you sent. The Asahi Linux SMC drivers would need these as well, so I'll probably first wait for the vsprintf maintainers and also Asahi Linux maintainers for their views.
> 
> Best regards
> Thomas
> 
>>> Best regards
>>> Thomas
>>> 
>>>> 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>
>>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>>> ---
>>>>  Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
>>>>  lib/test_printf.c                         | 39 +++++++++++++++++++----
>>>>  lib/vsprintf.c                            | 35 ++++++++++++++++----
>>>>  scripts/checkpatch.pl                     |  2 +-
>>>>  4 files changed, 94 insertions(+), 14 deletions(-)
>>>> 
>>>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>>>> index ecccc0473..bd420e8aa 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/test_printf.c b/lib/test_printf.c
>>>> index 59dbe4f9a..b9e8afc01 100644
>>>> --- a/lib/test_printf.c
>>>> +++ b/lib/test_printf.c
>>>> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>>>>      software_node_unregister_node_group(group);
>>>>  }
>>>>  +struct fourcc_struct {
>>>> +    u32 code;
>>>> +    const char *str;
>>>> +};
>>>> +
>>>> +static void __init fourcc_pointer_test(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 __init fourcc_pointer(void)
>>>>  {
>>>> -    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(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
>>>> +    fourcc_pointer_test(try_ch, ARRAY_SIZE(try_ch), "%p4ch");
>>>> +    fourcc_pointer_test(try_cn, ARRAY_SIZE(try_cn), "%p4cn");
>>>> +    fourcc_pointer_test(try_cl, ARRAY_SIZE(try_cl), "%p4cl");
>>>> +    fourcc_pointer_test(try_cb, ARRAY_SIZE(try_cb), "%p4cb");
>>>>  }
>>>>    static void __init
>>>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>>>> index 56fe96319..56511a994 100644
>>>> --- a/lib/vsprintf.c
>>>> +++ b/lib/vsprintf.c
>>>> @@ -1781,27 +1781,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 7b28ad331..5595a0898 100755
>>>> --- a/scripts/checkpatch.pl
>>>> +++ b/scripts/checkpatch.pl
>>>> @@ -6904,7 +6904,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;
>>>>                      }
>>> --
>>> --
>>> Thomas Zimmermann
>>> Graphics Driver Developer
>>> SUSE Software Solutions Germany GmbH
>>> Frankenstrasse 146, 90461 Nuernberg, Germany
>>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>>> HRB 36809 (AG Nuernberg)
>>> 
> 
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
> 

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

[Aditya Garg]

> On 12 Mar 2025, at 5:33 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> 
> 
>> On 12 Mar 2025, at 5:29 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> 
>> Hi
>> 
>>>> Am 12.03.25 um 12:49 schrieb Aditya Garg:
>>> 
>>>>> On 12 Mar 2025, at 5:16 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>> 
>>>> Hi
>>>> 
>>>>> Am 12.03.25 um 10:05 schrieb 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
>>>> That looks like someone trying to be too clever for their own good. Just my 2 cts.
>>> I don't understand what you are trying to say. Anyways, I thought it's obvious, but Petr's Ack is still left and thus cannot be merged into DRM for now unless he says so in this thread.
>> 
>> I'm trying to say that the author of this patch found the %p4cc functionality and over-generalized the feature. Source code should express the idea of what it's doing in clear terms. %p4ch somehow doesn't do that for me. Printing 4 bytes in various orders without context seems arbitrary and confusing.
>> 
>> (I don't really have a say here. I'm just asking to reconsider this change.)
> 
> Ah I see. I'll checkout the macros you sent. The Asahi Linux SMC drivers would need these as well, so I'll probably first wait for the vsprintf maintainers and also Asahi Linux maintainers for their views.
>> 
You also might have noticed that the FourCCs actually make sense in this driver. Eg: REDY, CLRD stand for APPLETBDRM_MSG_SIGNAL_READINESS and APPLETBDRM_MSG_CLEAR_DISPLAY respectively.
>> Best regards
>> Thomas
>> 
>>>> Best regards
>>>> Thomas
>>>> 
>>>>> 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>
>>>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>>>> ---
>>>>> Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
>>>>> lib/test_printf.c                         | 39 +++++++++++++++++++----
>>>>> lib/vsprintf.c                            | 35 ++++++++++++++++----
>>>>> scripts/checkpatch.pl                     |  2 +-
>>>>> 4 files changed, 94 insertions(+), 14 deletions(-)
>>>>> 
>>>>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>>>>> index ecccc0473..bd420e8aa 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/test_printf.c b/lib/test_printf.c
>>>>> index 59dbe4f9a..b9e8afc01 100644
>>>>> --- a/lib/test_printf.c
>>>>> +++ b/lib/test_printf.c
>>>>> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>>>>>     software_node_unregister_node_group(group);
>>>>> }
>>>>> +struct fourcc_struct {
>>>>> +    u32 code;
>>>>> +    const char *str;
>>>>> +};
>>>>> +
>>>>> +static void __init fourcc_pointer_test(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 __init fourcc_pointer(void)
>>>>> {
>>>>> -    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(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
>>>>> +    fourcc_pointer_test(try_ch, ARRAY_SIZE(try_ch), "%p4ch");
>>>>> +    fourcc_pointer_test(try_cn, ARRAY_SIZE(try_cn), "%p4cn");
>>>>> +    fourcc_pointer_test(try_cl, ARRAY_SIZE(try_cl), "%p4cl");
>>>>> +    fourcc_pointer_test(try_cb, ARRAY_SIZE(try_cb), "%p4cb");
>>>>> }
>>>>>   static void __init
>>>>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>>>>> index 56fe96319..56511a994 100644
>>>>> --- a/lib/vsprintf.c
>>>>> +++ b/lib/vsprintf.c
>>>>> @@ -1781,27 +1781,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 7b28ad331..5595a0898 100755
>>>>> --- a/scripts/checkpatch.pl
>>>>> +++ b/scripts/checkpatch.pl
>>>>> @@ -6904,7 +6904,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;
>>>>>                     }
>>>> --
>>>> --
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Frankenstrasse 146, 90461 Nuernberg, Germany
>>>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>>>> HRB 36809 (AG Nuernberg)
>>>> 
>> 
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>> 

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

[Sven Peter]

Hi,


On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:
>> On 12 Mar 2025, at 5:29 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> 
>> Hi
>> 
>>> Am 12.03.25 um 12:49 schrieb Aditya Garg:
>>> 
>>>>> On 12 Mar 2025, at 5:16 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>> 
>>>> Hi
>>>> 
>>>>> Am 12.03.25 um 10:05 schrieb 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
>>>> That looks like someone trying to be too clever for their own good. Just my 2 cts.
>>> I don't understand what you are trying to say. Anyways, I thought it's obvious, but Petr's Ack is still left and thus cannot be merged into DRM for now unless he says so in this thread.
>> 
>> I'm trying to say that the author of this patch found the %p4cc functionality and over-generalized the feature. Source code should express the idea of what it's doing in clear terms. %p4ch somehow doesn't do that for me. Printing 4 bytes in various orders without context seems arbitrary and confusing.
>> 
>> (I don't really have a say here. I'm just asking to reconsider this change.)
>
> Ah I see. I'll checkout the macros you sent. The Asahi Linux SMC 
> drivers would need these as well, so I'll probably first wait for the 
> vsprintf maintainers and also Asahi Linux maintainers for their views.

I don't have a strong opinion either way: for SMC I just need to print
FourCC keys for debugging / information in a few places.

I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
and was just going to use macros to print the SMC FourCC keys similar to
DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
the topic later.

Right now I have these in my local tree (only compile tested so far):

#define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
#define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)

which are then used like this:

	dev_info(dev,
		"Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
		 smc->key_count, SMC_KEY_ARG(smc->first_key),
		 SMC_KEY_ARG(smc->last_key));

Best,

Sven

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

[Aditya Garg]

> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
> 
> Hi,
> 
> 
> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:
>>>> On 12 Mar 2025, at 5:29 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>> 
>>> Hi
>>> 
>>>> Am 12.03.25 um 12:49 schrieb Aditya Garg:
>>>> 
>>>>>> On 12 Mar 2025, at 5:16 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>>> 
>>>>> Hi
>>>>> 
>>>>>> Am 12.03.25 um 10:05 schrieb 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
>>>>> That looks like someone trying to be too clever for their own good. Just my 2 cts.
>>>> I don't understand what you are trying to say. Anyways, I thought it's obvious, but Petr's Ack is still left and thus cannot be merged into DRM for now unless he says so in this thread.
>>> 
>>> I'm trying to say that the author of this patch found the %p4cc functionality and over-generalized the feature. Source code should express the idea of what it's doing in clear terms. %p4ch somehow doesn't do that for me. Printing 4 bytes in various orders without context seems arbitrary and confusing.
>>> 
>>> (I don't really have a say here. I'm just asking to reconsider this change.)
>> 
>> Ah I see. I'll checkout the macros you sent. The Asahi Linux SMC
>> drivers would need these as well, so I'll probably first wait for the
>> vsprintf maintainers and also Asahi Linux maintainers for their views.
> 
> I don't have a strong opinion either way: for SMC I just need to print
> FourCC keys for debugging / information in a few places.
> 
> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
> and was just going to use macros to print the SMC FourCC keys similar to
> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
> the topic later.
> 
> Right now I have these in my local tree (only compile tested so far):
> 
> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)

That seems to be a nice alternative, which I guess Thomas was also suggesting.

> which are then used like this:
> 
>    dev_info(dev,
>        "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
>         smc->key_count, SMC_KEY_ARG(smc->first_key),
>         SMC_KEY_ARG(smc->last_key));
> 
> Best,
> 
> Sven

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

[Andy Shevchenko]

On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
> > On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
> > On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:

...

> > I don't have a strong opinion either way: for SMC I just need to print
> > FourCC keys for debugging / information in a few places.
> > 
> > I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
> > and was just going to use macros to print the SMC FourCC keys similar to
> > DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
> > the topic later.
> > 
> > Right now I have these in my local tree (only compile tested so far):
> > 
> > #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
> > #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
> 
> That seems to be a nice alternative, which I guess Thomas was also suggesting.

I don't think it's "nice". Each of the approaches has pros and cons.
You can start from bloat-o-meter here and compare it with your %p extension.

Also, can you show the bloat-o-meter output for the vsprintf.c?

> > which are then used like this:
> > 
> >    dev_info(dev,
> >        "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
> >         smc->key_count, SMC_KEY_ARG(smc->first_key),
> >         SMC_KEY_ARG(smc->last_key));

-- 
With Best Regards,
Andy Shevchenko

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

[Aditya Garg]

> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
>>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:
> 
> ...
> 
>>> I don't have a strong opinion either way: for SMC I just need to print
>>> FourCC keys for debugging / information in a few places.
>>> 
>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
>>> and was just going to use macros to print the SMC FourCC keys similar to
>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
>>> the topic later.
>>> 
>>> Right now I have these in my local tree (only compile tested so far):
>>> 
>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
>> 
>> That seems to be a nice alternative, which I guess Thomas was also suggesting.
> 
> I don't think it's "nice". Each of the approaches has pros and cons.

I would prefer vsprintf, but if it's not there, that remains as nice right?

> You can start from bloat-o-meter here and compare it with your %p extension.
> 
> Also, can you show the bloat-o-meter output for the vsprintf.c?

vsprintf isn't a kernel module, is it? I'll have to compile a new kernel I guess.
> 
>>> which are then used like this:
>>> 
>>>   dev_info(dev,
>>>       "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
>>>        smc->key_count, SMC_KEY_ARG(smc->first_key),
>>>        SMC_KEY_ARG(smc->last_key));
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

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

[Andy Shevchenko]

On Wed, Mar 12, 2025 at 07:35:54PM +0000, Aditya Garg wrote:
> > On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
> >>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
> >>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:

...

> >>> I don't have a strong opinion either way: for SMC I just need to print
> >>> FourCC keys for debugging / information in a few places.
> >>> 
> >>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
> >>> and was just going to use macros to print the SMC FourCC keys similar to
> >>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
> >>> the topic later.
> >>> 
> >>> Right now I have these in my local tree (only compile tested so far):
> >>> 
> >>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
> >>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
> >> 
> >> That seems to be a nice alternative, which I guess Thomas was also suggesting.
> > 
> > I don't think it's "nice". Each of the approaches has pros and cons.
> 
> I would prefer vsprintf, but if it's not there, that remains as nice right?

Nope, it remains us with the only approach (besides copy'n'paste everywhere
which is error prone).

> > You can start from bloat-o-meter here and compare it with your %p extension.
> > 
> > Also, can you show the bloat-o-meter output for the vsprintf.c?
> 
> vsprintf isn't a kernel module, is it? I'll have to compile a new kernel I guess.

You can just compile one file. We need an object out of it, we don't it to be
linked.

> >>> which are then used like this:
> >>> 
> >>>   dev_info(dev,
> >>>       "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
> >>>        smc->key_count, SMC_KEY_ARG(smc->first_key),
> >>>        SMC_KEY_ARG(smc->last_key));

-- 
With Best Regards,
Andy Shevchenko

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

[Aditya Garg]

> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:
> 
> ...
> 
>>> I don't have a strong opinion either way: for SMC I just need to print
>>> FourCC keys for debugging / information in a few places.
>>> 
>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
>>> and was just going to use macros to print the SMC FourCC keys similar to
>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
>>> the topic later.
>>> 
>>> Right now I have these in my local tree (only compile tested so far):
>>> 
>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
>> 
>> That seems to be a nice alternative, which I guess Thomas was also suggesting.
> 
> I don't think it's "nice". Each of the approaches has pros and cons.
> You can start from bloat-o-meter here and compare it with your %p extension.
> 
> Also, can you show the bloat-o-meter output for the vsprintf.c?

Here are your outputs:

---------------------------------------------------------------------
For appletbdrm:

aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO
add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
Function                                     old     new   delta
appletbdrm_read_response                     395     459     +64
appletbdrm_probe                            1786    1767     -19
Total: Before=13418, After=13463, chg +0.34%

aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO -c
add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
Function                                     old     new   delta
appletbdrm_read_response                     395     459     +64
appletbdrm_probe                            1786    1767     -19
Total: Before=5217, After=5262, chg +0.86%
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
Data                                         old     new   delta
Total: Before=1560, After=1560, chg +0.00%
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
RO Data                                      old     new   delta
Total: Before=6641, After=6641, chg +0.00%

aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO -d
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
Data                                         old     new   delta
Total: Before=8201, After=8201, chg +0.00%

aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO -t
add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
Function                                     old     new   delta
appletbdrm_read_response                     395     459     +64
appletbdrm_probe                            1786    1767     -19
Total: Before=5217, After=5262, chg +0.86%

---------------------------------------------------------------------
For vsprintf:

aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW
add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
Function                                     old     new   delta
fourcc_string                                479     699    +220
Total: Before=26454, After=26674, chg +0.83%

aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW -c
add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
Function                                     old     new   delta
fourcc_string                                479     699    +220
Total: Before=24718, After=24938, chg +0.89%
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
Data                                         old     new   delta
Total: Before=229, After=229, chg +0.00%
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
RO Data                                      old     new   delta
Total: Before=1507, After=1507, chg +0.00%

aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW -d
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
Data                                         old     new   delta
Total: Before=1736, After=1736, chg +0.00%

aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW -t
add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
Function                                     old     new   delta
fourcc_string                                479     699    +220
Total: Before=24718, After=24938, chg +0.89%


> 
>>> which are then used like this:
>>> 
>>>  dev_info(dev,
>>>      "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
>>>       smc->key_count, SMC_KEY_ARG(smc->first_key),
>>>       SMC_KEY_ARG(smc->last_key));
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

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

[Andy Shevchenko]

On Thu, Mar 13, 2025 at 07:26:05AM +0000, Aditya Garg wrote:
> > On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
> >>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
> >>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:

...

> >>> I don't have a strong opinion either way: for SMC I just need to print
> >>> FourCC keys for debugging / information in a few places.
> >>> 
> >>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
> >>> and was just going to use macros to print the SMC FourCC keys similar to
> >>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
> >>> the topic later.
> >>> 
> >>> Right now I have these in my local tree (only compile tested so far):
> >>> 
> >>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
> >>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
> >> 
> >> That seems to be a nice alternative, which I guess Thomas was also suggesting.
> > 
> > I don't think it's "nice". Each of the approaches has pros and cons.
> > You can start from bloat-o-meter here and compare it with your %p extension.
> > 
> > Also, can you show the bloat-o-meter output for the vsprintf.c?
> 
> Here are your outputs:

Thank you!

> ---------------------------------------------------------------------
> For appletbdrm:
> 
> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO
> add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
> Function                                     old     new   delta
> appletbdrm_read_response                     395     459     +64
> appletbdrm_probe                            1786    1767     -19
> Total: Before=13418, After=13463, chg +0.34%

This is enough, no need to repeat this for every parameter.

> ---------------------------------------------------------------------
> For vsprintf:
> 
> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW
> add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
> Function                                     old     new   delta
> fourcc_string                                479     699    +220
> Total: Before=26454, After=26674, chg +0.83%

So, we get +220 bytes vs +43 bytes. It means if we found 5+ users, it worth
doing.

> >>> which are then used like this:
> >>> 
> >>>  dev_info(dev,
> >>>      "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
> >>>       smc->key_count, SMC_KEY_ARG(smc->first_key),
> >>>       SMC_KEY_ARG(smc->last_key));

-- 
With Best Regards,
Andy Shevchenko

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

[Aditya Garg]

> On 13 Mar 2025, at 2:19 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> On Thu, Mar 13, 2025 at 07:26:05AM +0000, Aditya Garg wrote:
>>>> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>>> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
>>>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
>>>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:
> 
> ...
> 
>>>>> I don't have a strong opinion either way: for SMC I just need to print
>>>>> FourCC keys for debugging / information in a few places.
>>>>> 
>>>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
>>>>> and was just going to use macros to print the SMC FourCC keys similar to
>>>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
>>>>> the topic later.
>>>>> 
>>>>> Right now I have these in my local tree (only compile tested so far):
>>>>> 
>>>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
>>>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
>>>> 
>>>> That seems to be a nice alternative, which I guess Thomas was also suggesting.
>>> 
>>> I don't think it's "nice". Each of the approaches has pros and cons.
>>> You can start from bloat-o-meter here and compare it with your %p extension.
>>> 
>>> Also, can you show the bloat-o-meter output for the vsprintf.c?
>> 
>> Here are your outputs:
> 
> Thank you!
> 
>> ---------------------------------------------------------------------
>> For appletbdrm:
>> 
>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO
>> add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
>> Function                                     old     new   delta
>> appletbdrm_read_response                     395     459     +64
>> appletbdrm_probe                            1786    1767     -19
>> Total: Before=13418, After=13463, chg +0.34%
> 
> This is enough, no need to repeat this for every parameter.
> 
>> ---------------------------------------------------------------------
>> For vsprintf:
>> 
>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW
>> add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
>> Function                                     old     new   delta
>> fourcc_string                                479     699    +220
>> Total: Before=26454, After=26674, chg +0.83%
> 
> So, we get +220 bytes vs +43 bytes. It means if we found 5+ users, it worth
> doing.
> 

Will it also depend upon the number of times it's being used? In appletbdrm, it is being used 3 times. Probably more in Asahi SMC.
>>>>> which are then used like this:
>>>>> 
>>>>> dev_info(dev,
>>>>>     "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
>>>>>      smc->key_count, SMC_KEY_ARG(smc->first_key),
>>>>>      SMC_KEY_ARG(smc->last_key));
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

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

[Andy Shevchenko]

On Thu, Mar 13, 2025 at 08:53:28AM +0000, Aditya Garg wrote:
> > On 13 Mar 2025, at 2:19 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Mar 13, 2025 at 07:26:05AM +0000, Aditya Garg wrote:
> >>>> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >>> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
> >>>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
> >>>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:

...

> >>>>> I don't have a strong opinion either way: for SMC I just need to print
> >>>>> FourCC keys for debugging / information in a few places.
> >>>>> 
> >>>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
> >>>>> and was just going to use macros to print the SMC FourCC keys similar to
> >>>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
> >>>>> the topic later.
> >>>>> 
> >>>>> Right now I have these in my local tree (only compile tested so far):
> >>>>> 
> >>>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
> >>>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
> >>>> 
> >>>> That seems to be a nice alternative, which I guess Thomas was also suggesting.
> >>> 
> >>> I don't think it's "nice". Each of the approaches has pros and cons.
> >>> You can start from bloat-o-meter here and compare it with your %p extension.
> >>> 
> >>> Also, can you show the bloat-o-meter output for the vsprintf.c?
> >> 
> >> Here are your outputs:
> > 
> > Thank you!
> > 
> >> ---------------------------------------------------------------------
> >> For appletbdrm:
> >> 
> >> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO
> >> add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
> >> Function                                     old     new   delta
> >> appletbdrm_read_response                     395     459     +64
> >> appletbdrm_probe                            1786    1767     -19
> >> Total: Before=13418, After=13463, chg +0.34%
> > 
> > This is enough, no need to repeat this for every parameter.
> > 
> >> ---------------------------------------------------------------------
> >> For vsprintf:
> >> 
> >> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW
> >> add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
> >> Function                                     old     new   delta
> >> fourcc_string                                479     699    +220
> >> Total: Before=26454, After=26674, chg +0.83%
> > 
> > So, we get +220 bytes vs +43 bytes. It means if we found 5+ users, it worth
> > doing.
> 
> Will it also depend upon the number of times it's being used? In appletbdrm,
> it is being used 3 times. Probably more in Asahi SMC.

Right, it depends on the usage count. Also on different architectures it may
give different results. On 32-bit it probably gives better statistics.

> >>>>> which are then used like this:
> >>>>> 
> >>>>> dev_info(dev,
> >>>>>     "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
> >>>>>      smc->key_count, SMC_KEY_ARG(smc->first_key),
> >>>>>      SMC_KEY_ARG(smc->last_key));

-- 
With Best Regards,
Andy Shevchenko

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

[Aditya Garg]

> On 13 Mar 2025, at 2:27 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> On Thu, Mar 13, 2025 at 08:53:28AM +0000, Aditya Garg wrote:
>>>> On 13 Mar 2025, at 2:19 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>>> On Thu, Mar 13, 2025 at 07:26:05AM +0000, Aditya Garg wrote:
>>>>>> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>>>>> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
>>>>>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
>>>>>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:
> 
> ...
> 
>>>>>>> I don't have a strong opinion either way: for SMC I just need to print
>>>>>>> FourCC keys for debugging / information in a few places.
>>>>>>> 
>>>>>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
>>>>>>> and was just going to use macros to print the SMC FourCC keys similar to
>>>>>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
>>>>>>> the topic later.
>>>>>>> 
>>>>>>> Right now I have these in my local tree (only compile tested so far):
>>>>>>> 
>>>>>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
>>>>>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
>>>>>> 
>>>>>> That seems to be a nice alternative, which I guess Thomas was also suggesting.
>>>>> 
>>>>> I don't think it's "nice". Each of the approaches has pros and cons.
>>>>> You can start from bloat-o-meter here and compare it with your %p extension.
>>>>> 
>>>>> Also, can you show the bloat-o-meter output for the vsprintf.c?
>>>> 
>>>> Here are your outputs:
>>> 
>>> Thank you!
>>> 
>>>> ---------------------------------------------------------------------
>>>> For appletbdrm:
>>>> 
>>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO
>>>> add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
>>>> Function                                     old     new   delta
>>>> appletbdrm_read_response                     395     459     +64
>>>> appletbdrm_probe                            1786    1767     -19
>>>> Total: Before=13418, After=13463, chg +0.34%
>>> 
>>> This is enough, no need to repeat this for every parameter.
>>> 
>>>> ---------------------------------------------------------------------
>>>> For vsprintf:
>>>> 
>>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW
>>>> add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
>>>> Function                                     old     new   delta
>>>> fourcc_string                                479     699    +220
>>>> Total: Before=26454, After=26674, chg +0.83%
>>> 
>>> So, we get +220 bytes vs +43 bytes. It means if we found 5+ users, it worth
>>> doing.
>> 
>> Will it also depend upon the number of times it's being used? In appletbdrm,
>> it is being used 3 times. Probably more in Asahi SMC.
> 
> Right, it depends on the usage count. Also on different architectures it may
> give different results. On 32-bit it probably gives better statistics.

Best to go ahead with vsprintf then. Petr, are you still there?
> 
>>>>>>> which are then used like this:
>>>>>>> 
>>>>>>> dev_info(dev,
>>>>>>>    "Initialized (%d keys " SMC_KEY_FMT " .. " SMC_KEY_FMT ")\n",
>>>>>>>     smc->key_count, SMC_KEY_ARG(smc->first_key),
>>>>>>>     SMC_KEY_ARG(smc->last_key));
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

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

[Petr Mladek]

Adding Kees into Cc to resolve how to get this patch into the mainline.

On Thu 2025-03-13 09:13:23, Aditya Garg wrote:
> 
> 
> > On 13 Mar 2025, at 2:27 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > 
> > On Thu, Mar 13, 2025 at 08:53:28AM +0000, Aditya Garg wrote:
> >>>> On 13 Mar 2025, at 2:19 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >>> On Thu, Mar 13, 2025 at 07:26:05AM +0000, Aditya Garg wrote:
> >>>>>> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >>>>> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
> >>>>>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
> >>>>>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:
> > 
> > ...
> > 
> >>>>>>> I don't have a strong opinion either way: for SMC I just need to print
> >>>>>>> FourCC keys for debugging / information in a few places.
> >>>>>>> 
> >>>>>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
> >>>>>>> and was just going to use macros to print the SMC FourCC keys similar to
> >>>>>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
> >>>>>>> the topic later.
> >>>>>>> 
> >>>>>>> Right now I have these in my local tree (only compile tested so far):
> >>>>>>> 
> >>>>>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
> >>>>>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
> >>>>>> 
> >>>>>> That seems to be a nice alternative, which I guess Thomas was also suggesting.
> >>>>> 
> >>>>> I don't think it's "nice". Each of the approaches has pros and cons.
> >>>>> You can start from bloat-o-meter here and compare it with your %p extension.
> >>>>> 
> >>>>> Also, can you show the bloat-o-meter output for the vsprintf.c?
> >>>> 
> >>>> Here are your outputs:
> >>> 
> >>> Thank you!
> >>> 
> >>>> ---------------------------------------------------------------------
> >>>> For appletbdrm:
> >>>> 
> >>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO
> >>>> add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
> >>>> Function                                     old     new   delta
> >>>> appletbdrm_read_response                     395     459     +64
> >>>> appletbdrm_probe                            1786    1767     -19
> >>>> Total: Before=13418, After=13463, chg +0.34%
> >>> 
> >>> This is enough, no need to repeat this for every parameter.
> >>> 
> >>>> ---------------------------------------------------------------------
> >>>> For vsprintf:
> >>>> 
> >>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW
> >>>> add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
> >>>> Function                                     old     new   delta
> >>>> fourcc_string                                479     699    +220
> >>>> Total: Before=26454, After=26674, chg +0.83%
> >>> 
> >>> So, we get +220 bytes vs +43 bytes. It means if we found 5+ users, it worth
> >>> doing.
> >> 
> >> Will it also depend upon the number of times it's being used? In appletbdrm,
> >> it is being used 3 times. Probably more in Asahi SMC.
> > 
> > Right, it depends on the usage count. Also on different architectures it may
> > give different results. On 32-bit it probably gives better statistics.
> 
> Best to go ahead with vsprintf then. Petr, are you still there?

I am here but there were many other things in the queue ;-)

I do not have strong opinion. I am not familiar with the FourCC
format and it looks like a magic to me. But it seems that it makes
sense for the users.

I personally find the %pcX modifiers a bit less hacky than
the two macros SMC_KEY_FMT/SMC_KEY_ARG.

So I am fine with this patch:

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


Now, the question is how to get this patch into the mainline.

Normally, it would make perfect sense to queue it via the DRM tree
because drivers/gpu/drm/tiny/appletbdrm.c is a new driver...

But this time there is a conflicting patchset which is reworking
the entire lib/test_printf.c file, see
20250307-printf-kunit-convert-v6-0-4d85c361c241@gmail.com
And it will likely be ready for the next merge window as well.
I am going to review it right away.

It is even more complicated because the patchset converting
the printf test module to KUNIT depends on another changes
in Kees' tree (moving kunit test modules to lib/tests/).
So it might be easier when it goes via Kees' tree.

And it might be easier when even this patch goes via Kees' tree.

My proposal:

I suggest to separate the fourcc_pointer() test update
to a separate patch and add it later after the merge window
when things settle down.

I mean to send the vsprintf.c, checkpatch.pl, and doc update
via DRM tree together with the new appletbdrm.c driver.

And update the selftest later when both DRM tree and KUNIT
update reaches mainline.

How does that sound, please?

Best Regards,
Petr

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

[Aditya Garg]

> On 13 Mar 2025, at 4:18 PM, Petr Mladek <pmladek@suse.com> wrote:
> 
> Adding Kees into Cc to resolve how to get this patch into the mainline.
> 
> On Thu 2025-03-13 09:13:23, Aditya Garg wrote:
>> 
>> 
>>> On 13 Mar 2025, at 2:27 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>>> 
>>> On Thu, Mar 13, 2025 at 08:53:28AM +0000, Aditya Garg wrote:
>>>>>> On 13 Mar 2025, at 2:19 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>>>>> On Thu, Mar 13, 2025 at 07:26:05AM +0000, Aditya Garg wrote:
>>>>>>>> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>>>>>>> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
>>>>>>>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
>>>>>>>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:
>>> 
>>> ...
>>> 
>>>>>>>>> I don't have a strong opinion either way: for SMC I just need to print
>>>>>>>>> FourCC keys for debugging / information in a few places.
>>>>>>>>> 
>>>>>>>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
>>>>>>>>> and was just going to use macros to print the SMC FourCC keys similar to
>>>>>>>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
>>>>>>>>> the topic later.
>>>>>>>>> 
>>>>>>>>> Right now I have these in my local tree (only compile tested so far):
>>>>>>>>> 
>>>>>>>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
>>>>>>>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
>>>>>>>> 
>>>>>>>> That seems to be a nice alternative, which I guess Thomas was also suggesting.
>>>>>>> 
>>>>>>> I don't think it's "nice". Each of the approaches has pros and cons.
>>>>>>> You can start from bloat-o-meter here and compare it with your %p extension.
>>>>>>> 
>>>>>>> Also, can you show the bloat-o-meter output for the vsprintf.c?
>>>>>> 
>>>>>> Here are your outputs:
>>>>> 
>>>>> Thank you!
>>>>> 
>>>>>> ---------------------------------------------------------------------
>>>>>> For appletbdrm:
>>>>>> 
>>>>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO
>>>>>> add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
>>>>>> Function                                     old     new   delta
>>>>>> appletbdrm_read_response                     395     459     +64
>>>>>> appletbdrm_probe                            1786    1767     -19
>>>>>> Total: Before=13418, After=13463, chg +0.34%
>>>>> 
>>>>> This is enough, no need to repeat this for every parameter.
>>>>> 
>>>>>> ---------------------------------------------------------------------
>>>>>> For vsprintf:
>>>>>> 
>>>>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW
>>>>>> add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
>>>>>> Function                                     old     new   delta
>>>>>> fourcc_string                                479     699    +220
>>>>>> Total: Before=26454, After=26674, chg +0.83%
>>>>> 
>>>>> So, we get +220 bytes vs +43 bytes. It means if we found 5+ users, it worth
>>>>> doing.
>>>> 
>>>> Will it also depend upon the number of times it's being used? In appletbdrm,
>>>> it is being used 3 times. Probably more in Asahi SMC.
>>> 
>>> Right, it depends on the usage count. Also on different architectures it may
>>> give different results. On 32-bit it probably gives better statistics.
>> 
>> Best to go ahead with vsprintf then. Petr, are you still there?
> 
> I am here but there were many other things in the queue ;-)
> 
> I do not have strong opinion. I am not familiar with the FourCC
> format and it looks like a magic to me. But it seems that it makes
> sense for the users.
> 
> I personally find the %pcX modifiers a bit less hacky than
> the two macros SMC_KEY_FMT/SMC_KEY_ARG.
> 
> So I am fine with this patch:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>
> 
> 
> Now, the question is how to get this patch into the mainline.
> 
> Normally, it would make perfect sense to queue it via the DRM tree
> because drivers/gpu/drm/tiny/appletbdrm.c is a new driver...
> 
> But this time there is a conflicting patchset which is reworking
> the entire lib/test_printf.c file, see
> 20250307-printf-kunit-convert-v6-0-4d85c361c241@gmail.com

Link seems to be broken
> And it will likely be ready for the next merge window as well.
> I am going to review it right away.
> 
> It is even more complicated because the patchset converting
> the printf test module to KUNIT depends on another changes
> in Kees' tree (moving kunit test modules to lib/tests/).
> So it might be easier when it goes via Kees' tree.
> 
> And it might be easier when even this patch goes via Kees' tree.
> 
> My proposal:
> 
> I suggest to separate the fourcc_pointer() test update
> to a separate patch and add it later after the merge window
> when things settle down.
> 
> I mean to send the vsprintf.c, checkpatch.pl, and doc update
> via DRM tree together with the new appletbdrm.c driver.

Sounds good. At least we can get it working. I’ll make sure the self
tests get updated once 6.15-rc1 gets released, or Kees can share
his tree, where I can add the tests as well.

I’ll send a v2 so that Thomas can take them up.
> 
> And update the selftest later when both DRM tree and KUNIT
> update reaches mainline.
> 
> How does that sound, please?
> 
> Best Regards,
> Petr

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

[Andy Shevchenko]

On Thu, Mar 13, 2025 at 11:06:54AM +0000, Aditya Garg wrote:
> > On 13 Mar 2025, at 4:18 PM, Petr Mladek <pmladek@suse.com> wrote:
> > On Thu 2025-03-13 09:13:23, Aditya Garg wrote:
> >>> On 13 Mar 2025, at 2:27 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >>> On Thu, Mar 13, 2025 at 08:53:28AM +0000, Aditya Garg wrote:
> >>>>>> On 13 Mar 2025, at 2:19 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >>>>> On Thu, Mar 13, 2025 at 07:26:05AM +0000, Aditya Garg wrote:
> >>>>>>>> On 13 Mar 2025, at 12:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >>>>>>> On Wed, Mar 12, 2025 at 07:14:36PM +0000, Aditya Garg wrote:
> >>>>>>>>> On 12 Mar 2025, at 9:05 PM, Sven Peter <sven@svenpeter.dev> wrote:
> >>>>>>>>> On Wed, Mar 12, 2025, at 13:03, Aditya Garg wrote:

...

> >>>>>>>>> I don't have a strong opinion either way: for SMC I just need to print
> >>>>>>>>> FourCC keys for debugging / information in a few places.
> >>>>>>>>> 
> >>>>>>>>> I'm preparing the SMC driver for upstreaming again (after a two year delay :-()
> >>>>>>>>> and was just going to use macros to print the SMC FourCC keys similar to
> >>>>>>>>> DRM_MODE_FMT/DRM_MODE_ARG for now to keep the series smaller and revisit
> >>>>>>>>> the topic later.
> >>>>>>>>> 
> >>>>>>>>> Right now I have these in my local tree (only compile tested so far):
> >>>>>>>>> 
> >>>>>>>>> #define SMC_KEY_FMT "%c%c%c%c (0x%08x)"
> >>>>>>>>> #define SMC_KEY_ARG(k) (k)>>24, (k)>>16, (k)>>8, (k), (k)
> >>>>>>>> 
> >>>>>>>> That seems to be a nice alternative, which I guess Thomas was also suggesting.
> >>>>>>> 
> >>>>>>> I don't think it's "nice". Each of the approaches has pros and cons.
> >>>>>>> You can start from bloat-o-meter here and compare it with your %p extension.
> >>>>>>> 
> >>>>>>> Also, can you show the bloat-o-meter output for the vsprintf.c?
> >>>>>> 
> >>>>>> Here are your outputs:
> >>>>> 
> >>>>> Thank you!
> >>>>> 
> >>>>>> ---------------------------------------------------------------------
> >>>>>> For appletbdrm:
> >>>>>> 
> >>>>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $P4 $MACRO
> >>>>>> add/remove: 0/0 grow/shrink: 1/1 up/down: 64/-19 (45)
> >>>>>> Function                                     old     new   delta
> >>>>>> appletbdrm_read_response                     395     459     +64
> >>>>>> appletbdrm_probe                            1786    1767     -19
> >>>>>> Total: Before=13418, After=13463, chg +0.34%
> >>>>> 
> >>>>> This is enough, no need to repeat this for every parameter.
> >>>>> 
> >>>>>> ---------------------------------------------------------------------
> >>>>>> For vsprintf:
> >>>>>> 
> >>>>>> aditya@MacBook:~/linux$ ./scripts/bloat-o-meter $OLD $NEW
> >>>>>> add/remove: 0/0 grow/shrink: 1/0 up/down: 220/0 (220)
> >>>>>> Function                                     old     new   delta
> >>>>>> fourcc_string                                479     699    +220
> >>>>>> Total: Before=26454, After=26674, chg +0.83%
> >>>>> 
> >>>>> So, we get +220 bytes vs +43 bytes. It means if we found 5+ users, it worth
> >>>>> doing.
> >>>> 
> >>>> Will it also depend upon the number of times it's being used? In appletbdrm,
> >>>> it is being used 3 times. Probably more in Asahi SMC.
> >>> 
> >>> Right, it depends on the usage count. Also on different architectures it may
> >>> give different results. On 32-bit it probably gives better statistics.
> >> 
> >> Best to go ahead with vsprintf then. Petr, are you still there?
> > 
> > I am here but there were many other things in the queue ;-)
> > 
> > I do not have strong opinion. I am not familiar with the FourCC
> > format and it looks like a magic to me. But it seems that it makes
> > sense for the users.
> > 
> > I personally find the %pcX modifiers a bit less hacky than
> > the two macros SMC_KEY_FMT/SMC_KEY_ARG.
> > 
> > So I am fine with this patch:
> > 
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> > Tested-by: Petr Mladek <pmladek@suse.com>
> > 
> > 
> > Now, the question is how to get this patch into the mainline.
> > 
> > Normally, it would make perfect sense to queue it via the DRM tree
> > because drivers/gpu/drm/tiny/appletbdrm.c is a new driver...
> > 
> > But this time there is a conflicting patchset which is reworking
> > the entire lib/test_printf.c file, see
> > 20250307-printf-kunit-convert-v6-0-4d85c361c241@gmail.com
> 
> Link seems to be broken

It works fine. Because it's not a link, it's Message-ID, you need to add
https://lore.kernel.org/r/ in front of it.

> > And it will likely be ready for the next merge window as well.
> > I am going to review it right away.
> > 
> > It is even more complicated because the patchset converting
> > the printf test module to KUNIT depends on another changes
> > in Kees' tree (moving kunit test modules to lib/tests/).
> > So it might be easier when it goes via Kees' tree.
> > 
> > And it might be easier when even this patch goes via Kees' tree.
> > 
> > My proposal:
> > 
> > I suggest to separate the fourcc_pointer() test update
> > to a separate patch and add it later after the merge window
> > when things settle down.
> > 
> > I mean to send the vsprintf.c, checkpatch.pl, and doc update
> > via DRM tree together with the new appletbdrm.c driver.
> 
> Sounds good. At least we can get it working. I’ll make sure the self
> tests get updated once 6.15-rc1 gets released, or Kees can share
> his tree, where I can add the tests as well.
> 
> I’ll send a v2 so that Thomas can take them up.
> > 
> > And update the selftest later when both DRM tree and KUNIT
> > update reaches mainline.
> > 
> > How does that sound, please?

To me sounds good, but I'm not a maintainer involved in all this :-)

-- 
With Best Regards,
Andy Shevchenko

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

[Kees Cook]

On Thu, Mar 13, 2025 at 11:48:32AM +0100, Petr Mladek wrote:
> So I am fine with this patch:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>
> 
> 
> Now, the question is how to get this patch into the mainline.
> 
> Normally, it would make perfect sense to queue it via the DRM tree
> because drivers/gpu/drm/tiny/appletbdrm.c is a new driver...
> 
> But this time there is a conflicting patchset which is reworking
> the entire lib/test_printf.c file, see
> 20250307-printf-kunit-convert-v6-0-4d85c361c241@gmail.com
> And it will likely be ready for the next merge window as well.
> I am going to review it right away.
> 
> It is even more complicated because the patchset converting
> the printf test module to KUNIT depends on another changes
> in Kees' tree (moving kunit test modules to lib/tests/).
> So it might be easier when it goes via Kees' tree.
> 
> And it might be easier when even this patch goes via Kees' tree.
> 
> My proposal:
> 
> I suggest to separate the fourcc_pointer() test update
> to a separate patch and add it later after the merge window
> when things settle down.
> 
> I mean to send the vsprintf.c, checkpatch.pl, and doc update
> via DRM tree together with the new appletbdrm.c driver.
> 
> And update the selftest later when both DRM tree and KUNIT
> update reaches mainline.
> 
> How does that sound, please?

I'm happy to do whatever makes this easiest.

If patch #1 here were rebased onto the "kunit move" tree:
https://web.git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/move-kunit-tests
then I could just take it there?

-Kees

-- 
Kees Cook

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

[Aditya Garg]

Hi

> On 13 Mar 2025, at 11:10 PM, Kees Cook <kees@kernel.org> wrote:
> 
> On Thu, Mar 13, 2025 at 11:48:32AM +0100, Petr Mladek wrote:
>> So I am fine with this patch:
>> 
>> Reviewed-by: Petr Mladek <pmladek@suse.com>
>> Tested-by: Petr Mladek <pmladek@suse.com>
>> 
>> 
>> Now, the question is how to get this patch into the mainline.
>> 
>> Normally, it would make perfect sense to queue it via the DRM tree
>> because drivers/gpu/drm/tiny/appletbdrm.c is a new driver...
>> 
>> But this time there is a conflicting patchset which is reworking
>> the entire lib/test_printf.c file, see
>> 20250307-printf-kunit-convert-v6-0-4d85c361c241@gmail.com
>> And it will likely be ready for the next merge window as well.
>> I am going to review it right away.
>> 
>> It is even more complicated because the patchset converting
>> the printf test module to KUNIT depends on another changes
>> in Kees' tree (moving kunit test modules to lib/tests/).
>> So it might be easier when it goes via Kees' tree.
>> 
>> And it might be easier when even this patch goes via Kees' tree.
>> 
>> My proposal:
>> 
>> I suggest to separate the fourcc_pointer() test update
>> to a separate patch and add it later after the merge window
>> when things settle down.
>> 
>> I mean to send the vsprintf.c, checkpatch.pl, and doc update
>> via DRM tree together with the new appletbdrm.c driver.
>> 
>> And update the selftest later when both DRM tree and KUNIT
>> update reaches mainline.
>> 
>> How does that sound, please?
> 
> I'm happy to do whatever makes this easiest.
> 
> If patch #1 here were rebased onto the "kunit move" tree:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/move-kunit-tests
> then I could just take it there?

I already sent the 1st patch to DRM. I can rebase the test-printf bit to this tree. Sounds good?

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

[Kees Cook]

On March 13, 2025 10:42:10 AM PDT, Aditya Garg <gargaditya08@live.com> wrote:
>I already sent the 1st patch to DRM. I can rebase the test-printf bit to this tree. Sounds good?

Yeah, sounds good to me! Thanks :)

-Kees

-- 
Kees Cook

[PATCH 2/2] drm/appletbdm: use %p4cl instead of %p4cc

[Aditya Garg]

From: Aditya Garg <gargaditya08@live.com>

Due to lack of a proper printk format, %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.

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

Re: [PATCH 2/2] drm/appletbdm: use %p4cl instead of %p4cc

[Thomas Zimmermann]

Hi

Am 12.03.25 um 10:06 schrieb Aditya Garg:
> From: Aditya Garg <gargaditya08@live.com>
>
> Due to lack of a proper printk format, %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.
>
> 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);

If you want to print out protocol-header tokens, why not use formatting 
macros as we do with DRM mode? There's one for the format string [1] and 
one for the argument. [2] It's not as fancy as the conversion code, but 
you'll get something for your driver that is easily understandable.

[1] 
https://elixir.bootlin.com/linux/v6.14-rc4/source/include/drm/drm_modes.h#L425
[2] 
https://elixir.bootlin.com/linux/v6.14-rc4/source/include/drm/drm_modes.h#L431

Best regards
Thomas

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

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH 2/2] drm/appletbdm: use %p4cl instead of %p4cc

[andriy.shevchenko]

On Wed, Mar 12, 2025 at 12:51:33PM +0100, Thomas Zimmermann wrote:
> Am 12.03.25 um 10:06 schrieb Aditya Garg:

...

> If you want to print out protocol-header tokens, why not use formatting
> macros as we do with DRM mode? There's one for the format string [1] and one
> for the argument. [2] It's not as fancy as the conversion code, but you'll
> get something for your driver that is easily understandable.

The benefit of the specific code formatters as in this patch at least in the
stack usage and hence a lot of code generated again and again. I believe you
can get rid of dozens of (kilo?) bytes in DRM on top of compensating this in
the printf() implementation. This backs us to the discussion on how the best
would be to implement custom printf() specifiers...

-- 
With Best Regards,
Andy Shevchenko