Em Mon, 22 Aug 2016 14:57:42 -0600
Jonathan Corbet <corbet@xxxxxxx> escreveu:
> We don't need to keep it as a single large file anymore; split it up so
> that it is easier to manage and the individual sections can be read
> directly as plain files.
>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
> Signed-off-by: Jonathan Corbet <corbet@xxxxxxx>
> ---
> Documentation/driver-api/basics.rst | 120 +++++
Hi Jon,
I noticed several issues on the converted document. Just commenting
a few of them, as they all follow a pattern: kernel-doc markups
needs review during the conversion to RST, because, unfortunately,
the conversion is not transparent, as we would want to.
So, I'm pointing a few issues I noticed below.
> Documentation/driver-api/basics.rst | 120 +++++
At kernel/kthread.c, on the documentation for:
struct task_struct * kthread_create_on_node(int (*threadfn) (void *data, void * data, int node, const char namefmt[], ...)
The original description is:
* If thread is going to be bound on a particular cpu, give its node
* in @node, to get NUMA affinity for kthread stack, or else give NUMA_NO_NODE.
* When woken, the thread will run @threadfn() with @data as its
* argument. @threadfn() can either call do_exit() directly if it is a
* standalone thread for which no one will call kthread_stop(), or
* return when 'kthread_should_stop()' is true (which means
* kthread_stop() has been called). The return value should be zero
* or a negative error number; it will be passed to kthread_stop().
On the output text, you'll see two places with "@:c:func:threadfn()".
The problem here is that threadfn() is a function argument. While this
used to work with DocBooks, now with Sphinx this is not handled well.
I got some other similar cases on media. There, I opted to just remove
the () on some places, or to replace it by \(\) to avoid kernel-doc
to do the wrong thing. See changeset 564aaf69208d ("[media] doc-rst:
add some needed escape codes").
We could, instead, teach kernel-doc to be smarter, but I'm afraid
that there will always be border cases that it won't cover.
The same problem happened with:
void kmsg_dump_rewind(struct kmsg_dumper * dumper)
Also, you should notice that it added several references to
kthread_create(), with is actually a define:
include/linux/kthread.h:#define kthread_create(threadfn, data, namefmt, arg...) \
It probably makes sense to add some markup for kernel-doc to parse it.
> Documentation/driver-api/drivers.rst | 654 -------------------------
At struct device description:
bool:1 offline
Set after successful invocation of bus type’s .:c:func:offline().
At request_firmware_nowait() description, you'all also see:
->:c:func:probe()
(won't mention about other occurrences of c: - you got the idea)
> Documentation/driver-api/message-based.rst | 30 ++
You should probably change NOTES by:
.. note::
Like on this changeset:
f58cad224796 [media] media-entry.h: Fix a note markup
The description for:
int KickStart(MPT_ADAPTER * ioc, int force, int sleepFlag)
Looked weird on my eyes. The original kernel-nano tag is:
/**
* KickStart - Perform hard reset of MPT adapter.
* @ioc: Pointer to MPT_ADAPTER structure
* @force: Force hard reset
* @sleepFlag: Specifies whether the process can sleep
*
* This routine places MPT adapter in diagnostic mode via the
* WriteSequence register, and then performs a hard reset of adapter
* via the Diagnostic register.
*
* Inputs: sleepflag - CAN_SLEEP (non-interrupt thread)
* or NO_SLEEP (interrupt thread, use mdelay)
* force - 1 if doorbell active, board fault state
* board operational, IOC_RECOVERY or
* IOC_BRINGUP and there is an alt_ioc.
* 0 else
*
* Returns:
* 1 - hard reset, READY
* 0 - no reset due to History bit, READY
* -1 - no reset due to History bit but not READY
* OR reset but failed to come READY
* -2 - no reset, could not enter DIAG mode
* -3 - reset but bad FW bit
*/
static int
KickStart(MPT_ADAPTER *ioc, int force, int sleepFlag)
The first input like:
* Inputs: sleepflag - CAN_SLEEP (non-interrupt thread)
Is internally converted to a bold output like:
*Inputs sleepflag - CAN_SLEEP (non-interrupt thread)*
And the remaining arguments will be mangled.
At returns arguments, for example, it should be changed to something
like (untested):
* Returns:
*
* - 1 - hard reset, READY
*
* - 0 - no reset due to History bit, READY
*
* - 1 - no reset due to History bit but not READY
* OR reset but failed to come READY
*
* - -2 - no reset, could not enter DIAG mode
*
* - -3 - reset but bad FW bit
So, basically, all kernel-doc tags should be reviewed for continuation
lines.
...
> diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
> new file mode 100644
> index 000000000000..b50c41011e47
> --- /dev/null
> +++ b/Documentation/driver-api/index.rst
> @@ -0,0 +1,24 @@
> +========================================
> +The Linux driver implementer's API guide
> +========================================
> +
> +The kernel offers a wide variety of interfaces to support the development
> +of device drivers. This document is an only somewhat organized collection
> +of some of those interfaces — it will hopefully get better over time! The
> +available subsections can be seen below.
> +
> +.. class:: toc-title
> +
> + Table of contents
> +
> +.. toctree::
> + :maxdepth: 2
I would add here a
:numbered:
This way, the entire section will be numbered, and you can remove the
manual numbers from the sections from
Documentation/driver-api/serial-interfaces.rst (patch 3/3).
> +
> + basics
> + infrastructure
> + message-based
> + sound
> + frame-buffer
> + input
> + serial-interfaces
> + miscellaneous
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
