public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Linus Walleij <linusw@kernel.org>,
	 Bartosz Golaszewski <brgl@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	 Shuah Khan <skhan@linuxfoundation.org>,
	linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Documentation: gpio: update the preferred method for using software node lookup
Date: Tue, 31 Mar 2026 11:41:19 -0700	[thread overview]
Message-ID: <acwTkgJ3BFAYIoLV@google.com> (raw)
In-Reply-To: <20260331-doc-gpio-swnodes-v1-1-3f84c268999b@oss.qualcomm.com>

Hi Bartosz,

On Tue, Mar 31, 2026 at 02:28:29PM +0200, Bartosz Golaszewski wrote:
> In its current version, the manual for converting of board files from
> using GPIO lookup tables to software nodes recommends leaving the
> software nodes representing GPIO controllers as "free-floating", not
> attached objects and relying on the matching of their names against the
> GPIO controller's name. This is an abuse of the software node API and
> makes it impossible to create fw_devlinks between GPIO suppliers and
> consumers in this case. We want to remove this behavior from GPIOLIB and
> to this end, work on converting all existing drivers to using "attached"
> software nodes.
> 
> Except for a few corner-cases where board files define consumers
> depending on GPIO controllers described in firmware - where we need to
> reference a real firmware node from a software node - which requires a
> more complex approach, most board files can easily be converted to using
> propert firmware node lookup.
> 
> Update the documentation to recommend attaching the GPIO chip's software
> nodes to the actual platform devices and show how to do it.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>  Documentation/driver-api/gpio/board.rst         | 15 +++++++++---
>  Documentation/driver-api/gpio/legacy-boards.rst | 32 ++++++++++++++++++-------
>  2 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/driver-api/gpio/board.rst b/Documentation/driver-api/gpio/board.rst
> index 0993cac891fb5e4887a1aee6deae273197c6aae1..c2880533742b1b55108f28853a3903cb273fe791 100644
> --- a/Documentation/driver-api/gpio/board.rst
> +++ b/Documentation/driver-api/gpio/board.rst
> @@ -108,9 +108,8 @@ macro, which ties a software node representing the GPIO controller with
>  consumer device. It allows consumers to use regular gpiolib APIs, such as
>  gpiod_get(), gpiod_get_optional().
>  
> -The software node representing a GPIO controller need not be attached to the
> -GPIO controller device. The only requirement is that the node must be
> -registered and its name must match the GPIO controller's label.
> +The software node representing a GPIO controller must be attached to the
> +GPIO controller device - either as the primary or the secondary firmware node.
>  
>  For example, here is how to describe a single GPIO-connected LED. This is an
>  alternative to using platform_data on legacy systems.
> @@ -153,6 +152,16 @@ alternative to using platform_data on legacy systems.
>  	};
>  	software_node_register_node_group(swnodes);
>  
> +	/*
> +	 * 5. Attach the GPIO controller's software node to the device and
> +	 *    register it.
> +	 */
> +	 static void gpio_foo_register(void)
> +	 {
> +		gpio_foo_pdev.dev.fwnode = software_node_fwnode(&gpio_controller_node);
> +		platform_device_register(&gpio_foo_pdev);

Maybe nudge people towards platform_device_register_full() with
info.swnode set to the software node? The change adding swnode to
platform_device_info is already in an immutable branch in driver core
tree and will be merged in the next release.

> +	 }
> +
>  	// Then register a platform_device for "leds-gpio" and associate
>  	// it with &led_device_swnode via .fwnode.
>  
> diff --git a/Documentation/driver-api/gpio/legacy-boards.rst b/Documentation/driver-api/gpio/legacy-boards.rst
> index 46e3a26dba772e5e5117866b5d202e76c8e2adf2..fac63dd38d5b71c3bf43b5286a432f6449f422d0 100644
> --- a/Documentation/driver-api/gpio/legacy-boards.rst
> +++ b/Documentation/driver-api/gpio/legacy-boards.rst
> @@ -36,12 +36,10 @@ Requirements for GPIO Properties
>  When using software nodes to describe GPIO connections, the following
>  requirements must be met for the GPIO core to correctly resolve the reference:
>  
> -1.  **The GPIO controller's software node "name" must match the controller's
> -    "label".** The gpiolib core uses this name to find the corresponding
> -    struct gpio_chip at runtime.
> -    This software node has to be registered, but need not be attached to the
> -    device representing the GPIO controller that is providing the GPIO in
> -    question. It may be left as a "free floating" node.
> +1.  **The GPIO controller's software node must be registered and attached to
> +    the controller's ``struct device`` either as its primary or secondary
> +    firmware node.** The gpiolib core uses the address of the firmware node to
> +    find the corresponding ``struct gpio_chip`` at runtime.
>  
>  2.  **The GPIO property must be a reference.** The ``PROPERTY_ENTRY_GPIO()``
>      macro handles this as it is an alias for ``PROPERTY_ENTRY_REF()``.
> @@ -75,6 +73,11 @@ A typical legacy board file might look like this:
>  
>    #define MYBOARD_GPIO_CONTROLLER "gpio-foo"
>  
> +  static struct platform_device myboard_gpio = {
> +        .name = MYBOARD_GPIO_CONTROLLER,
> +        .id = PLATFORM_DEVID_NONE,
> +  };

