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

[Jani Nikula]

On Fri, 26 Aug 2016, Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx> wrote:
> Em Fri, 26 Aug 2016 11:34:38 +0200
> Markus Heiser <markus.heiser@xxxxxxxxxxx> escreveu:
>
>> Am 23.08.2016 um 16:43 schrieb Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>:
>> 
>> > Em Mon, 22 Aug 2016 14:57:40 -0600
>> > Jonathan Corbet <corbet@xxxxxxx> 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:

Ditto.

Jani.

>
> - 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

-- 
Jani Nikula, Intel Open Source Technology Center