[Intel-gfx] [PATCH] drm: Documentation style guide

[Daniel Vetter]

On Tue, Dec 15, 2015 at 02:56:10PM +0000, Dave Gordon wrote:
> On 14/12/15 15:39, Thierry Reding wrote:
> >On Wed, Dec 09, 2015 at 05:08:02PM +0100, Daniel Vetter wrote:
> >>Every time I type or review docs this seems a bit different. Try to
> >>document the common style so we can try to unify at least new docs.
> >>
> >>v2: Spelling fixes from Pierre, Laurent and Jani.
> >>
> >>v3: More spelling fixes from Lukas.
> >>
> >>Cc: Pierre Moreau <pierre.morrow at free.fr>
> >>Cc: Jani Nikula <jani.nikula at linux.intel.com>
> >>Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >>Cc: Lukas Wunner <lukas at wunner.de>
> >>Acked-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >>Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> >>Link: http://patchwork.freedesktop.org/patch/msgid/1449564561-3896-1-git-send-email-daniel.vetter@ffwll.ch
> >>---
> >>  Documentation/DocBook/gpu.tmpl | 37 +++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 37 insertions(+)
> >>
> >>diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> >>index 749b8e2f2113..c66d6412f573 100644
> >>--- a/Documentation/DocBook/gpu.tmpl
> >>+++ b/Documentation/DocBook/gpu.tmpl
> >>@@ -124,6 +124,43 @@
> >>      <para>
> >>        [Insert diagram of typical DRM stack here]
> >>      </para>
> >>+  <sect1>
> >>+    <title>Style Guidelines</title>
> >>+    <para>
> >>+      For consistency this documentation uses American English. Abbreviations
> >>+      are written as all-uppercase, for example: DRM, KMS, IOCTL, CRTC, and so
> >>+      on. To aid in reading, documentations make full use of the markup
> >
> >"..., the documentation makes full use of..." and perhaps "makes use of
> >the full set of markup characters that kerneldoc provides".
> >
> >>+      characters kerneldoc provides: @parameter for function parameters, @member
> >>+      for structure members, &structure to reference structures and
> >>+      function() for functions. These all get automatically hyperlinked if
> >>+      kerneldoc for the referenced objects exists. When referencing entries in
> >>+      function vtables please use ->vfunc(). Note that kerneldoc does
> >>+      not support referencing struct members directly, so please add a reference
> >>+      to the vtable struct somewhere in the same paragraph or at least section.
> >>+    </para>
> >>+    <para>
> >>+      Except in special situations (to separate locked from unlocked variants)
> >>+      locking requirements for functions aren't documented in the kerneldoc.
> >>+      Instead locking should be check at runtime using e.g.
> >
> >"should be checked"
> >
> >>+      <code>WARN_ON(!mutex_is_locked(...));</code>. Since it's much easier to
> >>+      ignore documentation than runtime noise this provides more value. And on
> >>+      top of that runtime checks do need to be updated when the locking rules
> >>+      change, increasing the chances that they're correct. Within the
> 
> A few commas to delimit subclauses would make this more readable:
> 
> Since it's much easier to ignore documentation than runtime noise, this
> provides more value. And on top of that, runtime checks have to be updated
> when the locking rules change, thus increasing the chances that they're
> correct.
> 
> >>+      documentation the locking rules should be explained in the relevant
> >>+      structures: Either in the comment for the lock explaining what it
> >>+      protects, or data fields need a note about which lock protects them, or
> >>+      both.
> >
> >I think you're supposed to have the "or" only in the final subsentence:
> >
> >	"either ... protects, data fields need ..., or both."
> 
> Within the documentation, the locking rules should be explained in comments
> on the relevant structures; these comments may be with the lock, explaining
> what it protects, or with the data, noting which lock protects it, or both
> -- in which case they should agree!
> 
> >>+    </para>
> >>+    <para>
> >>+      Functions which have a non-<code>void</code> return value should have a
> >>+      section called "Returns" explaining the expected return values in
> >>+      different cases and their meanings. Currently there's no consensus whether
> >>+      that section name should be all upper-case or not, and whether it should
> >>+      end in a colon or not. Go with the file-local style. Other common section
> >
> >I thought the colon was necessary for kerneldoc to turn it into a
> >section?
> >
> >Overall, long overdue, so thanks for writing it up:
> >
> >Acked-by: Thierry Reding <treding at nvidia.com>

Unfortunately pull request with this already went to Dave before I could
take your feedback into account. Anyone up for a quick follow-up patch
that I could vacuum up in time for 4.5 (i.e. until Thu latest)?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch