1. 06 Apr, 2020 1 commit
  2. 04 Apr, 2020 2 commits
    • Hans de Goede's avatar
      ACPI: PM: Add acpi_[un]register_wakeup_handler() · ddfd9dcf
      Hans de Goede authored
      Since commit fdde0ff8 ("ACPI: PM: s2idle: Prevent spurious SCIs from
      waking up the system") the SCI triggering without there being a wakeup
      cause recognized by the ACPI sleep code will no longer wakeup the system.
      
      This works as intended, but this is a problem for devices where the SCI
      is shared with another device which is also a wakeup source.
      
      In the past these, from the pov of the ACPI sleep code, spurious SCIs
      would still cause a wakeup so the wakeup from the device sharing the
      interrupt would actually wakeup the system. This now no longer works.
      
      This is a problem on e.g. Bay Trail-T and Cherry Trail devices where
      some peripherals (typically the XHCI controller) can signal a
      Power Management Event (PME) to the Power Management Controller (PMC)
      to wakeup the system, this uses the same interrupt as the SCI.
      These wakeups are handled through a special INT0002 ACPI device which
      checks for events in the GPE0a_STS for this and takes care of acking
      the PME so that the shared interrupt stops triggering.
      
      The change to the ACPI sleep code to ignore the spurious SCI, causes
      the system to no longer wakeup on these PME events. To make things
      worse this means that the INT0002 device driver interrupt handler will
      no longer run, causing the PME to not get cleared and resulting in the
      system hanging. Trying to wakeup the system after such a PME through e.g.
      the power button no longer works.
      
      Add an acpi_register_wakeup_handler() function which registers
      a handler to be called from acpi_s2idle_wake() and when the handler
      returns true, return true from acpi_s2idle_wake().
      
      The INT0002 driver will use this mechanism to check the GPE0a_STS
      register from acpi_s2idle_wake() and to tell the system to wakeup
      if a PME is signaled in the register.
      
      Fixes: fdde0ff8 ("ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system")
      Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
      Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      ddfd9dcf
    • Qian Cai's avatar
      x86: ACPI: fix CPU hotplug deadlock · 696ac2e3
      Qian Cai authored
      Similar to commit 0266d81e ("acpi/processor: Prevent cpu hotplug
      deadlock") except this is for acpi_processor_ffh_cstate_probe():
      
      "The problem is that the work is scheduled on the current CPU from the
      hotplug thread associated with that CPU.
      
      It's not required to invoke these functions via the workqueue because
      the hotplug thread runs on the target CPU already.
      
      Check whether current is a per cpu thread pinned on the target CPU and
      invoke the function directly to avoid the workqueue."
      
       WARNING: possible circular locking dependency detected
       ------------------------------------------------------
       cpuhp/1/15 is trying to acquire lock:
       ffffc90003447a28 ((work_completion)(&wfc.work)){+.+.}-{0:0}, at: __flush_work+0x4c6/0x630
      
       but task is already holding lock:
       ffffffffafa1c0e8 (cpuidle_lock){+.+.}-{3:3}, at: cpuidle_pause_and_lock+0x17/0x20
      
       which lock already depends on the new lock.
      
       the existing dependency chain (in reverse order) is:
      
       -> #1 (cpu_hotplug_lock){++++}-{0:0}:
       cpus_read_lock+0x3e/0xc0
       irq_calc_affinity_vectors+0x5f/0x91
       __pci_enable_msix_range+0x10f/0x9a0
       pci_alloc_irq_vectors_affinity+0x13e/0x1f0
       pci_alloc_irq_vectors_affinity at drivers/pci/msi.c:1208
       pqi_ctrl_init+0x72f/0x1618 [smartpqi]
       pqi_pci_probe.cold.63+0x882/0x892 [smartpqi]
       local_pci_probe+0x7a/0xc0
       work_for_cpu_fn+0x2e/0x50
       process_one_work+0x57e/0xb90
       worker_thread+0x363/0x5b0
       kthread+0x1f4/0x220
       ret_from_fork+0x27/0x50
      
       -> #0 ((work_completion)(&wfc.work)){+.+.}-{0:0}:
       __lock_acquire+0x2244/0x32a0
       lock_acquire+0x1a2/0x680
       __flush_work+0x4e6/0x630
       work_on_cpu+0x114/0x160
       acpi_processor_ffh_cstate_probe+0x129/0x250
       acpi_processor_evaluate_cst+0x4c8/0x580
       acpi_processor_get_power_info+0x86/0x740
       acpi_processor_hotplug+0xc3/0x140
       acpi_soft_cpu_online+0x102/0x1d0
       cpuhp_invoke_callback+0x197/0x1120
       cpuhp_thread_fun+0x252/0x2f0
       smpboot_thread_fn+0x255/0x440
       kthread+0x1f4/0x220
       ret_from_fork+0x27/0x50
      
       other info that might help us debug this:
      
       Chain exists of:
       (work_completion)(&wfc.work) --> cpuhp_state-up --> cpuidle_lock
      
       Possible unsafe locking scenario:
      
       CPU0                    CPU1
       ----                    ----
       lock(cpuidle_lock);
                               lock(cpuhp_state-up);
                               lock(cpuidle_lock);
       lock((work_completion)(&wfc.work));
      
       *** DEADLOCK ***
      
       3 locks held by cpuhp/1/15:
       #0: ffffffffaf51ab10 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x69/0x2f0
       #1: ffffffffaf51ad40 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x69/0x2f0
       #2: ffffffffafa1c0e8 (cpuidle_lock){+.+.}-{3:3}, at: cpuidle_pause_and_lock+0x17/0x20
      
       Call Trace:
       dump_stack+0xa0/0xea
       print_circular_bug.cold.52+0x147/0x14c
       check_noncircular+0x295/0x2d0
       __lock_acquire+0x2244/0x32a0
       lock_acquire+0x1a2/0x680
       __flush_work+0x4e6/0x630
       work_on_cpu+0x114/0x160
       acpi_processor_ffh_cstate_probe+0x129/0x250
       acpi_processor_evaluate_cst+0x4c8/0x580
       acpi_processor_get_power_info+0x86/0x740
       acpi_processor_hotplug+0xc3/0x140
       acpi_soft_cpu_online+0x102/0x1d0
       cpuhp_invoke_callback+0x197/0x1120
       cpuhp_thread_fun+0x252/0x2f0
       smpboot_thread_fn+0x255/0x440
       kthread+0x1f4/0x220
       ret_from_fork+0x27/0x50
      Signed-off-by: default avatarQian Cai <cai@lca.pw>
      Tested-by: default avatarBorislav Petkov <bp@suse.de>
      [ rjw: Subject ]
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      696ac2e3
  3. 01 Apr, 2020 2 commits
  4. 30 Mar, 2020 5 commits
  5. 28 Mar, 2020 2 commits
  6. 27 Mar, 2020 1 commit
  7. 26 Mar, 2020 1 commit
  8. 25 Mar, 2020 2 commits
  9. 24 Mar, 2020 1 commit
  10. 22 Mar, 2020 2 commits
  11. 17 Mar, 2020 1 commit
  12. 14 Mar, 2020 6 commits
    • Takashi Iwai's avatar
      ACPI: PCI: Use scnprintf() for avoiding potential buffer overflow · edd66086
      Takashi Iwai authored
      Since snprintf() returns the would-be-output size instead of the
      actual output size, the succeeding calls may go beyond the given
      buffer limit.  Fix it by replacing with scnprintf().
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      edd66086
    • Takashi Iwai's avatar
      ACPI: fan: Use scnprintf() for avoiding potential buffer overflow · 949fe25f
      Takashi Iwai authored
      Since snprintf() returns the would-be-output size instead of the
      actual output size, the succeeding calls may go beyond the given
      buffer limit.  Fix it by replacing with scnprintf().
      
      Also adjust the argument to really match with the actually remaining
      buffer size.
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      949fe25f
    • Rafael J. Wysocki's avatar
      ACPI: EC: Eliminate EC_FLAGS_QUERY_HANDSHAKE · b1e14999
      Rafael J. Wysocki authored
      The EC_FLAGS_QUERY_HANDSHAKE switch is never set in the current
      code (the only function setting it is defined under #if 0) and
      has no effect whatever, so eliminate it and drop the code
      depending on it.
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      b1e14999
    • Rafael J. Wysocki's avatar
      ACPI: EC: Do not clear boot_ec_is_ecdt in acpi_ec_add() · 65a691f5
      Rafael J. Wysocki authored
      The reason for clearing boot_ec_is_ecdt in acpi_ec_add() (if a
      PNP0C09 device object matching the ECDT boot EC had been found in
      the namespace) was to cause acpi_ec_ecdt_start() to return early,
      but since the latter does not look at boot_ec_is_ecdt any more,
      acpi_ec_add() need not clear it.
      
      Moreover, doing that may be confusing as it may cause "DSDT" to be
      printed instead of "ECDT" in the EC initialization completion
      message, so stop doing it.
      
      While at it, split the EC initialization completion message into
      two messages, one regarding the boot EC and another one printed
      regardless of whether or not the EC at hand is the boot one.
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      65a691f5
    • Rafael J. Wysocki's avatar
      ACPI: EC: Simplify acpi_ec_ecdt_start() and acpi_ec_init() · 98ada3c5
      Rafael J. Wysocki authored
      Notice that the return value of acpi_ec_init() is discarded anyway,
      so it can be void and it doesn't need to check the return values of
      acpi_bus_register_driver() and acpi_ec_ecdt_start() called by it.
      Thus the latter can be void too and it really has nothing to do
      if acpi_ec_add() has already found an EC matching the boot one in the
      namespace.  Also, acpi_ec_ecdt_get_handle() can be folded into it.
      
      Modify the code accordingly and while at it create a propoer kerneldoc
      comment to document acpi_ec_ecdt_start() and move the remark regarding
      ASUS X550ZE along with the related bug URL from acpi_ec_init() into
      that comment.
      
      Additionally, fix up a stale comment in acpi_ec_init().
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      98ada3c5
    • Rafael J. Wysocki's avatar
      ACPI: EC: Consolidate event handler installation code · 03e9a0e0
      Rafael J. Wysocki authored
      Commit 406857f7 ("ACPI: EC: add support for hardware-reduced
      systems") made ec_install_handlers() return an error on failures
      to configure a GPIO IRQ for the EC, but that is inconsistent with
      the handling of the GPE event handler installation failures even
      though it is exactly the same issue and the driver can respond to
      it in the same way in both cases (the EC can be actively polled
      for events through its registers if the event handler installation
      fails).
      
      Moreover, it requires acpi_ec_add() to take that special case into
      account and disagrees with the ec_install_handlers() header comment.
      
      For this reason, rework the event handler installation code in
      ec_install_handlers() to explicitly take deferred probing (that
      may be needed in the GPIO IRQ case) into account and to avoid
      failing the EC initialization in any other case.
      
      Among other things, reduce code duplication between
      install_gpe_event_handler() and install_gpio_irq_event_handler() by
      moving some code from there into ec_install_handlers() itself and
      simplify the error code path in acpi_ec_add().
      
      While at it, turn the ec_install_handlers() header comment into
      a proper kerneldoc one and add some general control flow information
      to it.
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      Tested-by: default avatarJian-Hong Pan <jian-hong@endlessm.com>
      03e9a0e0
  13. 04 Mar, 2020 5 commits
  14. 02 Mar, 2020 2 commits
    • Rafael J. Wysocki's avatar
      ACPI: EC: Use fast path in acpi_ec_add() for DSDT boot EC · 3d9b8dd8
      Rafael J. Wysocki authored
      If the boot EC comes from the DSDT, its ACPI handle is equal to the
      handle of a device object with the PNP0C09 device ID.  If that
      device object is passed to acpi_ec_add(), it is not necessary to
      allocate a new EC structure for it and parse it, because that has
      been done already, so change the function to use the fast path in
      that case.
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      3d9b8dd8
    • Rafael J. Wysocki's avatar
      ACPI: EC: Simplify acpi_ec_add() · e3cfabcd
      Rafael J. Wysocki authored
      First, notice that if the device ID in acpi_ec_add() is equal to
      ACPI_ECDT_HID, boot_ec_is_ecdt must be set, because this means
      that the device object passed to acpi_ec_add() comes from
      acpi_ec_ecdt_start() which fails if boot_ec_is_ecdt is unset.
      Accordingly, boot_ec_is_ecdt need not be set again in that case,
      so drop that redundant update of it from the code.
      
      Next, ec->handle must be a valid ACPI handle right before
      returning 0 from acpi_ec_add(), because it either is the handle
      of the device object passed to that function, or it is the boot EC
      handle coming from acpi_ec_ecdt_start() which fails if it cannot
      find a valid handle for the boot EC.  Moreover, the object with
      that handle is regarded as a valid representation of the EC in all
      cases, so there is no reason to avoid the _DEP list update walk if
      that handle is the boot EC handle.  Accordingly, drop the dep_update
      local variable from acpi_ec_add() and call acpi_walk_dep_device_list()
      for ec->handle unconditionally before returning 0 from it.
      
      Finally, the ec local variable in acpi_ec_add() need not be
      initialized to NULL and the status local variable declaration
      can be moved to the block in which it is used, so change the code
      in accordance with these observations.
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      e3cfabcd
  15. 29 Feb, 2020 1 commit
  16. 27 Feb, 2020 3 commits
    • Rafael J. Wysocki's avatar
      ACPI: EC: Drop AE_NOT_FOUND special case from ec_install_handlers() · 7247f0c2
      Rafael J. Wysocki authored
      If the status value returned by acpi_install_address_space_handler()
      in ec_install_handlers() is AE_NOT_FOUND, it is treated in a special
      way, apparently because it might mean a _REG method evaluation
      failure (at least that is the case according to the comment in
      there), but acpi_install_address_space_handler() does not take
      _REG evaluation errors into account at all, so the AE_NOT_FOUND
      special handling is confusing at best.
      
      For this reason, change ec_install_handlers() to stop the EC and
      return -ENODEV on all acpi_install_address_space_handler() errors.
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      7247f0c2
    • Rafael J. Wysocki's avatar
      ACPI: EC: Avoid passing redundant argument to functions · a2b69177
      Rafael J. Wysocki authored
      After commit 406857f7 ("ACPI: EC: add support for hardware-reduced
      systems") the handle_events argument passed to ec_install_handlers()
      and acpi_ec_setup() is redundant, because it is always 'false' when
      the device argument passed to them in NULL and it is always 'true'
      otherwise, so the device argument can be tested against NULL instead
      of testing the handle_events one.
      
      Accordingly, modify ec_install_handlers() and acpi_ec_setup() to take
      two arguments and reduce the number of checks in the former.
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      a2b69177
    • Rafael J. Wysocki's avatar
      ACPI: EC: Avoid printing confusing messages in acpi_ec_setup() · c823c17a
      Rafael J. Wysocki authored
      It doesn't really make sense to pass ec->handle of the ECDT EC to
      acpi_handle_info(), because it is set to ACPI_ROOT_OBJECT in
      acpi_ec_ecdt_probe(), so rework acpi_ec_setup() to avoid using
      acpi_handle_info() for printing messages.
      
      First, notice that the "Used as first EC" message is not really
      useful, because it is immediately followed by a more meaningful one
      from either acpi_ec_ecdt_probe() or acpi_ec_dsdt_probe() (the latter
      also includes the EC object path), so drop it altogether.
      
      Second, use pr_info() for printing the EC configuration information.
      
      While at it, make the code in question avoid printing invalid GPE or
      IRQ numbers and make it print the GPE/IRQ information only when the
      driver is ready to handle events.
      
      Fixes: 72c77b7e ("ACPI / EC: Cleanup first_ec/boot_ec code")
      Fixes: 406857f7 ("ACPI: EC: add support for hardware-reduced systems")
      Cc: 5.5+ <stable@vger.kernel.org> # 5.5+
      Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      c823c17a
  17. 21 Feb, 2020 1 commit
  18. 17 Feb, 2020 1 commit
  19. 16 Feb, 2020 1 commit