| Message ID | 20201111222008.39993-1-andriy.shevchenko@linux.intel.com |
|---|---|
| Headers | show |
| Series | gpiolib: acpi: pin configuration fixes | expand |
On Thu, Nov 12, 2020 at 12:21 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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@kwiius.com> > Cc: Coiby Xu <coiby.xu@gmail.com> > Cc: Vasile-Laurentiu Stanimir <vasile-laurentiu.stanimir@windriver.com> > > [1]: https://lore.kernel.org/linux-gpio/20201106181938.GA41213@black.fi.intel.com/ > > 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 >
+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@gmail.com> wrote: > > On Thu, Nov 12, 2020 at 12:21 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> 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@kwiius.com> > > Cc: Coiby Xu <coiby.xu@gmail.com> > > Cc: Vasile-Laurentiu Stanimir <vasile-laurentiu.stanimir@windriver.com> > > > > [1]: https://lore.kernel.org/linux-gpio/20201106181938.GA41213@black.fi.intel.com/ > > > > 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
On Thu, Nov 12, 2020 at 3:16 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > Linus, Bart, it would be nice to hear from you about GPIO library > cleanups and in general about the series... The series: Acked-by: Linus Walleij <linus.walleij@linaro.org> Can you send me a pull request for the contents of the v7 patch series so I can pull it in? Yours, Linus Walleij
On Sun, Nov 15, 2020 at 05:04:08PM +0100, Linus Walleij wrote: > On Thu, Nov 12, 2020 at 3:16 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > Linus, Bart, it would be nice to hear from you about GPIO library > > cleanups and in general about the series... > > The series: > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > Can you send me a pull request for the contents of the v7 patch series > so I can pull it in? Sure. Will do this week (presumably today or tomorrow).
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 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@kwiius.com> Cc: Coiby Xu <coiby.xu@gmail.com> Cc: Vasile-Laurentiu Stanimir <vasile-laurentiu.stanimir@windriver.com> [1]: https://lore.kernel.org/linux-gpio/20201106181938.GA41213@black.fi.intel.com/ 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(-)