mbox series

[v3,0/6] Add support for software nodes to gpiolib

Message ID 20221031-gpiolib-swnode-v3-0-0282162b0fa4@gmail.com
Headers show
Series Add support for software nodes to gpiolib | expand

Message

Dmitry Torokhov Nov. 9, 2022, 7:30 p.m. UTC
This series attempts to add support for software nodes to gpiolib, using
software node references. This allows us to convert more drivers to the
generic device properties and drop support for custom platform data.

To describe a GPIO via software nodes we can create the following data
items:

/* Node representing the GPIO controller/GPIO bank */
static const struct software_node gpio_bank_b_node = {
        .name = "B",
};

/*
 * Properties that will be assigned to a software node assigned to
 * the device that used platform data.
 */
static const struct property_entry simone_key_enter_props[] = {
        PROPERTY_ENTRY_U32("linux,code", KEY_ENTER),
        PROPERTY_ENTRY_STRING("label", "enter"),
        PROPERTY_ENTRY_REF("gpios", &gpio_bank_b_node, 123, GPIO_ACTIVE_LOW),
        { }
};

The code in gpiolib handling software nodes uses the name in the
software node representing GPIO controller to locate the actual instance
of GPIO controller.

To: Linus Walleij <linus.walleij@linaro.org>
To: Bartosz Golaszewski <brgl@bgdev.pl>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-gpio@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-acpi@vger.kernel.org

---
Changes in v3:
- Addressed more Andy's comments
- Link to v2: https://lore.kernel.org/r/20221031-gpiolib-swnode-v2-0-81f55af5fa0e@gmail.com

Changes in v2:
- reworked the series to be independent of other in-flight patches.
  That meant keeping devm_gpiod_get_from_of_node() for now.
- removed handling of secondary nodes, it deserves a separate patch
  series
- fixed refcounting when handling swnodes (Andy)
- added include/linux/gpio/property.h with PROPERTY_ENTRY_GPIO (Andy)
- addressed most of the rest of Andy's comments
- collected reviewed-by and acked-by
- Link to v1: https://lore.kernel.org/r/20221031-gpiolib-swnode-v1-0-a0ab48d229c7@gmail.com

---
Dmitry Torokhov (6):
      gpiolib: of: change of_find_gpio() to accept device node
      gpiolib: acpi: change acpi_find_gpio() to accept firmware node
      gpiolib: acpi: teach acpi_find_gpio() to handle data-only nodes
      gpiolib: acpi: avoid leaking ACPI details into upper gpiolib layers
      gpiolib: consolidate GPIO lookups
      gpiolib: add support for software nodes

 drivers/gpio/Makefile         |   1 +
 drivers/gpio/gpiolib-acpi.c   | 132 +++++++++++++++-----------
 drivers/gpio/gpiolib-acpi.h   |  54 +----------
 drivers/gpio/gpiolib-of.c     |   7 +-
 drivers/gpio/gpiolib-of.h     |   4 +-
 drivers/gpio/gpiolib-swnode.c | 123 +++++++++++++++++++++++++
 drivers/gpio/gpiolib-swnode.h |  14 +++
 drivers/gpio/gpiolib.c        | 209 ++++++++++++++++--------------------------
 include/linux/gpio/property.h |  11 +++
 9 files changed, 315 insertions(+), 240 deletions(-)
---
base-commit: b6fc3fddade7a194bd141a49f2689e50f796ef46
change-id: 20221031-gpiolib-swnode-948203f49b23

Comments

Dmitry Torokhov Nov. 10, 2022, 5:24 p.m. UTC | #1
On Thu, Nov 10, 2022 at 04:13:57PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 09, 2022 at 11:30:43AM -0800, Dmitry Torokhov wrote:
> > Ensure that all paths to obtain/look up GPIOD from generic
> > consumer-visible APIs go through the new gpiod_find_and_request()
> > helper, so that we can easily extend it with support for new firmware
> > mechanisms.
> > 
> > The only exception is OF-specific [devm_]gpiod_get_from_of_node() API
> > that is still being used by a couple of drivers and will be removed as
> > soon as patches converting them to use generic fwnode/device APIs are
> > accepted.
> 
> ...
> 
> > +	if (!IS_ERR_OR_NULL(fwnode))
> 
> As pointed earlier I still think this is not needed. Even for the sake of
> showing an intent, the not-found fwnode(i.e. GPIO), will be handled anyway...
> 
> > +		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
> > +					    &flags, &lookupflags);
> > +
> > +	if (gpiod_not_found(desc) && platform_lookup_allowed) {
> 
> ...here by gpiod_not_found() which is an exact intention in both cases above
> (fwnode is not provided / invalid or GPIO wasn't found).

Thank you for the thorough reviews.

I think at this point I will leave to to Bart and Linus to decide what
form they prefer here. From the execution point there is no practical
difference, it is all syntactic sugar.

Thanks.
Bartosz Golaszewski Nov. 11, 2022, 9:49 p.m. UTC | #2
On Wed, Nov 9, 2022 at 8:30 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> This series attempts to add support for software nodes to gpiolib, using
> software node references. This allows us to convert more drivers to the
> generic device properties and drop support for custom platform data.
>
> To describe a GPIO via software nodes we can create the following data
> items:
>
> /* Node representing the GPIO controller/GPIO bank */
> static const struct software_node gpio_bank_b_node = {
>         .name = "B",
> };
>
> /*
>  * Properties that will be assigned to a software node assigned to
>  * the device that used platform data.
>  */
> static const struct property_entry simone_key_enter_props[] = {
>         PROPERTY_ENTRY_U32("linux,code", KEY_ENTER),
>         PROPERTY_ENTRY_STRING("label", "enter"),
>         PROPERTY_ENTRY_REF("gpios", &gpio_bank_b_node, 123, GPIO_ACTIVE_LOW),
>         { }
> };
>
> The code in gpiolib handling software nodes uses the name in the
> software node representing GPIO controller to locate the actual instance
> of GPIO controller.
>
> To: Linus Walleij <linus.walleij@linaro.org>
> To: Bartosz Golaszewski <brgl@bgdev.pl>
> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
>
> ---
> Changes in v3:
> - Addressed more Andy's comments
> - Link to v2: https://lore.kernel.org/r/20221031-gpiolib-swnode-v2-0-81f55af5fa0e@gmail.com
>
> Changes in v2:
> - reworked the series to be independent of other in-flight patches.
>   That meant keeping devm_gpiod_get_from_of_node() for now.
> - removed handling of secondary nodes, it deserves a separate patch
>   series
> - fixed refcounting when handling swnodes (Andy)
> - added include/linux/gpio/property.h with PROPERTY_ENTRY_GPIO (Andy)
> - addressed most of the rest of Andy's comments
> - collected reviewed-by and acked-by
> - Link to v1: https://lore.kernel.org/r/20221031-gpiolib-swnode-v1-0-a0ab48d229c7@gmail.com
>
> ---
> Dmitry Torokhov (6):
>       gpiolib: of: change of_find_gpio() to accept device node
>       gpiolib: acpi: change acpi_find_gpio() to accept firmware node
>       gpiolib: acpi: teach acpi_find_gpio() to handle data-only nodes
>       gpiolib: acpi: avoid leaking ACPI details into upper gpiolib layers
>       gpiolib: consolidate GPIO lookups
>       gpiolib: add support for software nodes
>
>  drivers/gpio/Makefile         |   1 +
>  drivers/gpio/gpiolib-acpi.c   | 132 +++++++++++++++-----------
>  drivers/gpio/gpiolib-acpi.h   |  54 +----------
>  drivers/gpio/gpiolib-of.c     |   7 +-
>  drivers/gpio/gpiolib-of.h     |   4 +-
>  drivers/gpio/gpiolib-swnode.c | 123 +++++++++++++++++++++++++
>  drivers/gpio/gpiolib-swnode.h |  14 +++
>  drivers/gpio/gpiolib.c        | 209 ++++++++++++++++--------------------------
>  include/linux/gpio/property.h |  11 +++
>  9 files changed, 315 insertions(+), 240 deletions(-)
> ---
> base-commit: b6fc3fddade7a194bd141a49f2689e50f796ef46
> change-id: 20221031-gpiolib-swnode-948203f49b23
>
> --
> Dmitry
>

Hi Dmitry,

What are the prerequisites of this series as it doesn't apply to my
gpio/for-next branch?

Bart
Dmitry Torokhov Nov. 11, 2022, 10:06 p.m. UTC | #3
On Fri, Nov 11, 2022 at 10:49:32PM +0100, Bartosz Golaszewski wrote:
> On Wed, Nov 9, 2022 at 8:30 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > This series attempts to add support for software nodes to gpiolib, using
> > software node references. This allows us to convert more drivers to the
> > generic device properties and drop support for custom platform data.
> >
> > To describe a GPIO via software nodes we can create the following data
> > items:
> >
> > /* Node representing the GPIO controller/GPIO bank */
> > static const struct software_node gpio_bank_b_node = {
> >         .name = "B",
> > };
> >
> > /*
> >  * Properties that will be assigned to a software node assigned to
> >  * the device that used platform data.
> >  */
> > static const struct property_entry simone_key_enter_props[] = {
> >         PROPERTY_ENTRY_U32("linux,code", KEY_ENTER),
> >         PROPERTY_ENTRY_STRING("label", "enter"),
> >         PROPERTY_ENTRY_REF("gpios", &gpio_bank_b_node, 123, GPIO_ACTIVE_LOW),
> >         { }
> > };
> >
> > The code in gpiolib handling software nodes uses the name in the
> > software node representing GPIO controller to locate the actual instance
> > of GPIO controller.
> >
> > To: Linus Walleij <linus.walleij@linaro.org>
> > To: Bartosz Golaszewski <brgl@bgdev.pl>
> > To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: linux-gpio@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-acpi@vger.kernel.org
> >
> > ---
> > Changes in v3:
> > - Addressed more Andy's comments
> > - Link to v2: https://lore.kernel.org/r/20221031-gpiolib-swnode-v2-0-81f55af5fa0e@gmail.com
> >
> > Changes in v2:
> > - reworked the series to be independent of other in-flight patches.
> >   That meant keeping devm_gpiod_get_from_of_node() for now.
> > - removed handling of secondary nodes, it deserves a separate patch
> >   series
> > - fixed refcounting when handling swnodes (Andy)
> > - added include/linux/gpio/property.h with PROPERTY_ENTRY_GPIO (Andy)
> > - addressed most of the rest of Andy's comments
> > - collected reviewed-by and acked-by
> > - Link to v1: https://lore.kernel.org/r/20221031-gpiolib-swnode-v1-0-a0ab48d229c7@gmail.com
> >
> > ---
> > Dmitry Torokhov (6):
> >       gpiolib: of: change of_find_gpio() to accept device node
> >       gpiolib: acpi: change acpi_find_gpio() to accept firmware node
> >       gpiolib: acpi: teach acpi_find_gpio() to handle data-only nodes
> >       gpiolib: acpi: avoid leaking ACPI details into upper gpiolib layers
> >       gpiolib: consolidate GPIO lookups
> >       gpiolib: add support for software nodes
> >
> >  drivers/gpio/Makefile         |   1 +
> >  drivers/gpio/gpiolib-acpi.c   | 132 +++++++++++++++-----------
> >  drivers/gpio/gpiolib-acpi.h   |  54 +----------
> >  drivers/gpio/gpiolib-of.c     |   7 +-
> >  drivers/gpio/gpiolib-of.h     |   4 +-
> >  drivers/gpio/gpiolib-swnode.c | 123 +++++++++++++++++++++++++
> >  drivers/gpio/gpiolib-swnode.h |  14 +++
> >  drivers/gpio/gpiolib.c        | 209 ++++++++++++++++--------------------------
> >  include/linux/gpio/property.h |  11 +++
> >  9 files changed, 315 insertions(+), 240 deletions(-)
> > ---
> > base-commit: b6fc3fddade7a194bd141a49f2689e50f796ef46
> > change-id: 20221031-gpiolib-swnode-948203f49b23
> >
> > --
> > Dmitry
> >
> 
> Hi Dmitry,
> 
> What are the prerequisites of this series as it doesn't apply to my
> gpio/for-next branch?

Hi Bart,

No prereqs, must be just a small context changes now that you merged
leds changes and the patch removing devm_fwnode_get_[index_]gpiod_from_child().

Let me rebase it on top of latest next tree and resend.