mbox series

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

Message ID 20221031-gpiolib-swnode-v2-0-81f55af5fa0e@gmail.com
Headers
Series Add support for software nodes to gpiolib |

Message

Dmitry Torokhov Nov. 9, 2022, 12:26 a.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 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 | 128 ++++++++++++++++++++++++
 drivers/gpio/gpiolib-swnode.h |  14 +++
 drivers/gpio/gpiolib.c        | 219 +++++++++++++++++-------------------------
 include/linux/gpio/property.h |  11 +++
 9 files changed, 329 insertions(+), 241 deletions(-)
---
base-commit: b6fc3fddade7a194bd141a49f2689e50f796ef46
change-id: 20221031-gpiolib-swnode-948203f49b23

-- 
Dmitry

Comments

Andy Shevchenko Nov. 9, 2022, 11:29 a.m. UTC | #1 
On Tue, Nov 08, 2022 at 04:26:45PM -0800, Dmitry Torokhov 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.

Thank for an update!

I have almost nothing serious except two nit-picks I think we can address:
- dropping const qualifier for no (?) reason
- having a superfluous check and extra dev_dbg()

If you are are going to address them, feel free to add my Rb tag to
the patches 5 & 6.
Dmitry Torokhov Nov. 9, 2022, 7:32 p.m. UTC | #2 
On Wed, Nov 09, 2022 at 01:29:41PM +0200, Andy Shevchenko wrote:
> On Tue, Nov 08, 2022 at 04:26:45PM -0800, Dmitry Torokhov 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.
> 
> Thank for an update!
> 
> I have almost nothing serious except two nit-picks I think we can address:
> - dropping const qualifier for no (?) reason
> - having a superfluous check and extra dev_dbg()
> 
> If you are are going to address them, feel free to add my Rb tag to
> the patches 5 & 6.

Thank you for the reviews. I addressed most of your comments, but
because I still left the check you were concerned about in I'd like an
explicit reviewed-by on the v3 if you OK giving it.

Thank you.
Andy Shevchenko Nov. 10, 2022, 2:16 p.m. UTC | #3 
On Wed, Nov 09, 2022 at 11:32:31AM -0800, Dmitry Torokhov wrote:
> On Wed, Nov 09, 2022 at 01:29:41PM +0200, Andy Shevchenko wrote:
> > On Tue, Nov 08, 2022 at 04:26:45PM -0800, Dmitry Torokhov wrote:

...

> > Thank for an update!
> > 
> > I have almost nothing serious except two nit-picks I think we can address:
> > - dropping const qualifier for no (?) reason
> > - having a superfluous check and extra dev_dbg()
> > 
> > If you are are going to address them, feel free to add my Rb tag to
> > the patches 5 & 6.
> 
> Thank you for the reviews. I addressed most of your comments, but
> because I still left the check you were concerned about in I'd like an
> explicit reviewed-by on the v3 if you OK giving it.

I gave for patch 6 and for patch 5 I still think we can get rid of the check.