On Wed, Jan 10, 2024 at 11:40:34AM +0000, Phil Howard wrote:
[snip]
> > +
> > +===================================
> > +GPIO Character Device Userspace API
> > +===================================
> > +
> > +This is latest version (v2) of the character device API, as defined in
> > +``include/uapi/linux/gpio.h.``
> > +
> > +.. note::
> > + Do NOT abuse userspace APIs to control hardware that has proper kernel
> > + drivers. There may already be a driver for your use case, and an existing
> > + kernel driver is sure to provide a superior solution to bitbashing
> > + from userspace.
> > +
> > + Read Documentation/driver-api/gpio/drivers-on-gpio.rst to avoid reinventing
> > + kernel wheels in userspace.
>
> I realise this is in part an emotional response, but very much
> "citation needed" on
> this one.
> While I believe Kernel drivers for things are a good idea, I
> don't believe
> userspace libraries are necessarily bad or wrong. They might be the first
> experience a future kernel dev has with hardware. Either way there are multiple
> ecosystems of userspace drivers both existing and thriving right now, and there
> are good reasons to reinvent kernel wheels in userspace.
>
What I stated is kernel policy as I understand it.
What you describe is what I would consider to be prototyping.
If you want play with bitbashing SPI, or whatever, go knock yourself out.
If you want to release that in a product then you really should
be using the kernel driver if one is available.
Bitbashing should be the last resort.
> At least some of these reasons relate to the (incorrectly assumed)
> insurmountable
> nature of kernel development vs just throwing together some Python. Including
> this loaded language just serves to reinforce that.
>
> You catch more flies with honey than with vinegar, so I'd probably soften to:
>
> Before abusing userspace APIs to bitbash drivers for your hardware you should
> read Documentation/driver-api/gpio/drivers-on-gpio.rst to see if your device has
> an existing kernel driver. If not, please consider contributing one.
>
The note is is a rewording of a section of the existing sysfs documentation:
DO NOT ABUSE SYSFS TO CONTROL HARDWARE THAT HAS PROPER KERNEL DRIVERS.
PLEASE READ THE DOCUMENT AT Subsystem drivers using GPIO TO AVOID REINVENTING
KERNEL WHEELS IN USERSPACE. I MEAN IT. REALLY.
So I've already toned down the vineger.
And your suggestion seems at odds with your earlier argument - we should suggest
that such a user write a kernel driver??
> > +
> > + Similarly, for multi-function lines there may be other subsystems, such as
> > + Documentation/spi/index.rst, Documentation/i2c/index.rst,
> > + Documentation/driver-api/pwm.rst, Documentation/w1/index.rst etc, that
> > + provide suitable drivers and APIs for your hardware.
>
> This is good, people trying to do PWM via userspace bitbashing on arbitrary pins
> (sometimes we really do just want to dim a bunch of LEDs without the cost of an
> extra driver IC) is kind of silly in hindsight. If we steer people
> toward the right
> subsystems, perhaps those can be improved for the benefit of all.
>
Indeed, this paragraph is in response to community discussions, one of
which was looking for a something official that says this.
Now there is one.
> > +
> > +Basic examples using the character device API can be found in ``tools/gpio/*``.
> > +
> > +The API is based around two major objects, the :ref:`gpio-v2-chip` and the
> > +:ref:`gpio-v2-line-request`.
> > +
> > +.. _gpio-v2-chip:
> > +
> > +Chip
> > +====
> > +
> > +The Chip represents a single GPIO chip and is exposed to userspace using device
> > +files of the form ``/dev/gpiochipX``.
>
> Is it worth clarifying that - afaik - the numbering of these device
> files is or can
> be arbitrary? Or, in the opposite case, that the order is guaranteed
> by the vendor's
> device tree configuration?
>
I consider that outside the scope of the API.
> > +
> > +Each chip supports a number of GPIO lines,
> > +:c:type:`chip.lines<gpiochip_info>`. Lines on the chip are identified by an
> > +``offset`` in the range from 0 to ``chip.lines - 1``, i.e. `[0,chip.lines)`.
>
> I don't recognise this syntax "`[0,chip.lines)`", typo, or me being clueless?
>
Sadly the latter.
To exand on Andy's response, within the notation '[' and ']' are inclusive,
'(' and ')' are exclusive.
Too esoteric?
[snip]
> > + - - ``EBUSY``
> > +
> > + - The ioctl can't be handled because the device is busy. Typically
> > + returned when an ioctl attempts something that would require the
> > + usage of a resource that was already allocated. The ioctl must not
> > + be retried without performing another action to fix the problem
> > + first.
>
> eg: When a line is already claimed by another process?
>
My preference would be to put a note in the appropriate ioctl than provide
specific examples in the error codes.
The descriptions here should remain general.
That one probably should be explicitly stated in GPIO_V2_GET_LINE_IOCTL.
[snip]
> > +Description
> > +===========
> > +
> > +On success, the requesting process is granted exclusive access to the line
> > +value, write access to the line configuration, and may receive events when
> > +edges are detected on the line, all of which are described in more detail in
> > +:ref:`gpio-v2-line-request`.
> > +
> > +A number of lines may be requested in the one line request, and request
> > +operations are performed on the requested lines by the kernel as atomically
> > +as possible. e.g. gpio-v2-line-get-values-ioctl.rst will read all the
> > +requested lines at once.
> > +
> > +The state of a line, including the value of output lines, is guaranteed to
> > +remain as requested until the returned file descriptor is closed. Once the
> > +file descriptor is closed, the state of the line becomes uncontrolled from
> > +the userspace perspective, and may revert to its default state.
>
> At the behest of the line driver? (an example of a line driver that
> has good reason
> for reverting might be useful here, to indicate that in the general
> case the user
> cannot assume the state of unclaimed lines)
>
I've tried to keep the kernel a black box from the userspace perspective.
And the sentence explicitly includes "from the userspace perspective"
for that reason.
Where I do provide details of the internal workings it remains high
level - "the kernel does this".
I do not want to go into the detais of kernel internal components here
- out of scope.
[snip]
> > +
> > +Description
> > +===========
> > +
> > +Update the configuration of previously requested lines, without releasing the
> > +line or introducing potential glitches.
>
> Is this guaranteed by all line drivers?
>
I'm not sure anything is guaranteed by all the line drivers ;-).
But, AIUI, we should be all good. AFAIAA, and I stand to be corrected if I'm
wrong, none of the actions we perform on the driver would trigger a glitch
unless the driver is buggy.
It certainly wont glitch output line values like releasing and requesting
the line with the new config could - and that is independent of driver.
[snip]
> > +Description
> > +===========
> > +
> > +Set the values of requested output lines.
> > +
> > +Only the values of output lines may be set.
> > +Attempting to set the value of an input line is an error (**EPERM**).
>
> User beware if they come from some cursed ecosystem where writing a value
> to an input line sets or enables/disables the bias,
>
> eg: https://www.arduino.cc/reference/en/language/functions/digital-io/digitalwrite/
>
Everything there boggles the mind.
How does this API do anything that such a user needs to "beware" of?
Here if they use their janky overloading behaviour they get an error and
have to switch to the correct knob. Is that scary ;-).
Cheers,
Kent.
