Re: [PATCH 0/3] RFC: The beginning of a proper driver-api book

[Mauro Carvalho Chehab <mchehab@s-opensource.com>]

Em Fri, 26 Aug 2016 11:34:38 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> Am 23.08.2016 um 16:43 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>:
> 
> > Em Mon, 22 Aug 2016 14:57:40 -0600
> > Jonathan Corbet <corbet@lwn.net> escreveu:
> >   
> >> This short series convers device-drivers.tmpl into the RST format, splits
> >> it up, and sets up the result under Documentation/driver-api/.  For added
> >> fun, I've taken one top-level file (hsi.txt) and folded it into the
> >> document as a way of showing the direction I'm thinking I would like things
> >> to go.  There is plenty more of this sort of work that could be done, to
> >> say the least - this is just a beginning!
> >> 
> >> The formatted results can be seen at:
> >> 
> >>   http://static.lwn.net/kerneldoc/driver-api/index.html  
> > 
> > Thanks for doing that! IMHO, the conversion of this book is indeed
> > one of the first things to be done.  
> 
> >> As part of the long-term task to turn Documentation/ into less of a horror
> >> movie, I'd like to collect documentation of the driver-specific API here.  
> 
> Hi, 
> 
> here are my 2cent, about the *generic* content from the kernel-doc
> directive:
> 
> .. kernel-doc:: kernel/sched/core.c
>   :export:
> 
> IMHO directives like the one above are to *generic*. If I read this directive
> I would expect, that all exported symbols are documented. But this is a false
> estimation!
> 
> In my POC I use a more restrictive kernel-doc parser 
> (https://github.com/return42/linuxdoc). For the directive above the parser
> logs, that some of the exported symbols are not found / not documented:
> 
> $ kernel-doc --quiet --list-exports kernel/sched/core.c
> [exported undocumented  ] set_cpus_allowed_ptr 
> [exported undocumented  ] kick_process 
> [exported function      ] wake_up_process 
> [exported undocumented  ] preempt_notifier_inc 
> [exported undocumented  ] preempt_notifier_dec 
> [exported function      ] preempt_notifier_register 
> [exported function      ] preempt_notifier_unregister 
> [exported undocumented  ] single_task_running 
> [exported undocumented  ] preempt_count_add 
> [exported undocumented  ] preempt_count_sub 
> [exported undocumented  ] schedule 
> [exported undocumented  ] preempt_schedule 
> [exported function      ] preempt_schedule_notrace 
> [exported undocumented  ] default_wake_function 
> [exported undocumented  ] set_user_nice 
> [exported function      ] sched_setscheduler 
> [exported undocumented  ] sched_setattr 
> [exported function      ] sched_setscheduler_nocheck 
> [exported undocumented  ] _cond_resched 
> [exported undocumented  ] __cond_resched_lock 
> [exported undocumented  ] __cond_resched_softirq 
> [exported function      ] yield 
> [exported function      ] yield_to 
> [exported undocumented  ] io_schedule_timeout 
> [exported undocumented  ] __might_sleep 
> [exported undocumented  ] ___might_sleep 
> 
> 
> The driver-api is full of *generic* content and IMHO it is not really clear 
> what would be a part of the resulting documentation. To illustrate, you
> can take a look on the (old) *automatic* conversion of mine at:
> 
>  http://return42.github.io/sphkerneldoc/books/device-drivers/index.html
> 
> There you see a list of 'Oops: Document generation inconsistency.' 
> This kind of missing documentation grows up with changes to 
> the source files while there are no errors reported.
> 
> What I mean: in most use cases it is better to be explicit and name the 
> functions, structs or whatever which should be a part of the documentation.
> e.g.::
> 
>  .. kernel-doc:: kernel/sched/core.c
>     :functions: wake_up_process yield ...
> 
> By being explicit, the kernel-doc parser has a chance to identify requested
> but missing documentation and log related error messages.
> 
> Summarized:
> 
> - *explicit* is better than implicit.
> - the *generic* part of kernel-doc parser should more restrictive
> 
> These are my thoughts, even if we do nothing to handle it, we 
> should aware about this.

I actually prefer the opposite:

- on a *.c file, it should assume that *all* exported symbols should be
  documented (either at the C code itself or at a header file);

- on a *.h file, it should assume that *all* structs, enums, typedefs,
  function prototypes and static inline functions should be documented.
  As I stated before, we should also add a way to document defines.
  Assuming that we add such way, for defines, it should implicitly
  ignore the ones used inside the header to enable/disable part of
  its contents, like:
	#define _FOO_H_
	#ifndef _FOO_H_
		....
	#endif

Then, add an option to allow explicitly ignoring symbols. The lack
of documentation for a symbol that matches the above criteria and
isn't explicitly ignored should be warned, as this is a documentation
gap that should be fixed.

Thanks,
Mauro