On Tue 2022-09-06 14:19:44, Russell King wrote:
> From: Hector Martin <marcan@xxxxxxxxx>
>
> %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-endian
> %p4cl Little-endian
> %p4cb Big-endian
> %p4cr Reverse-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. %p4cr would
> allow printing LSByte-first FOURCCs stored in host endian order
> (other than the hex form being in character order, not the integer
> value).
>
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -625,6 +625,38 @@ Passed by reference.
> %p4cc Y10 little-endian (0x20303159)
> %p4cc NV12 big-endian (0xb231564e)
>
> +Generic FourCC code
> +-------------------
> +
> +::
> + %p4c[hnbl] gP00 (0x67503030)
> +
> +Print a generic FourCC code, as both ASCII characters and its numerical
> +value as hexadecimal.
> +
> +The additional ``h``, ``r``, ``b``, and ``l`` specifiers are used to specify
> +host, reversed, big or little endian order data respectively. Host endian
> +order means the data is interpreted as a 32-bit integer and the most
> +significant byte is printed first; that is, the character code as printed
> +matches the byte order stored in memory on big-endian systems, and is reversed
> +on little-endian systems.
> +
> +Passed by reference.
> +
> +Examples for a little-endian machine, given &(u32)0x67503030::
> +
> + %p4ch gP00 (0x67503030)
> + %p4cl gP00 (0x67503030)
> + %p4cb 00Pg (0x30305067)
> + %p4cr 00Pg (0x30305067)
Nit: I would prefer to keep the same order (h,r,b,l) everywhere.
I guess that you wanted to show exactly the same results next
to each other. But it is not the case on big-endian anyway.
> +
> +Examples for a big-endian machine, given &(u32)0x67503030::
> +
> + %p4ch gP00 (0x67503030)
> + %p4cl 00Pg (0x30305067)
> + %p4cb gP00 (0x67503030)
> + %p4cr 00Pg (0x30305067)
Same here.
> +
> Thanks
> ======
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3c1853a9d1c0..31707499f90f 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1757,27 +1757,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 pix_fmt = false;
Nit: I would prefer "pixel_fmt". I am not a graphics guy and wondered
what "pix" did stands for ;-)
> 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':
> + val = orig;
> + break;
> + case 'r':
> + val = orig = swab32(orig);
> + break;
> + case 'l':
> + val = orig = le32_to_cpu(orig);
> + break;
> + case 'b':
> + val = orig = be32_to_cpu(orig);
> + break;
> + case 'c':
> + /* Pixel formats are printed LSB-first */
> + val = swab32(orig & ~BIT(31));
> + pix_fmt = true;
> + break;
> + default:
> + return error_string(buf, end, "(%p4?)", spec);
> + }
>
> for (i = 0; i < sizeof(u32); i++) {
> - unsigned char c = val >> (i * 8);
> + unsigned char c = val >> ((3 - i) * 8);
This hardcodes '3' but the for-cycle uses i < sizeof(u32).
We should be consistent.
A solution would be:
int i;
for (i = sizeof(u32); --i >= 0;) {
unsigned char c = val >> (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 (pix_fmt) {
> + *p++ = ' ';
> + strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> + p += strlen(p);
> + }
>
> *p++ = ' ';
> *p++ = '(';
Best Regards,
Petr
