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

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Tue, Nov 10, 2020 at 4:10 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> 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>

Thanks!

I still would like Mika to have a look and perform a couple more tests myself.
And I have one emergency task right now, so I think we have time to
let others have a look and comment / test / etc.

>
> 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(-)
> >
>


-- 
With Best Regards,
Andy Shevchenko