Em Mon, 18 Dec 2017 17:32:11 +0200 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> escreveu: > Hi Mauro, > > On Monday, 18 December 2017 17:13:26 EET Mauro Carvalho Chehab wrote: > > Em Fri, 13 Oct 2017 15:38:11 +0300 Laurent Pinchart escreveu: > > > On Thursday, 28 September 2017 00:46:51 EEST Mauro Carvalho Chehab wrote: > > >> Currently, there's no way to document #define foo <value> > > >> with kernel-doc. So, convert it to an enum, and document. > > > > > > The documentation seems fine to me (except for one comment below). > > > However, converting macros to an enum just to work around a defect of the > > > documentation system doesn't seem like a good idea to me. I'd rather find > > > a way to document macros. > > > > I agree that this limitation should be fixed. > > > > Yet, in this specific case where we have an "array" of defines, all > > associated to the same field (even being a bitmask), and assuming that > > we would add a way for kernel-doc to parse this kind of defines > > (not sure how easy/doable would be), then, in order to respect the > > way kernel-doc markup is, the documentation for those macros would be: > > > > > > /** > > * define: Just log the ioctl name + error code > > */ > > #define V4L2_DEV_DEBUG_IOCTL 0x01 > > /** > > * define: Log the ioctl name arguments + error code > > */ > > #define V4L2_DEV_DEBUG_IOCTL_ARG 0x02 > > /** > > * define: Log the file operations open, release, mmap and get_unmapped_area > > */ > > #define V4L2_DEV_DEBUG_FOP 0x04 > > /** > > * define: Log the read and write file operations and the VIDIOC_(D)QBUF > > ioctls */ > > #define V4L2_DEV_DEBUG_STREAMING 0x08 > > > > IMHO, this is a way easier to read/understand by humans, and a way more > > coincise: > > > > /** > > * enum v4l2_debug_flags - Device debug flags to be used with the video > > * device debug attribute > > * > > * @V4L2_DEV_DEBUG_IOCTL: Just log the ioctl name + error code. > > * @V4L2_DEV_DEBUG_IOCTL_ARG: Log the ioctl name arguments + error code. > > * @V4L2_DEV_DEBUG_FOP: Log the file operations and open, release, > > * mmap and get_unmapped_area syscalls. > > * @V4L2_DEV_DEBUG_STREAMING: Log the read and write syscalls and > > * :c:ref:`VIDIOC_[Q|DQ]BUFF <VIDIOC_QBUF>` ioctls. > > */ > > > > It also underlines the aspect that those names are grouped altogether. > > > > So, IMHO, the main reason to place them inside an enum and document > > as such is that it looks a way better for humans to read. > > As we're talking about extending kerneldoc to document macros, we're free to > decide on a format that would make it easy and clear. Based on your above > example, we could write it > > /** > * define v4l2_debug_flags - Device debug flags to be used with the video > * device debug attribute > * > * @V4L2_DEV_DEBUG_IOCTL: Just log the ioctl name + error code. > * @V4L2_DEV_DEBUG_IOCTL_ARG: Log the ioctl name arguments + error code. > * @V4L2_DEV_DEBUG_FOP: Log the file operations and open, release, > * mmap and get_unmapped_area syscalls. > * @V4L2_DEV_DEBUG_STREAMING: Log the read and write syscalls and > * :c:ref:`VIDIOC_[Q|DQ]BUFF <VIDIOC_QBUF>` ioctls. > */ > > That would be simple, clear, and wouldn't require a code change to workaround > a limitation of the documentation system. That works for me but that would be against the way other things are parsed by kernel-doc. Besides that, I suspect that a patchset adding support for it at kernel-doc will be painful, as it currently assumes that a /** * ... */ markup refers just to the first non-blank line just below it. So, such patch will likely need to add a hack in order to support it, but I may be wrong. I guess we'll end by needing support for #define foo bar type of defines some day. Anyway, I suspect that, for those debug macros, the best is to change it to an enum of bits. I sent a proposal with such approach: https://patchwork.linuxtv.org/patch/46092/ Thanks, Mauro
