Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Joe Perches <joe@perches.com>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
Date: Thu, 10 Dec 2020 15:55:27 +0200	[thread overview]
Message-ID: <20201210135526.GH25763@paasikivi.fi.intel.com> (raw)
In-Reply-To: <CAHp75VcY_b7uaGWoEa1Y6YDk0MmmzC4hV2yx8zVT7J-fD67Hyg@mail.gmail.com>

Hi Andy,

On Thu, Dec 10, 2020 at 03:05:02PM +0200, Andy Shevchenko wrote:
> On Thu, Dec 10, 2020 at 2:16 PM Petr Mladek <pmladek@suse.com> wrote:
> > On Fri 2020-11-13 12:54:41, Sakari Ailus wrote:
> > > Add a printk modifier %p4cc (for pixel format) for printing V4L2 and DRM
> > > pixel formats denoted by fourccs. The fourcc encoding is the same for both
> > > so the same implementation can be used.
> > >
> > > Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >
> > Andy, Rasmus,
> >
> > the last version looks fine to me. I am going to push it.
> > Please, speak up if you are against it.
> 
> My concerns are:
> - not so standard format of representation (why not to use
> string_escape_mem() helper?) or is it?

The format string may contain spaces that are not meant to be printed.
Other unprintable chacaters should not be present (at least not in V4L2
pixelformats). The hexadecimal representation is there to convey the
numerical value and that originally came from DRM, not V4L2.

> - no compatibility with generic 4cc
>   (I would rather have an additional specifier here for v4l2 cases.

What do you mean by "generic 4cc"? There are two users of 4cc codes in the
kernel that I know of: V4L2 and DRM. Something that does not refer to
in-memory pixel formats?

> OTOH generic %p4cc to me sounds like an equivalent to %4pEh (but we
> have similar cases with MAC where %6ph is the same as %pM).
> 
> But I'm not insisting on them, consider it like just my 2 cents to the
> discussion.

Ack.

-- 
Kind regards,

Sakari Ailus