On Wed, 11 Dec 2019 21:42:58 +0100
Arnd Bergmann <arnd@xxxxxxxx> wrote:
> Documentation/process/botching-up-ioctls.rst was orignally
> written as a blog post for DRM driver writers, so it it misses
> some points while going into a lot of detail on others.
>
> Try to provide a replacement that addresses typical issues
> across a wider range of subsystems, and follows the style of
> the core-api documentation better.
>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
Thanks for improving the docs! I have a few nits outside of the content
itself.
> Documentation/core-api/index.rst | 1 +
> Documentation/core-api/ioctl.rst | 250 +++++++++++++++++++++++++++++++
> 2 files changed, 251 insertions(+)
> create mode 100644 Documentation/core-api/ioctl.rst
So you left the old document in place; was that intentional?
> diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
> index ab0eae1c153a..3f28b2f668be 100644
> --- a/Documentation/core-api/index.rst
> +++ b/Documentation/core-api/index.rst
> @@ -39,6 +39,7 @@ Core utilities
> ../RCU/index
> gcc-plugins
> symbol-namespaces
> + ioctl
>
>
> Interfaces for kernel debugging
> diff --git a/Documentation/core-api/ioctl.rst b/Documentation/core-api/ioctl.rst
> new file mode 100644
> index 000000000000..cb2c86ae63e7
> --- /dev/null
> +++ b/Documentation/core-api/ioctl.rst
> @@ -0,0 +1,250 @@
> +======================
> +ioctl based interfaces
> +======================
> +
> +:c:func:`ioctl` is the most common way for applications to interface
Please don't use :c:func: anymore. If you just say "ioctl()" the right
thing will happen (which is nothing here, since there isn't anything that
makes sense to link to in the internal kernel context).
We need a checkpatch rule for :c:func: I guess.
Similarly, later on you have:
> +Timeout values and timestamps should ideally use CLOCK_MONOTONIC time,
> +as returned by ``ktime_get_ns()`` or ``ktime_get_ts64()``. Unlike
> +CLOCK_REALTIME, this makes the timestamps immune from jumping backwards
> +or forwards due to leap second adjustments and clock_settime() calls.
Making those functions ``literal`` will defeat the automatic
cross-referencing. Better to just say ktime_get_ns() without quotes.
[...]
> +* On the x86-32 (i386) architecture, the alignment of 64-bit variables
> + is only 32 bit, but they are naturally aligned on most other
> + architectures including x86-64. This means a structure like
> +
> + ::
You don't need the extra lines here; just say "...a structure like::"
> + struct foo {
> + __u32 a;
> + __u64 b;
> + __u32 c;
> + };
> +
Thanks,
jon
