Re: [PATCH v7 00/18] gpiolib: acpi: pin configuration fixes

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

+Cc Mika and Hans

Linus, Bart, it would be nice to hear from you about GPIO library
cleanups and in general about the series...

On Thu, Nov 12, 2020 at 12:24 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Thu, Nov 12, 2020 at 12:21 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> >
> > There are fixes (and plenty cleanups) that allow to take into consideration
> > more parameters in ACPI, i.e. bias for GpioInt() and debounce timeout
> > for Operation Regions, Events and GpioInt() resources.
> >
> > During review Hans noted, that gpiod_set_debounce() returns -ENOTSUPP for
> > the cases when feature is not supported either by driver or a controller.
> >
> > It appears that we have slightly messy API here:
> >
> >   FUNC                  Relation with ENOTSUPP
> >
> >   gpiod_set_config()     returns if not supported
> >   gpiod_set_debounce()   as gpiod_set_config() above
> >   gpio_set_debounce()    legacy wrapper on top of gpiod_set_debounce()
> >   gpiod_set_transitory() skips it (returns okay) with debug message
> >   gpio_set_config()      returns if not supported
> >   gpio_set_bias()        skips it (returns okay)
>
> Mika, the above list is all about GPIO descriptors with an exception
> of gpio_set_debounce(). The gpio_set_*(struct gpio_desc *desc, ...)
> schema is used not only for the above set of functions, but also
> another *internal* ones
>
> gpio_set_open_drain_value_commit()
> gpio_set_open_source_value_commit()
>
> Hope this helps.
>
> I think I can move gpio_set_debounce_timeout out of gpiod_* group of
> declarations in gpiolib.h to avoid confusion. Will it work for you?
>
> > Last two functions are internal to GPIO library, while the rest is
> > exported API. In order to be consistent with both naming schemas
> > the series introduces gpio_set_debounce_timeout() that considers
> > the feature optional. New API is only for internal use.
> >
> > While at it, the few first patches clean up the current GPIO library
> > code to unify it to some extend.
> >
> > The above is followed by changes made in ACPI GPIO library part.
> >
> > The bias patch highly depends on Intel pin control driver changes
> > (they are material for v5.10 [1]), due to this and amount of the
> > prerequisite changes this series is probably not supposed to be
> > backported (at least right now).
> >
> > The last patch adds Intel GPIO tree as official one for ACPI GPIO
> > changes.
> >
> > Assuming [1] makes v5.10 this series can be sent as PR to Linus
> > for v5.11 cycle.
> >
> > Note, some patches are also depend to the code from GPIO fixes / for-next
> > repositories. Unfortunately there is no one repository which contains all
> > up to date for-next changes against GPIO subsystem. That's why I have merged
> > Linus' for-next followed by Bart's for-next branches as prerequisites
> > to the series.
> >
> > Cc: Jamie McClymont <jamie@xxxxxxxxxx>
> > Cc: Coiby Xu <coiby.xu@xxxxxxxxx>
> > Cc: Vasile-Laurentiu Stanimir <vasile-laurentiu.stanimir@xxxxxxxxxxxxx>
> >
> > [1]: https://lore.kernel.org/linux-gpio/20201106181938.GA41213@xxxxxxxxxxxxxxxxxx/
> >
> > Changelog v7:
> > - sent correct set of patches
> >
> > Changelog v6:
> > - added tags (Hans, Mika, Linus)
> > - dropped temporary variables in couple of patches (Mika)
> > - dropped "Add temporary variable to gpiod_set_transitory()..." (Mika)
> > - added patch to move assignments outside a lock (Mika)
> > - added patch from Vasile
> > - due to above rebased accordingly the affected ones
> >
> > Changelog v5:
> > - introduced gpio_set_debounce_timeout()
> > - made a prerequisite refactoring in GPIO library code
> > - updated the rest accordingly
> >
> > Changelog v4:
> > - extended debounce setting to ACPI events and Operation Regions
> > - added Ack (Linus)
> > - added few more cleanup patches, including MAINTAINERS update
> >
> > Changelog v3:
> > - dropped upstreamed OF patch
> > - added debounce fix
> >
> > Andy Shevchenko (17):
> >   gpiolib: Replace unsigned by unsigned int
> >   gpiolib: add missed break statement
> >   gpiolib: use proper API to pack pin configuration parameters
> >   gpiolib: Extract gpio_set_config_with_argument() for future use
> >   gpiolib: move bias related code from gpio_set_config() to
> >     gpio_set_bias()
> >   gpiolib: Extract gpio_set_config_with_argument_optional() helper
> >   gpiolib: Introduce gpio_set_debounce_timeout() for internal use
> >   gpiolib: acpi: Respect bias settings for GpioInt() resource
> >   gpiolib: acpi: Use named item for enum gpiod_flags variable
> >   gpiolib: acpi: Take into account debounce settings
> >   gpiolib: acpi: Move non-critical code outside of critical section
> >   gpiolib: acpi: Move acpi_gpio_to_gpiod_flags() upper in the code
> >   gpiolib: acpi: Make acpi_gpio_to_gpiod_flags() usable for GpioInt()
> >   gpiolib: acpi: Extract acpi_request_own_gpiod() helper
> >   gpiolib: acpi: Convert pin_index to be u16
> >   gpiolib: acpi: Use BIT() macro to increase readability
> >   gpiolib: acpi: Make Intel GPIO tree official for GPIO ACPI work
> >
> > Vasile-Laurentiu Stanimir (1):
> >   gpiolib: acpi: Set initial value for output pin based on bias and
> >     polarity
> >
> >  MAINTAINERS                   |   1 +
> >  drivers/gpio/gpiolib-acpi.c   | 138 +++++++++++++++++++++-------------
> >  drivers/gpio/gpiolib-acpi.h   |   2 +
> >  drivers/gpio/gpiolib.c        |  98 ++++++++++++++----------
> >  drivers/gpio/gpiolib.h        |   1 +
> >  include/linux/gpio/consumer.h |   4 +-
> >  6 files changed, 147 insertions(+), 97 deletions(-)
> >
> > --
> > 2.28.0
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
With Best Regards,
Andy Shevchenko