Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1470627 > unrolled thread

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

Started byMarkus Heiser <markus.heiser@darmarit.de>
First post2016-08-26 11:40 +0200
Last post2016-08-26 12:20 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 0/3] RFC: The beginning of a proper driver-api book Markus Heiser <markus.heiser@darmarit.de> - 2016-08-26 11:40 +0200
    Re: [PATCH 0/3] RFC: The beginning of a proper driver-api book Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2016-08-26 12:10 +0200
      Re: [PATCH 0/3] RFC: The beginning of a proper driver-api book Jani Nikula <jani.nikula@intel.com> - 2016-08-26 12:20 +0200

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

FromMarkus Heiser <markus.heiser@darmarit.de>
Date2016-08-26 11:40 +0200
SubjectRe: [PATCH 0/3] RFC: The beginning of a proper driver-api book
Message-ID<salMd-5aQ-23@gated-at.bofh.it>
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.

-- Markus --
> 
>> Arguably gpu/ and the media API stuff should eventually move here, though
>> we can discuss the color of that particular shed some other day.
> 
> Yeah, let's finish converting everything to rst, and then see what would
> be the best way to organize it.
> 
> I have a split feelings with regards to media kAPI and driver-specific
> stuff if it is better to keep it at the device-drivers book, or at a
> single media book (with currently has 1031 pages on PDF output),
> but let's postpone such discussion.
> 
> I'm afraid that we'll end needing to split it more than what would be
> desired, in order to fix some issues with the PDF output 
> part/chapter/section numbering. Unfortunately, ReST markup is too poor
> to describe multi-part books, and doesn't have anything to describe
> multi-book projects.
> 
>> Meanwhile, I'd appreciate comments on the general idea.
> 
> I like the general idea of breaking device-drivers.tmpl into multiple
> files. IMHO, that will make easier to maintain it long term.
> 
> After looking on all 3 patches, they all look good, as a concept.
> 
> Yet, IMHO, there are a few things to be fixed before merging it. See
> my comments to patch 2/3.
> 
> Regards,
> Mauro
> 
>> 
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
>> Cc: Sebastian Reichel <sre@kernel.org>
>> 
>> Jonathan Corbet (3):
>> Docs: sphinxify device-drivers.tmpl
>> docs: split up the driver book
>> docs: Pull HSI documentation together
>> 
>> Documentation/DocBook/Makefile                 |   2 +-
>> Documentation/DocBook/device-drivers.tmpl      | 521 -------------------------
>> Documentation/driver-api/basics.rst            | 120 ++++++
>> Documentation/driver-api/frame-buffer.rst      |  62 +++
>> Documentation/driver-api/index.rst             |  24 ++
>> Documentation/driver-api/infrastructure.rst    | 169 ++++++++
>> Documentation/driver-api/input.rst             |  51 +++
>> Documentation/driver-api/message-based.rst     |  30 ++
>> Documentation/driver-api/miscellaneous.rst     |  50 +++
>> Documentation/driver-api/serial-interfaces.rst | 189 +++++++++
>> Documentation/driver-api/sound.rst             |  54 +++
>> Documentation/hsi.txt                          |  75 ----
>> Documentation/index.rst                        |   1 +
>> MAINTAINERS                                    |   2 +-
>> 14 files changed, 752 insertions(+), 598 deletions(-)
>> delete mode 100644 Documentation/DocBook/device-drivers.tmpl
>> create mode 100644 Documentation/driver-api/basics.rst
>> create mode 100644 Documentation/driver-api/frame-buffer.rst
>> create mode 100644 Documentation/driver-api/index.rst
>> create mode 100644 Documentation/driver-api/infrastructure.rst
>> create mode 100644 Documentation/driver-api/input.rst
>> create mode 100644 Documentation/driver-api/message-based.rst
>> create mode 100644 Documentation/driver-api/miscellaneous.rst
>> create mode 100644 Documentation/driver-api/serial-interfaces.rst
>> create mode 100644 Documentation/driver-api/sound.rst
>> delete mode 100644 Documentation/hsi.txt
>> 
> 
> 
> 
> Thanks,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-doc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [next] | [standalone]


#1470641

FromMauro Carvalho Chehab <mchehab@s-opensource.com>
Date2016-08-26 12:10 +0200
Message-ID<samff-5zJ-17@gated-at.bofh.it>
In reply to#1470627
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

[toc] | [prev] | [next] | [standalone]


#1470643

FromJani Nikula <jani.nikula@intel.com>
Date2016-08-26 12:20 +0200
Message-ID<samoV-5D1-3@gated-at.bofh.it>
In reply to#1470641
On Fri, 26 Aug 2016, Mauro Carvalho Chehab <mchehab@s-opensource.com> wrote:
> 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:

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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web