Same here, let's use platform_device_info.

> +
>    /* LED setup */
>    static const struct gpio_led myboard_leds[] = {
>    	{
> @@ -124,6 +127,7 @@ A typical legacy board file might look like this:
>    	gpiod_add_lookup_table(&myboard_leds_gpios);
>    	gpiod_add_lookup_table(&myboard_buttons_gpios);
>  
> +        platform_device_register(&myboard_gpio);

Use tabs.

>    	platform_device_register_data(NULL, "leds-gpio", -1,
>    				      &myboard_leds_pdata, sizeof(myboard_leds_pdata));
>    	platform_device_register_data(NULL, "gpio-keys", -1,
> @@ -141,8 +145,7 @@ Step 1: Define the GPIO Controller Node
>  ***************************************
>  
>  First, define a software node that represents the GPIO controller that the
> -LEDs and buttons are connected to. The ``name`` of this node must match the
> -name of the driver for the GPIO controller (e.g., "gpio-foo").
> +LEDs and buttons are connected to. The ``name`` of this node is optional.
>  
>  .. code-block:: c
>  
> @@ -257,6 +260,16 @@ software nodes using the ``fwnode`` field in struct platform_device_info.
>    	if (error)
>    		return error;
>  
> +  	memset(&pdev_info, 0, sizeof(pdev_info));
> +  	pdev_info.name = MYBOARD_GPIO_CONTROLLER;
> +  	pdev_info.id = PLATFORM_DEVID_NONE;
> +  	pdev_info.fwnode = software_node_fwnode(&myboard_gpio_controller_node);

	pdev_info.swnode = ...

> +  	gpio_pdev = platform_device_register_full(&pdev_info);
> +  	if (IS_ERR(gpio_pdev)) {
> +  		error = PTR_ERR(gpio_pdev);
> +  		goto err_unregister_nodes;
> +  	}
> +
>    	memset(&pdev_info, 0, sizeof(pdev_info));
>    	pdev_info.name = "leds-gpio";
>    	pdev_info.id = PLATFORM_DEVID_NONE;
> @@ -264,6 +277,7 @@ software nodes using the ``fwnode`` field in struct platform_device_info.
>    	leds_pdev = platform_device_register_full(&pdev_info);
>    	if (IS_ERR(leds_pdev)) {
>    		error = PTR_ERR(leds_pdev);
> +  		platform_device_unregister(gpio_pdev);
>    		goto err_unregister_nodes;
>    	}
>  
> @@ -274,6 +288,7 @@ software nodes using the ``fwnode`` field in struct platform_device_info.
>    	keys_pdev = platform_device_register_full(&pdev_info);
>    	if (IS_ERR(keys_pdev)) {
>    		error = PTR_ERR(keys_pdev);
> +  		platform_device_unregister(gpio_pdev);
>    		platform_device_unregister(leds_pdev);
>    		goto err_unregister_nodes;
>    	}
> @@ -289,6 +304,7 @@ software nodes using the ``fwnode`` field in struct platform_device_info.
>    {
>    	platform_device_unregister(keys_pdev);
>    	platform_device_unregister(leds_pdev);
> +	platform_device_unregister(gpio_pdev);
>    	software_node_unregister_node_group(myboard_swnodes);
>    }

Thanks.

-- 
Dmitry

      reply	other threads:[~2026-03-31 18:41 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-31 12:28 [PATCH] Documentation: gpio: update the preferred method for using software node lookup Bartosz Golaszewski
2026-03-31 18:41 ` Dmitry Torokhov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=acwTkgJ3BFAYIoLV@google.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=brgl@kernel.org \
    --cc=corbet@lwn.net \
    --cc=linusw@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox