Re: [PATCH v2 10/10] Documentation: gpio: document configfs interface for gpio-aggregator

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Den-san.

On Mon, 3 Feb 2025 at 04:14, Koichiro Den <koichiro.den@canonical.com> wrote:
> Add documentation for the newly added configfs-based interface for GPIO
> aggregator.
>
> Signed-off-by: Koichiro Den <koichiro.den@canonical.com>

Thanks for your patch!

> --- a/Documentation/admin-guide/gpio/gpio-aggregator.rst
> +++ b/Documentation/admin-guide/gpio/gpio-aggregator.rst
> @@ -69,6 +69,99 @@ write-only attribute files in sysfs.
>                     $ echo gpio-aggregator.0 > delete_device
>
>
> +Aggregating GPIOs using Configfs
> +--------------------------------
> +
> +**Group:** ``/config/gpio-aggregator``
> +
> +    This is the root directory of the gpio-aggregator configfs tree.
> +
> +**Group:** ``/config/gpio-aggregator/<example-name>``
> +
> +    This directory represents a GPIO aggregator device. You can assign any
> +    name to ``<example-name>`` (e.g., ``agg0``), except names starting with
> +    ``_auto`` prefix, which are reserved for auto-generated configfs
> +    entries corresponding to devices created via Sysfs.
> +
> +**Attribute:** ``/config/gpio-aggregator/<example-name>/live``
> +
> +    The ``live`` attribute allows to trigger the actual creation of the device
> +    once it's fully configured. The accepted values are: ``1`` to enable the
> +    virtual device and ``0`` to disable and tear it down.

As the code uses kstrtobool(), it accepts variants of
yes/true/on/no/false/off, too.

> +
> +**Attribute:** ``/config/gpio-aggregator/<example-name>/dev_name``
> +
> +    The read-only ``dev_name`` attribute exposes the name of the device as it
> +    will appear in the system on the platform bus (e.g. ``gpio-aggregator.0``).
> +    This is useful for identifying a character device for the newly created
> +    aggregator. If it's ``gpio-aggregator.0``,
> +    ``/sys/devices/platform/gpio-aggregator.0/gpiochipX`` path tells you that the
> +    GPIO device id is ``X``.
> +
> +You must create subdirectories for each virtual line you want to
> +instantiate, named exactly as ``line0``, ``line1``, ..., ``lineY``, when
> +you want to instantiate ``Y+1`` (Y >= 0) lines.  Configure all lines before
> +activating the device by setting ``live`` to 1.
> +
> +**Group:** ``/config/gpio-aggregator/<example-name>/<lineY>/``
> +
> +    This directory represents a GPIO line to include in the aggregator.
> +
> +**Attribute:** ``/config/gpio-aggregator/<example-name>/<lineY>/key``
> +
> +**Attribute:** ``/config/gpio-aggregator/<example-name>/<lineY>/offset``
> +
> +    If ``offset`` is >= 0:
> +        * ``key`` specifies the name of the chip this GPIO belongs to
> +        * ``offset`` specifies the line offset within that chip.
> +    If ``offset`` is <0:

Missing space before 0.
Please add "(default)", so the user knows he can skip writing to this
file when specifying a GPIO line name.

> +        * ``key`` needs to specify the GPIO line name.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds