[Deepin-Kernel-SIG] [linux 6.6.y] [Upstream] Sync bugfix for intel mei from upstream#1492
[Deepin-Kernel-SIG] [linux 6.6.y] [Upstream] Sync bugfix for intel mei from upstream#1492opsiff wants to merge 9 commits intodeepin-community:linux-6.6.yfrom
Conversation
mainline inclusion from mainline-v6.8-rc1 category: bugfix CONFIG_INTEL_MEI_VSC_HW can be set to built-in even with CONFIG_MEI=m, but then the driver is not built because Kbuild never enters the drivers/misc/mei directory for built-in files, leading to a link failure: ERROR: modpost: "vsc_tp_reset" [drivers/misc/mei/mei-vsc.ko] undefined! ERROR: modpost: "vsc_tp_init" [drivers/misc/mei/mei-vsc.ko] undefined! ERROR: modpost: "vsc_tp_xfer" [drivers/misc/mei/mei-vsc.ko] undefined! ERROR: modpost: "vsc_tp_need_read" [drivers/misc/mei/mei-vsc.ko] undefined! ERROR: modpost: "vsc_tp_intr_enable" [drivers/misc/mei/mei-vsc.ko] undefined! ERROR: modpost: "vsc_tp_intr_synchronize" [drivers/misc/mei/mei-vsc.ko] undefined! ERROR: modpost: "vsc_tp_intr_disable" [drivers/misc/mei/mei-vsc.ko] undefined! ERROR: modpost: "vsc_tp_register_event_cb" [drivers/misc/mei/mei-vsc.ko] undefined! Add an explicit dependency on CONFIG_MEI that was apparently missing, to ensure the VSC_HW driver cannot be built-in with MEI itself being a loadable module. Fixes: 566f5ca ("mei: Add transport driver for IVSC device") Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> Reviewed-by: Wentong Wu <wentong.wu@intel.com> Link: https://lore.kernel.org/r/20231214183946.109124-1-arnd@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit 95171e4) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
mainline inclusion from mainline-v6.14-rc6 category: bugfix The _CRS ACPI resources table has 2 entries for the host wakeup GPIO, the first one being a regular GpioIo () resource while the second one is a GpioInt () resource for the same pin. The acpi_gpio_mapping table used by vsc-tp.c maps the first Gpio () resource to "wakeuphost-gpios" where as the second GpioInt () entry is mapped to "wakeuphostint-gpios". Using "wakeuphost" to request the GPIO as was done until now, means that the gpiolib-acpi code does not know that the GPIO is active-low as that info is only available in the GpioInt () entry. Things were still working before due to the following happening: 1. Since the 2 entries point to the same pin they share a struct gpio_desc 2. The SPI core creates the SPI device vsc-tp.c binds to and calls acpi_dev_gpio_irq_get(). This does use the second entry and sets FLAG_ACTIVE_LOW in gpio_desc.flags . 3. vsc_tp_probe() requests the "wakeuphost" GPIO and inherits the active-low flag set by acpi_dev_gpio_irq_get() But there is a possible scenario where things do not work: 1. - 3. happen as above 4. After requesting the "wakeuphost" GPIO, the "resetfw" GPIO is requested next, but its USB GPIO controller is not available yet, so this call returns -EPROBE_DEFER. 5. The gpio_desc for "wakeuphost" is put() and during this the active-low flag is cleared from gpio_desc.flags . 6. Later on vsc_tp_probe() requests the "wakeuphost" GPIO again, but now it is not marked active-low. The difference can also be seen in /sys/kernel/debug/gpio, which contains the following line for this GPIO: gpio-535 ( |wakeuphost ) in hi IRQ ACTIVE LOW If the second scenario is hit the "ACTIVE LOW" at the end disappears and things do not work. Fix this by requesting the GPIO through the "wakeuphostint" mapping instead which provides active-low info without relying on acpi_dev_gpio_irq_get() pre-populating this info in the gpio_desc. Link: https://bugzilla.redhat.com/show_bug.cgi?id=2316918 Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> Tested-by: Sakari Ailus <sakari.ailus@linux.intel.com> Fixes: 566f5ca ("mei: Add transport driver for IVSC device") Cc: stable <stable@kernel.org> Link: https://lore.kernel.org/r/20250214212425.84021-1-hdegoede@redhat.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit fdb1ada) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
mainline inclusion from mainline-v6.13-rc1 category: bugfix The only 2 callers of vsc_tp_reset() are: 1. mei_vsc_hw_reset(), which immediataly calls vsc_tp_intr_disable() afterwards. 2. vsc_tp_shutdown() which immediately calls free_irq() afterwards. So neither actually wants the interrupt to be enabled after resetting the chip and having the interrupt enabled for a short time afer the reset is undesirable. Drop the enable_irq() call from vsc_tp_reset(), so that the interrupt is left disabled after vsc_tp_reset(). Link: intel/ivsc-driver#51 Signed-off-by: Hans de Goede <hdegoede@redhat.com> Link: https://lore.kernel.org/r/20241106220102.40549-1-hdegoede@redhat.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit 49988a7) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
mainline inclusion from mainline-v6.17-rc1 category: bugfix mei_vsc_hw_reset() gets called from mei_start() and mei_stop() in the latter case we do not need to re-init the VSC by calling vsc_tp_init(). mei_stop() only happens on shutdown and driver unbind. On shutdown we don't need to load + boot the firmware and if the driver later is bound to the device again then mei_start() will do another reset. The intr_enable flag is true when called from mei_start() and false on mei_stop(). Skip vsc_tp_init() when intr_enable is false. This avoids unnecessarily uploading the firmware, which takes 11 seconds. This change reduces the poweroff/reboot time by 11 seconds. Fixes: 386a766 ("mei: Add MEI hardware support for IVSC device") Signed-off-by: Hans de Goede <hansg@kernel.org> Reviewed-by: Alexander Usyskin <alexander.usyskin@intel.com> Link: https://lore.kernel.org/r/20250623085052.12347-3-hansg@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit 880af85) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
mainline inclusion from mainline-v6.17-rc1 category: bugfix The event_notify callback which runs from vsc_tp_thread_isr may call vsc_tp_xfer() which locks the mutex. So the ISR depends on the mutex. Move the mutex_destroy() call to after free_irq() to ensure that the ISR is not running while the mutex is destroyed. Fixes: 566f5ca ("mei: Add transport driver for IVSC device") Signed-off-by: Hans de Goede <hansg@kernel.org> Link: https://lore.kernel.org/r/20250623085052.12347-6-hansg@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit 35b7f35) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
mainline inclusion from mainline-v6.17-rc1 category: bugfix vsc_tp_register_event_cb() can race with vsc_tp_thread_isr(), add a mutex to protect against this. Fixes: 566f5ca ("mei: Add transport driver for IVSC device") Signed-off-by: Hans de Goede <hansg@kernel.org> Link: https://lore.kernel.org/r/20250623085052.12347-7-hansg@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit 18f14b2) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
mainline inclusion from mainline-v6.17-rc1 category: bugfix Make mei_vsc_remove() properly unset the callback to avoid a dead callback sticking around after probe errors or unbinding of the platform driver. Fixes: 386a766 ("mei: Add MEI hardware support for IVSC device") Signed-off-by: Hans de Goede <hansg@kernel.org> Link: https://lore.kernel.org/r/20250623085052.12347-8-hansg@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit 6175c69) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
mainline inclusion from mainline-v6.17-rc1 category: bugfix The event_notify callback in some cases calls vsc_tp_xfer(), which checks tp->assert_cnt and waits for it through the tp->xfer_wait wait-queue. And tp->assert_cnt is increased and the tp->xfer_wait queue is woken o from the interrupt handler. So the interrupt handler which is running the event callback is waiting for itself to signal that it can continue. This happens to work because the event callback runs from the threaded ISR handler and while that is running the hard ISR handler will still get called a second / third time for further interrupts and it is the hard ISR handler which does the atomic_inc() and wake_up() calls. But having the threaded ISR handler wait for its own interrupt to trigger again is not how a threaded ISR handler is supposed to be used. Move the running of the event callback from a threaded interrupt handler to a workqueue since a threaded ISR should not wait for events from its own interrupt. This is a preparation patch for moving the atomic_inc() and wake_up() calls to the threaded ISR handler, which is necessary to fix a locking issue. Fixes: 566f5ca ("mei: Add transport driver for IVSC device") Signed-off-by: Hans de Goede <hansg@kernel.org> Link: https://lore.kernel.org/r/20250623085052.12347-9-hansg@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit de88b02) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
mainline inclusion
from mainline-v6.17-rc1
category: bugfix
Kernels build with CONFIG_PROVE_RAW_LOCK_NESTING report the following
tp-vsc lockdep error:
=============================
[ BUG: Invalid wait context ]
...
swapper/10/0 is trying to lock:
ffff88819c271888 (&tp->xfer_wait){....}-{3:3},
at: __wake_up (kernel/sched/wait.c:106 kernel/sched/wait.c:127)
...
Call Trace:
<IRQ>
...
__raw_spin_lock_irqsave (./include/linux/spinlock_api_smp.h:111)
__wake_up (kernel/sched/wait.c:106 kernel/sched/wait.c:127)
vsc_tp_isr (drivers/misc/mei/vsc-tp.c:110) mei_vsc_hw
__handle_irq_event_percpu (kernel/irq/handle.c:158)
handle_irq_event (kernel/irq/handle.c:195 kernel/irq/handle.c:210)
handle_edge_irq (kernel/irq/chip.c:833)
...
</IRQ>
The root-cause of this is the IRQF_NO_THREAD flag used by the intel-pinctrl
code. Setting IRQF_NO_THREAD requires all interrupt handlers for GPIO ISRs
to use raw-spinlocks only since normal spinlocks can sleep in PREEMPT-RT
kernels and with IRQF_NO_THREAD the interrupt handlers will always run in
an atomic context [1].
vsc_tp_isr() calls wake_up(&tp->xfer_wait), which uses a regular spinlock,
breaking the raw-spinlocks only rule for Intel GPIO ISRs.
Make vsc_tp_isr() run as threaded ISR instead of as hard ISR to fix this.
Fixes: 566f5ca ("mei: Add transport driver for IVSC device")
Link: https://lore.kernel.org/linux-gpio/18ab52bd-9171-4667-a600-0f52ab7017ac@kernel.org/ [1]
Signed-off-by: Hans de Goede <hansg@kernel.org>
Link: https://lore.kernel.org/r/20250623085052.12347-10-hansg@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit cee3dba)
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Reviewer's GuideSyncs multiple upstream fixes for the Intel MEI VSC transport: adjusts IRQ handling and event notifications to use a workqueue with proper locking and teardown, fixes GPIO naming and reset/interrupt behavior, and ensures event callbacks are cleared on probe errors and device removal to avoid races and lockdep issues. Sequence diagram for IRQ-driven VSC event notification via workqueuesequenceDiagram
participant SPI as SpiController
participant IRQ as IRQSubsystem
participant TP as VscTpDriver
participant WQ as SystemWorkqueue
participant MEI as MeiVscDriver
SPI->>IRQ: Assert wakeuphostint (falling edge)
IRQ->>TP: vsc_tp_isr(irq, tp)
activate TP
TP->>TP: wake_up(xfer_wait)
TP->>WQ: schedule_work(event_work)
deactivate TP
WQ->>TP: vsc_tp_event_work(event_work)
activate TP
TP->>TP: lock(event_notify_mutex)
alt event_notify is set
TP->>MEI: event_notify(event_notify_context)
else event_notify is NULL
TP->>TP: skip callback
end
TP->>TP: unlock(event_notify_mutex)
deactivate TP
Updated class diagram for VSC transport and MEI VSC driver structuresclassDiagram
class vsc_tp {
atomic_t assert_cnt
wait_queue_head_t xfer_wait
work_struct event_work
vsc_tp_event_cb_t event_notify
void* event_notify_context
mutex event_notify_mutex
mutex mutex
void vsc_tp_reset()
int vsc_tp_register_event_cb(vsc_tp* tp, vsc_tp_event_cb_t event_cb, void* context)
}
class VscTpDriver {
irqreturn_t vsc_tp_isr(int irq, void* data)
void vsc_tp_event_work(work_struct* work)
int vsc_tp_probe(spi_device* spi)
void vsc_tp_remove(spi_device* spi)
}
class mei_vsc_hw {
vsc_tp* tp
}
class mei_device {
void* dev
}
class MeiVscDriver {
int mei_vsc_hw_reset(mei_device* mei_dev, bool intr_enable)
int mei_vsc_probe(platform_device* pdev)
void mei_vsc_remove(platform_device* pdev)
}
vsc_tp <|-- VscTpDriver
mei_vsc_hw o-- vsc_tp
MeiVscDriver o-- mei_vsc_hw
MeiVscDriver o-- mei_device
Flow diagram for MEI VSC hardware reset with conditional interrupt handlingflowchart TD
A["mei_vsc_hw_reset(mei_dev, intr_enable)"] --> B["Call vsc_tp_reset(hw->tp)"]
B --> C{"intr_enable?"}
C -->|No| D["Return 0"]
C -->|Yes| E["Call vsc_tp_init(hw->tp, mei_dev->dev)"]
E --> F["Return result"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Syncs upstream fixes for the Intel MEI VSC transport/path in the 6.6.y kernel, focusing on safer event notification, IRQ/reset handling, and Kconfig dependency correctness.
Changes:
- Move VSC event notification off the IRQ thread into a workqueue; add locking around the event callback registration/invocation.
- Adjust reset/interrupt behavior and GPIO line name usage for host-wakeup signaling.
- Add Kconfig dependency to ensure the VSC HW transport is built with the base MEI stack.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| drivers/misc/mei/vsc-tp.c | Reworks IRQ/event notification flow (workqueue), updates GPIO lookup, and adjusts teardown ordering. |
| drivers/misc/mei/platform-vsc.c | Tweaks hw_reset behavior and unsets event callback on probe-error/remove. |
| drivers/misc/mei/Kconfig | Adds missing dependency between VSC HW transport and INTEL_MEI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| irq_set_status_flags(spi->irq, IRQ_DISABLE_UNLAZY); | ||
| ret = request_threaded_irq(spi->irq, vsc_tp_isr, vsc_tp_thread_isr, | ||
| ret = request_threaded_irq(spi->irq, NULL, vsc_tp_isr, | ||
| IRQF_TRIGGER_FALLING | IRQF_ONESHOT, | ||
| dev_name(dev), tp); |
There was a problem hiding this comment.
request_threaded_irq() is switched to run vsc_tp_isr() as the threaded handler, but vsc_tp_request_irq() (used by suspend/resume) still references the removed vsc_tp_thread_isr, which will cause a build failure. Please update vsc_tp_request_irq() to match the new IRQ registration pattern (primary handler NULL, threaded handler vsc_tp_isr()) or reintroduce the missing handler as appropriate.
| mutex_init(&tp->mutex); | ||
| mutex_init(&tp->event_notify_mutex); | ||
| INIT_WORK(&tp->event_work, vsc_tp_event_work); |
There was a problem hiding this comment.
In vsc_tp_probe(), the IRQ is requested before event_notify_mutex is initialized and before INIT_WORK(&tp->event_work, ...). Since the IRQ handler schedules tp->event_work, an interrupt firing immediately after request_threaded_irq() can queue/execute an uninitialized work item and use an uninitialized mutex. Initialize the mutex and work_struct before requesting the IRQ, or request the IRQ with IRQF_NO_AUTOEN and explicitly enable it after initialization.
| @@ -364,8 +367,6 @@ void vsc_tp_reset(struct vsc_tp *tp) | |||
| gpiod_set_value_cansleep(tp->wakeupfw, 1); | |||
|
|
|||
| atomic_set(&tp->assert_cnt, 0); | |||
There was a problem hiding this comment.
vsc_tp_reset() no longer re-enables the IRQ after the reset sequence (the enable_irq() call was dropped). Since the function still disables the IRQ earlier, this can leak IRQ disable-depth across reset/stop/resume flows and potentially leave the IRQ permanently disabled. Please ensure the IRQ enable/disable balance is preserved (e.g., manage IRQ state only via the MEI intr_enable/intr_disable ops, or otherwise restore the prior IRQ state after reset).
| atomic_set(&tp->assert_cnt, 0); | |
| atomic_set(&tp->assert_cnt, 0); | |
| enable_irq(tp->spi->irq); |
| mei_stop(mei_dev); | ||
|
|
||
| vsc_tp_register_event_cb(hw->tp, NULL, NULL); | ||
|
|
There was a problem hiding this comment.
Since VSC events are now delivered via vsc_tp's workqueue, mei_stop()/mei_reset() no longer implicitly synchronize with the event callback (it used to run in the IRQ thread). In mei_vsc_remove(), the callback is unset only after mei_stop(), leaving a window where a previously queued VSC event-work item can still run mei_vsc_event_cb() while teardown is in progress. Consider unsetting the callback before calling mei_stop() (and similarly in other teardown paths) so that any queued work will observe a NULL callback and exit quickly.
Arnd Bergmann (1):
mei: fix vsc dependency
Hans de Goede (8):
mei: vsc: Use "wakeuphostint" when getting the host wakeup GPIO
mei: vsc: Do not re-enable interrupt from vsc_tp_reset()
mei: vsc: Don't re-init VSC from mei_vsc_hw_reset() on stop
mei: vsc: Destroy mutex after freeing the IRQ
mei: vsc: Event notifier fixes
mei: vsc: Unset the event callback on remove and probe errors
mei: vsc: Run event callback from a workqueue
mei: vsc: Fix "BUG: Invalid wait context" lockdep error
drivers/misc/mei/Kconfig | 1 +
drivers/misc/mei/platform-vsc.c | 8 ++++++-
drivers/misc/mei/vsc-tp.c | 41 ++++++++++++++++++++-------------
3 files changed, 33 insertions(+), 17 deletions(-)
Summary by Sourcery
Synchronize upstream Intel MEI VSC bug fixes, improving interrupt handling, event notification, and reset behavior for the VSC transport, and aligning GPIO usage with the upstream driver.
Bug Fixes: