Em Mon, 18 Dec 2017 17:32:11 +0200
Laurent Pinchart <laurent.pinch...@ideasonboard.com> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
