| Message ID | 1449677282-30832-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@free.fr> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Lukas Wunner <lukas@wunner.de> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Daniel Vetter <daniel.vetter@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 > + 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." > + </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@nvidia.com>
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@free.fr> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Cc: Lukas Wunner <lukas@wunner.de> >> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: Daniel Vetter <daniel.vetter@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@nvidia.com> > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
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@free.fr> > >>Cc: Jani Nikula <jani.nikula@linux.intel.com> > >>Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>Cc: Lukas Wunner <lukas@wunner.de> > >>Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>Signed-off-by: Daniel Vetter <daniel.vetter@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@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
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 + 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. + <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 + 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. + </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 + names are "Notes" with information for dangerous or tricky corner cases, + and "FIXME" where the interface could be cleaned up. + </para> + </sect1> </chapter> <!-- Internals -->