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

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

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