On Wed, Aug 22, 2018 at 4:59 PM, Eric Engestrom
[off-list ref] wrote:

On Tuesday, 2018-08-21 17:44:17 +0100, Brian Starkey wrote:

quoted

Hi Matthew,

On Tue, Aug 21, 2018 at 09:26:39AM -0700, Matthew Wilcox wrote:

quoted

On Tue, Aug 21, 2018 at 05:16:11PM +0100, Brian Starkey wrote:

quoted

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)

Looks reasonable. I'd even put the #define right within each enum line
(as a reminder so people don't forget to add them. Would happily ack a
patch to mass-convert, if that ups the odds of good kerneldoc for all
this.

enum also should support the inline style of kerneldoc (otherwise I
guess we'd need to fix that first, or it just makes no sense at all).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch