All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Engestrom <eric.engestrom@intel.com>
To: Brian Starkey <brian.starkey@arm.com>
Cc: Matthew Wilcox <willy@infradead.org>,
	alexandru-cosmin.gheorghe@arm.com, corbet@lwn.net,
	airlied@linux.ie, daniel.vetter@ffwll.ch,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, seanpaul@chromium.org,
	liviu.dudau@arm.com, ayan.halder@arm.com
Subject: Re: [PATCH] drm/fourcc: Add DOC: overview comment
Date: Wed, 22 Aug 2018 15:59:24 +0100	[thread overview]
Message-ID: <20180822145924.GA13763@intel.com> (raw)
In-Reply-To: <20180821164416.GA11553@e107564-lin.cambridge.arm.com>

On Tuesday, 2018-08-21 17:44:17 +0100, Brian Starkey wrote:
> Hi Matthew,
> 
> On Tue, Aug 21, 2018 at 09:26:39AM -0700, Matthew Wilcox wrote:
> > On Tue, Aug 21, 2018 at 05:16:11PM +0100, Brian Starkey wrote:
> > > There's a number of things which haven't previously been documented
> > > around the usage of format modifiers. Capture the current
> > > understanding in an overview comment and add it to the rst
> > > documentation.
> > > 
> > > Ideally, the generated documentation would also include documentation
> > > of all of the #defines, but the kernel-doc system doesn't currently
> > > support kernel-doc comments on #define constants.
> > 
> > Can you turn them into enums?  This seems to work ok:
> > 
> > -/* color index */
> > -#define DRM_FORMAT_C8          fourcc_code('C', '8', ' ', ' ') /* [7:0] C */
> > -
> > -/* 8 bpp Red */
> > -#define DRM_FORMAT_R8          fourcc_code('R', '8', ' ', ' ') /* [7:0] R */
> > +enum {
> > +       /* color index */
> > +       DRM_FORMAT_C8   = fourcc_code('C', '8', ' ', ' '), /* [7:0] C */
> > +       /* 8 bpp Red */
> > +       DRM_FORMAT_R8   = fourcc_code('R', '8', ' ', ' '), /* [7:0] R */
> > +};
> > 
> > but I appreciate this is user API and maybe there's some code out there
> > that does #ifndef DRM_FORMAT_C8 ...
> 
> Thanks for the suggestion, Daniel did mention the same. However,
> unfortunately I don't think we can safely change the UAPI header in
> this manner.

You could get the best of both worlds by doing both:

  enum {
    foo = fourcc(...),
    bar = fourcc(...),
  }
  #define foo foo
  #define bar bar

It would mean a bit more code though, but that way these would now be
enums (with all the advantages of enums vs plain literals) and still
pass #ifdef checks :)

(BTW, on the "maybe there's some code that does #ifdef": I can tell you
there is indeed, having written this myself for an out-of-tree driver
for customer-modified kernels that may contain additional formats)

> 
> Cheers,
> -Brian
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2018-08-22 14:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-21 16:16 [PATCH] drm/fourcc: Add DOC: overview comment Brian Starkey
2018-08-21 16:16 ` Brian Starkey
2018-08-21 16:26 ` Matthew Wilcox
2018-08-21 16:44   ` Brian Starkey
2018-08-21 16:44     ` Brian Starkey
2018-08-22 14:59     ` Eric Engestrom [this message]
2018-08-22 15:11       ` Daniel Vetter
2018-08-22 15:11         ` Daniel Vetter
2018-08-22 15:57         ` Brian Starkey
2018-08-23 14:34           ` Matthew Wilcox
2018-08-23 15:40             ` Brian Starkey
2018-08-21 16:51 ` Daniel Vetter
2018-08-21 16:51   ` Daniel Vetter
2018-08-21 17:09   ` Alexandru-Cosmin Gheorghe
2018-08-21 17:09     ` Alexandru-Cosmin Gheorghe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180822145924.GA13763@intel.com \
    --to=eric.engestrom@intel.com \
    --cc=airlied@linux.ie \
    --cc=alexandru-cosmin.gheorghe@arm.com \
    --cc=ayan.halder@arm.com \
    --cc=brian.starkey@arm.com \
    --cc=corbet@lwn.net \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liviu.dudau@arm.com \
    --cc=seanpaul@chromium.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.