Re: [PATCH v5 00/17] gpiolib: acpi: pin configuration fixes

[Hans de Goede <hdegoede@xxxxxxxxxx>]

Hi,

On 11/9/20 9:53 PM, Andy Shevchenko 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)
> 
> 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 do 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 entire series looks good to me, feel free to add my:

Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

to patches 1-16 (patch 17 makes sense to me, but I do not feel
I'm the right person to ack it).

Regards,

Hans




> 
> 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
> Bart's for-current followed by Linus' fixes followed by Bart's for-next
> followed by Linus' for-next branches as prerequisites to the series.
> 
> Cc: Jamie McClymont <jamie@xxxxxxxxxx>
> 
> [1]: https://lore.kernel.org/linux-gpio/20201106181938.GA41213@xxxxxxxxxxxxxxxxxx/
> 
> 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: Add temporary variable to gpiod_set_transitory() for cleaner
>     code
>   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: Extract 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 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
> 
>  MAINTAINERS                   |   1 +
>  drivers/gpio/gpiolib-acpi.c   | 130 ++++++++++++++++++++--------------
>  drivers/gpio/gpiolib-acpi.h   |   2 +
>  drivers/gpio/gpiolib.c        |  97 ++++++++++++++-----------
>  drivers/gpio/gpiolib.h        |   1 +
>  include/linux/gpio/consumer.h |   4 +-
>  6 files changed, 141 insertions(+), 94 deletions(-)
>