On Wed, Aug 22, 2018 at 4:59 PM, Eric Engestrom <eric.engestrom@xxxxxxxxx> wrote: > 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) 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel