diff mbox series

Documentation: gpio: describe uAPI behaviour when hardware doesn't support requested config

Message ID 20240123133828.141222-1-warthog618@gmail.com
State New
Headers show
Series Documentation: gpio: describe uAPI behaviour when hardware doesn't support requested config | expand

Commit Message

Kent Gibson Jan. 23, 2024, 1:38 p.m. UTC 
The existing uAPI documentation does not adequately describe how the kernel
handles the case where the underlying hardware or driver does not support
the requested configuration.

Add a Configuration Support section describing that behaviour to both the
v1 and v2 documentation, and better document the errors returned where the
requested configuration cannot be supported.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---

My bad for this not being part of the recently applied documentation series,
but it didn't occur to me that this wasn't described until about the time
that was being applied.  OTOH this patch is far smaller than a respin of
that series would've been.

I've kept it as a single patch as it is all related, even if it spans v1
and v2.  There is also a trivial typo fix in gpio-handle-set-config-ioctl.rst
that I noticed while I was there that in my eyes didn't warrant a separate
patch.

Cheers,
Kent.

 .../userspace-api/gpio/error-codes.rst        |  3 +-
 .../gpio/gpio-get-lineevent-ioctl.rst         |  6 ++
 .../gpio/gpio-get-linehandle-ioctl.rst        | 39 +++++++++++++
 .../gpio/gpio-handle-set-config-ioctl.rst     |  5 +-
 .../gpio/gpio-v2-get-line-ioctl.rst           | 57 ++++++++++++++++++-
 .../gpio/gpio-v2-line-set-config-ioctl.rst    |  3 +-
 6 files changed, 106 insertions(+), 7 deletions(-)

Comments

Andy Shevchenko Jan. 23, 2024, 3:44 p.m. UTC | #1 
On Tue, Jan 23, 2024 at 3:39 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> The existing uAPI documentation does not adequately describe how the kernel
> handles the case where the underlying hardware or driver does not support
> the requested configuration.
>
> Add a Configuration Support section describing that behaviour to both the
> v1 and v2 documentation, and better document the errors returned where the
> requested configuration cannot be supported.

...

> +Bias             best effort

So, best effort means that in some cases it won't fail. It reminds me
of the baud rate setting in serial (TermIOS). The question here is how
does user space know that it fell in one of such cases? (In termios
the IOCTL updates the respective fields and then user space can get
settings to see what has actually been applied.)

Floating line is not good in some cases and user space really wants to
know that and treat it as an error (if needed). Hence the above Q. I
believe this needs to be explained in the documentation.

...

> +Bias             best effort

Ditto.

...

Personally I would still do two patches per ABI version, but it's up
to Bart what he wants to see at the end.

--
With Best Regards,
Andy Shevchenko
Kent Gibson Jan. 24, 2024, 12:18 a.m. UTC | #2 
On Tue, Jan 23, 2024 at 05:44:52PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 23, 2024 at 3:39 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > The existing uAPI documentation does not adequately describe how the kernel
> > handles the case where the underlying hardware or driver does not support
> > the requested configuration.
> >
> > Add a Configuration Support section describing that behaviour to both the
> > v1 and v2 documentation, and better document the errors returned where the
> > requested configuration cannot be supported.
>
> ...
>
> > +Bias             best effort
>

This documents the behaviour of the uAPI as it stands, so is your
problem with the documentation or the uAPI?

> So, best effort means that in some cases it won't fail. It reminds me
> of the baud rate setting in serial (TermIOS). The question here is how
> does user space know that it fell in one of such cases? (In termios
> the IOCTL updates the respective fields and then user space can get
> settings to see what has actually been applied.)
>

Best effort means it will try, but if it fails it will continue
regardless.  So the configuration is advisory, not strictly required.

As stated in the docs, userspace cannot currently tell, at least not via
the uAPI.

> Floating line is not good in some cases and user space really wants to
> know that and treat it as an error (if needed). Hence the above Q. I
> believe this needs to be explained in the documentation.
>

Indeed, and I think it is explained in the documentation - worst case it
will float.  And you wont know.  That is the way it is.

This originally came about as setting bias is entangled with
setting direction in gpiod_direction_input(), and it is best effort
there.  The reasoning stated in the doc is what I recall from
conversations at the time.

Adding bias support was the first bit of kernel code I wrote so I wasn't
about to go refactoring the guts of gpiolib - though if I were to do it
now I probably would.

If you consider the current behaviour to be a bug then we can change
that behaviour, e.g. clearing the bias setting in the line info (bascially
the desc flags) if setting fails.
But if it is baked into the ABI then we need to extend the uAPI,
e.g. with a flag requesting that the bias config be mandatory.

Cheers,
Kent.
Bartosz Golaszewski Jan. 25, 2024, 8:43 a.m. UTC | #3 
On Tue, Jan 23, 2024 at 2:39 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> The existing uAPI documentation does not adequately describe how the kernel
> handles the case where the underlying hardware or driver does not support
> the requested configuration.
>
> Add a Configuration Support section describing that behaviour to both the
> v1 and v2 documentation, and better document the errors returned where the
> requested configuration cannot be supported.
>
> Signed-off-by: Kent Gibson <warthog618@gmail.com>
> ---

I applied it but shortened the commit title a bit.

Bart
diff mbox series

Patch

diff --git a/Documentation/userspace-api/gpio/error-codes.rst b/Documentation/userspace-api/gpio/error-codes.rst
index edf01f2cf9d2..6bf2948990cd 100644
--- a/Documentation/userspace-api/gpio/error-codes.rst
+++ b/Documentation/userspace-api/gpio/error-codes.rst
@@ -65,7 +65,8 @@  GPIO Error Codes
 
     -  - ``ENXIO``
 
-       -  No device corresponding to this device special file exists.
+       -  Typically returned when a feature requiring interrupt support was
+          requested, but the line does not support interrupts.
 
 .. note::
 
diff --git a/Documentation/userspace-api/gpio/gpio-get-lineevent-ioctl.rst b/Documentation/userspace-api/gpio/gpio-get-lineevent-ioctl.rst
index 7d0b932925c6..09a9254f38cf 100644
--- a/Documentation/userspace-api/gpio/gpio-get-lineevent-ioctl.rst
+++ b/Documentation/userspace-api/gpio/gpio-get-lineevent-ioctl.rst
@@ -48,6 +48,12 @@  to its default state.
 
 Requesting a line already in use is an error (**EBUSY**).
 
+Requesting edge detection on a line that does not support interrupts is an
+error (**ENXIO**).
+
+As with the :ref:`line handle<gpio-get-linehandle-config-support>`, the
+bias configuration is best effort.
+
 Closing the ``chip_fd`` has no effect on existing line events.
 
 Configuration Rules
diff --git a/Documentation/userspace-api/gpio/gpio-get-linehandle-ioctl.rst b/Documentation/userspace-api/gpio/gpio-get-linehandle-ioctl.rst
index c8256afe306e..9112a9d31174 100644
--- a/Documentation/userspace-api/gpio/gpio-get-linehandle-ioctl.rst
+++ b/Documentation/userspace-api/gpio/gpio-get-linehandle-ioctl.rst
@@ -76,6 +76,45 @@  If no bias flags are set then the bias configuration is not changed.
 
 Requesting an invalid configuration is an error (**EINVAL**).
 
+
+.. _gpio-get-linehandle-config-support:
+
+Configuration Support
+---------------------
+
+Where the requested configuration is not directly supported by the underlying
+hardware and driver, the kernel applies one of these approaches:
+
+ - reject the request
+ - emulate the feature in software
+ - treat the feature as best effort
+
+The approach applied depends on whether the feature can reasonably be emulated
+in software, and the impact on the hardware and userspace if the feature is not
+supported.
+The approach applied for each feature is as follows:
+
+==============   ===========
+Feature          Approach
+==============   ===========
+Bias             best effort
+Direction        reject
+Drive            emulate
+==============   ===========
+
+Bias is treated as best effort to allow userspace to apply the same
+configuration for platforms that support internal bias as those that require
+external bias.
+Worst case the line floats rather than being biased as expected.
+
+Drive is emulated by switching the line to an input when the line should not
+be driven.
+
+In all cases, the configuration reported by gpio-get-lineinfo-ioctl.rst
+is the requested configuration, not the resulting hardware configuration.
+Userspace cannot determine if a feature is supported in hardware, is
+emulated, or is best effort.
+
 Return Value
 ============
 
diff --git a/Documentation/userspace-api/gpio/gpio-handle-set-config-ioctl.rst b/Documentation/userspace-api/gpio/gpio-handle-set-config-ioctl.rst
index 8f1e748dccc8..d002a84681ac 100644
--- a/Documentation/userspace-api/gpio/gpio-handle-set-config-ioctl.rst
+++ b/Documentation/userspace-api/gpio/gpio-handle-set-config-ioctl.rst
@@ -41,12 +41,13 @@  line or introducing potential glitches.
 
 The configuration applies to all requested lines.
 
-The same :ref:`gpio-get-linehandle-config-rules` that apply when requesting the
+The same :ref:`gpio-get-linehandle-config-rules` and
+:ref:`gpio-get-linehandle-config-support` that apply when requesting the
 lines also apply when updating the line configuration.
 
 The motivating use case for this command is changing direction of
 bi-directional lines between input and output, but it may be used more
-generally move lines seamlessly from one configuration state to another.
+generally to move lines seamlessly from one configuration state to another.
 
 To only change the value of output lines, use
 gpio-handle-set-line-values-ioctl.rst.
diff --git a/Documentation/userspace-api/gpio/gpio-v2-get-line-ioctl.rst b/Documentation/userspace-api/gpio/gpio-v2-get-line-ioctl.rst
index d76e614c8343..56b975801b6a 100644
--- a/Documentation/userspace-api/gpio/gpio-v2-get-line-ioctl.rst
+++ b/Documentation/userspace-api/gpio/gpio-v2-get-line-ioctl.rst
@@ -74,7 +74,8 @@  If no bias flags are set then the bias configuration is not changed.
 
 The edge flags, ``GPIO_V2_LINE_FLAG_EDGE_xxx``, require
 ``GPIO_V2_LINE_FLAG_INPUT`` to be set and may be combined to detect both rising
-and falling edges.
+and falling edges.  Requesting edge detection from a line that does not support
+it is an error (**ENXIO**).
 
 Only one event clock flag, ``GPIO_V2_LINE_FLAG_EVENT_CLOCK_xxx``, may be set.
 If none are set then the event clock defaults to ``CLOCK_MONOTONIC``.
@@ -86,11 +87,61 @@  The :c:type:`debounce_period_us<gpio_v2_line_attribute>` attribute may only
 be applied to lines with ``GPIO_V2_LINE_FLAG_INPUT`` set. When set, debounce
 applies to both the values returned by gpio-v2-line-get-values-ioctl.rst and
 the edges returned by gpio-v2-line-event-read.rst.  If not
-supported directly by hardware, the debouncing is performed in software by the
-kernel.
+supported directly by hardware, debouncing is emulated in software by the
+kernel.  Requesting debounce on a line that supports neither debounce in
+hardware nor interrupts, as required for software emulation, is an error
+(**ENXIO**).
 
 Requesting an invalid configuration is an error (**EINVAL**).
 
+.. _gpio-v2-get-line-config-support:
+
+Configuration Support
+---------------------
+
+Where the requested configuration is not directly supported by the underlying
+hardware and driver, the kernel applies one of these approaches:
+
+ - reject the request
+ - emulate the feature in software
+ - treat the feature as best effort
+
+The approach applied depends on whether the feature can reasonably be emulated
+in software, and the impact on the hardware and userspace if the feature is not
+supported.
+The approach applied for each feature is as follows:
+
+==============   ===========
+Feature          Approach
+==============   ===========
+Bias             best effort
+Debounce         emulate
+Direction        reject
+Drive            emulate
+Edge Detection   reject
+==============   ===========
+
+Bias is treated as best effort to allow userspace to apply the same
+configuration for platforms that support internal bias as those that require
+external bias.
+Worst case the line floats rather than being biased as expected.
+
+Debounce is emulated by applying a filter to hardware interrupts on the line.
+An edge event is generated after an edge is detected and the line remains
+stable for the debounce period.
+The event timestamp corresponds to the end of the debounce period.
+
+Drive is emulated by switching the line to an input when the line should not
+be actively driven.
+
+Edge detection requires interrupt support, and is rejected if that is not
+supported. Emulation by polling can still be performed from userspace.
+
+In all cases, the configuration reported by gpio-v2-get-lineinfo-ioctl.rst
+is the requested configuration, not the resulting hardware configuration.
+Userspace cannot determine if a feature is supported in hardware, is
+emulated, or is best effort.
+
 Return Value
 ============
 
diff --git a/Documentation/userspace-api/gpio/gpio-v2-line-set-config-ioctl.rst b/Documentation/userspace-api/gpio/gpio-v2-line-set-config-ioctl.rst
index 126c2626ba6b..9b942a8a53ca 100644
--- a/Documentation/userspace-api/gpio/gpio-v2-line-set-config-ioctl.rst
+++ b/Documentation/userspace-api/gpio/gpio-v2-line-set-config-ioctl.rst
@@ -37,7 +37,8 @@  line or introducing potential glitches.
 
 The new configuration must specify the configuration of all requested lines.
 
-The same :ref:`gpio-v2-get-line-config-rules` that apply when requesting the lines
+The same :ref:`gpio-v2-get-line-config-rules` and
+:ref:`gpio-v2-get-line-config-support` that apply when requesting the lines
 also apply when updating the line configuration.
 
 The motivating use case for this command is changing direction of