An error occurred fetching the project authors.
  1. 04 Oct, 2022 1 commit
  2. 19 Jul, 2022 1 commit
  3. 08 Apr, 2022 2 commits
    • Andy Shevchenko's avatar
      gpiolib: acpi: Convert type for pin to be unsigned · 0c2cae09
      Andy Shevchenko authored
      A pin that comes from ACPI tables is of unsigned type. This also applies
      to the internal APIs which use unsigned int to store the pin. Convert
      type for pin to be unsigned in the places where it's not yet true.
      
      While at it, add a stub for acpi_get_and_request_gpiod() for the sake
      of consistency in the APIs.
      Signed-off-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      0c2cae09
    • Linus Torvalds's avatar
      gpiolib: acpi: use correct format characters · 213d266e
      Linus Torvalds authored
      When compiling with -Wformat, clang emits the following warning:
      
        gpiolib-acpi.c:393:4: warning: format specifies type 'unsigned char' but the argument has type 'int' [-Wformat]
                              pin);
                              ^~~
      
      So warning that '%hhX' is paired with an 'int' is all just completely
      mindless and wrong. Sadly, I can see a different bogus warning reason
      why people would want to use '%02hhX'.
      
      Again, the *sane* thing from a human perspective is to use '%02X. But
      if the compiler doesn't do any range analysis at all, it could decide
      that "Oh, that print format could need up to 8 bytes of space in the
      result". Using '%02hhX' would cut that down to two.
      
      And since we use
      
              char ev_name[5];
      
      and currently use "_%c%02hhX" as the format string, even a compiler
      that doesn't notice that "pin <= 255" test that guards this all will
      go "OK, that's at most 4 bytes and the final NUL termination, so it's
      fine".
      
      While a compiler - like gcc - that only sees that the original source
      of the 'pin' value is a 'unsigned short' array, and then doesn't take
      the "pin <= 255" into account, will warn like this:
      
        gpiolib-acpi.c: In function 'acpi_gpiochip_request_interrupt':
        gpiolib-acpi.c:206:24: warning: '%02X' directive writing between 2 and 4 bytes into a region of size 3 [-Wformat-overflow=]
             sprintf(ev_name, "_%c%02X",
                                  ^~~~
        gpiolib-acpi.c:206:20: note: directive argument in the range [0, 65535]
      
      because gcc isn't being very good at that argument range analysis either.
      
      In other words, the original use of 'hhx' was bogus to begin with, and
      due to *another* compiler warning being bad, and we had that bad code
      being written back in 2016 to work around _that_ compiler warning
      (commit e40a3ae1: "gpio: acpi: work around false-positive
      -Wstring-overflow warning").
      
      Sadly, two different bad compiler warnings together does not make for
      one good one.
      
      It just makes for even more pain.
      
      End result: I think the simplest and cleanest option is simply the
      proposed change which undoes that '%hhX' change for gcc, and replaces
      it with just using a slightly bigger stack allocation. It's not like
      a 5-byte allocation is in any way likely to have saved any actual stack,
      since all the other variables in that function are 'int' or bigger.
      
      False-positive compiler warnings really do make people write worse
      code, and that's a problem. But on a scale of bad code, I feel that
      extending the buffer trivially is better than adding a pointless cast
      that literally makes no sense.
      
      At least in this case the end result isn't unreadable or buggy. We've
      had several cases of bad compiler warnings that caused changes that
      were actually horrendously wrong.
      
      Fixes: e40a3ae1 ("gpio: acpi: work around false-positive -Wstring-overflow warning")
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Signed-off-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      213d266e
  4. 07 Mar, 2022 1 commit
  5. 03 Jan, 2022 1 commit
  6. 25 Nov, 2021 2 commits
  7. 15 Nov, 2021 2 commits
  8. 20 Oct, 2021 1 commit
  9. 22 Sep, 2021 1 commit
    • Hans de Goede's avatar
      gpiolib: acpi: Make set-debounce-timeout failures non fatal · cef0d022
      Hans de Goede authored
      Commit 8dcb7a15 ("gpiolib: acpi: Take into account debounce settings")
      made the gpiolib-acpi code call gpio_set_debounce_timeout() when requesting
      GPIOs.
      
      This in itself is fine, but it also made gpio_set_debounce_timeout()
      errors fatal, causing the requesting of the GPIO to fail. This is causing
      regressions. E.g. on a HP ElitePad 1000 G2 various _AEI specified GPIO
      ACPI event sources specify a debouncy timeout of 20 ms, but the
      pinctrl-baytrail.c only supports certain fixed values, the closest
      ones being 12 or 24 ms and pinctrl-baytrail.c responds with -EINVAL
      when specified a value which is not one of the fixed values.
      
      This is causing the acpi_request_own_gpiod() call to fail for 3
      ACPI event sources on the HP ElitePad 1000 G2, which in turn is causing
      e.g. the battery charging vs discharging status to never get updated,
      even though a charger has been plugged-in or unplugged.
      
      Make gpio_set_debounce_timeout() errors non fatal, warning about the
      failure instead, to fix this regression.
      
      Note we should probably also fix various pinctrl drivers to just
      pick the first bigger discrete value rather then returning -EINVAL but
      this will need to be done on a per driver basis, where as this fix
      at least gets us back to where things were before and thus restores
      functionality on devices where this was lost due to
      gpio_set_debounce_timeout() errors.
      
      Fixes: 8dcb7a15 ("gpiolib: acpi: Take into account debounce settings")
      Depends-on: 2e2b496c ("gpiolib: acpi: Extract acpi_request_own_gpiod() helper")
      Reviewed-by: default avatarMika Westerberg <mika.westerberg@linux.intel.com>
      Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
      Acked-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      Signed-off-by: default avatarBartosz Golaszewski <brgl@bgdev.pl>
      cef0d022
  10. 07 Jun, 2021 1 commit
  11. 04 Jun, 2021 2 commits
  12. 05 May, 2021 1 commit
  13. 26 Mar, 2021 1 commit
  14. 08 Mar, 2021 3 commits
  15. 01 Dec, 2020 1 commit
  16. 16 Nov, 2020 10 commits
  17. 14 Sep, 2020 1 commit
  18. 23 Aug, 2020 1 commit
  19. 16 Apr, 2020 1 commit
  20. 24 Mar, 2020 1 commit
  21. 11 Mar, 2020 3 commits
    • Hans de Goede's avatar
      gpiolib: acpi: Add quirk to ignore EC wakeups on HP x2 10 BYT + AXP288 model · 0e91506b
      Hans de Goede authored
      Commit aa23ca3d ("gpiolib: acpi: Add honor_wakeup module-option +
      quirk mechanism") was added to deal with spurious wakeups on one specific
      model of the HP x2 10 series. In the mean time I have learned that there
      are at least 3 different HP x2 10 models:
      
      Bay Trail SoC + AXP288 PMIC
      Cherry Trail SoC + AXP288 PMIC
      Cherry Trail SoC + TI PMIC
      
      And the original quirk is only correct for (and only matches the)
      Cherry Trail SoC + TI PMIC model.
      
      The Bay Trail SoC + AXP288 PMIC model has different DMI strings, has
      the external EC interrupt on a different GPIO pin and only needs to ignore
      wakeups on the EC interrupt, the INT0002 device works fine on this model.
      
      This commit adds an extra DMI based quirk for the HP x2 10 BYT + AXP288
      model, ignoring wakeups for ACPI GPIO events on the EC interrupt pin
      on this model. This fixes spurious wakeups from suspend on this model.
      
      Fixes: aa23ca3d ("gpiolib: acpi: Add honor_wakeup module-option + quirk mechanism")
      Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
      Link: https://lore.kernel.org/r/20200302111225.6641-3-hdegoede@redhat.comAcked-by: default avatarMika Westerberg <mika.westerberg@linux.intel.com>
      Signed-off-by: default avatarLinus Walleij <linus.walleij@linaro.org>
      0e91506b
    • Hans de Goede's avatar
      gpiolib: acpi: Rework honor_wakeup option into an ignore_wake option · 2ccb21f5
      Hans de Goede authored
      Commit aa23ca3d ("gpiolib: acpi: Add honor_wakeup module-option +
      quirk mechanism") was added to deal with spurious wakeups on one specific
      model of the HP x2 10 series.
      
      The approach taken there was to add a bool controlling wakeup support for
      all ACPI GPIO events. This was sufficient for the specific HP x2 10 model
      the commit was trying to fix, but in the mean time other models have
      turned up which need a similar workaround to avoid spurious wakeups from
      suspend, but only for one of the pins on which the ACPI tables request
      ACPI GPIO events.
      
      Since the honor_wakeup option was added to be able to ignore wake events,
      the name was perhaps not the best, this commit renames it to ignore_wake
      and changes it to a string with the following format:
      gpiolib_acpi.ignore_wake=controller@pin[,controller@pin[,...]]
      
      This allows working around spurious wakeup issues on a per pin basis.
      
      This commit also reworks the existing quirk for the HP x2 10 so that
      it functions as before.
      
      Note:
      -This removes the honor_wakeup parameter. This has only been upstream for
       a short time and to the best of my knowledge there are no users using
       this module parameter.
      
      -The controller@pin[,controller@pin[,...]] syntax is based on an existing
       kernel module parameter using the same controller@pin format. That version
       uses ';' as separator, but in practice that is problematic because grub2
       cannot handle this without taking special care to escape the ';', so here
       we are using a ',' as separator instead which does not have this issue.
      
      Fixes: aa23ca3d ("gpiolib: acpi: Add honor_wakeup module-option + quirk mechanism")
      Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
      Link: https://lore.kernel.org/r/20200302111225.6641-2-hdegoede@redhat.comAcked-by: default avatarMika Westerberg <mika.westerberg@linux.intel.com>
      Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      Signed-off-by: default avatarLinus Walleij <linus.walleij@linaro.org>
      2ccb21f5
    • Hans de Goede's avatar
      gpiolib: acpi: Correct comment for HP x2 10 honor_wakeup quirk · efaa87fa
      Hans de Goede authored
      Commit aa23ca3d ("gpiolib: acpi: Add honor_wakeup module-option +
      quirk mechanism") added a quirk for some models of the HP x2 10 series.
      
      There are 2 issues with the comment describing the quirk:
      1) The comment claims the DMI quirk applies to all Cherry Trail based HP x2
         10 models. In the mean time I have learned that there are at least 3
         models of the HP x2 10 models:
      
         Bay Trail SoC + AXP288 PMIC
         Cherry Trail SoC + AXP288 PMIC
         Cherry Trail SoC + TI PMIC
      
         And this quirk's DMI matches only match the Cherry Trail SoC + TI PMIC
         SoC, which is good because we want a slightly different quirk for the
         others. This commit updates the comment to make it clear that the quirk
         is only for the Cherry Trail SoC + TI PMIC models.
      
      2) The comment says that it is ok to disable wakeup on all ACPI GPIO event
         handlers, because there is only the one for the embedded-controller
         events. This is not true, there also is a handler for the special
         INT0002 device which is related to USB wakeups. We need to also disable
         wakeups on that one because the device turns of the USB-keyboard built
         into the dock when closing the lid. The XHCI controller takes a while
         to notice this, so it only notices it when already suspended, causing
         a spurious wakeup because of this. So disabling wakeup on all handlers
         is the right thing to do, but not because there only is the one handler
         for the EC events. This commit updates the comment to correctly reflect
         this.
      
      Fixes: aa23ca3d ("gpiolib: acpi: Add honor_wakeup module-option + quirk mechanism")
      Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
      Link: https://lore.kernel.org/r/20200302111225.6641-1-hdegoede@redhat.comAcked-by: default avatarMika Westerberg <mika.westerberg@linux.intel.com>
      Signed-off-by: default avatarLinus Walleij <linus.walleij@linaro.org>
      efaa87fa
  22. 07 Jan, 2020 2 commits