On Thu, Dec 12, 2019 at 9:16 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> On Wed, Dec 11, 2019 at 9:53 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> > +``include/uapi/asm-generic/ioctl.h`` provides four macros for defining
> > +ioctl commands that follow modern conventions: ``_IOC``, ``_IOR``,
> > +``_IOW``, and ``_IORW``. These should be used for all new commands,
> > +with the correct parameters:
> > +
> > +_IO/_IOR/_IOW/_IOWR
>
> This says _IO....
>
> > + The macro name determines whether the argument is used for passing
> > + data into kernel (_IOW), from the kernel (_IOR), both (_IOWR) or is
>
> into the kernel
> , or is
>
> > + not a pointer (_IOC). It is possible but not recommended to pass an
> > + integer value instead of a pointer with _IOC.
>
> ...which is not explained here, but _IOC is?
I mean _IO() everywhere, I would consider _IOC an implementation
detail and not document that here.
s/_IOC/_IO/ throughout the document now.
> > +size
> > + The name of the data type pointed to by the argument, the command
> > + number encodes the ``sizeof(size)`` value in a 13-bit or 14-bit integer,
> > + leading to a limit of 8191 bytes for the maximum size of the argument.
> > + Note: do not pass sizeof(type) type into _IOR/IOW, as that will lead
> > + to encoding sizeof(sizeof(type)), i.e. sizeof(size_t).
>
> Looks like "size" could be renamed, to make this more obvious?
Changed to data_type now, which is what I found in some other
documentation.
> > +space timespec exactly. The get_timespec64() and put_timespec64() helper
> > +functions canbe used to ensure that the layout remains compatible with
>
> can be
done.
> > +
> > +``ktime_get_real_ns()`` can be used for CLOCK_REALTIME timestamps that
> > +may be required for timestamps that need to be persistent across a reboot
>
> Drop "may be required for timestamps that"?
done.
> > +* ``long`` and ``unsigned long`` are the size of a register, so
> > + they can be either 32 bit or 64 bit wide and cannot be used in portable
>
> 32-bit or 64-bit (for consistency with the rest of the document)
The convention I was trying to follow is to write
"32-bit userspace" or "32-bit processor"
but
"this type is 32 bit wide"
I wasn't consistent though, changed it now as you suggested.
> > +
> > +* On ARM OABI user space, 16-bit member variables have 32-bit
> > + alignment, making them incompatible with modern EABI kernels.
> > + Conversely, on the m68k architecture, all struct members have at most
> > + 16-bit alignment. These rarely cause problems as neither ARM-OABI nor
>
> "have at most 16-bit alignment" sounds a bit weird to me, as a member
> may have a greater alignment.
> "struct members are not guaranteed to have an alignment greater than 16-bit"?
done.
> > +
> > +As explained for the compat mode, it is best to not avoid any padding in
>
> best to avoid any implicit padding?
done.
> > +
> > +* The complexity of user space access and data structure layout at done
>
> is done
changed
> > +There are many cases in which ioctl is not the best solution for a
> > +problem. Alternatives include
>
> :
done
> > +* System calls are a better choice for a system-wide feature that
> > + is not tied to a physical device or constrained by the file system
> > + permissions of a character device node
> > +
> > +* netlink is the preferred way of configuring any network related
> > + objects through sockets.
> > +
> > +* debugfs is used for ad-hoc interfaces for debugging functionality
> > + that does not need to be exposed as a stable interface to applications.
> > +
> > +* sysfs is a good way to expose the state of an in-kernel object
> > + that is not tied to a file descriptor.
> > +
> > +* configfs can be used for more complex configuration than sysfs
> > +
> > +* A custom file system can provide extra flexibility with a simple
> > + user interface but add a lot of complexity in the implementation.
>
> adds ... to
done.
Thanks for all the suggestions!
Arnd
