* [PATCH v2 1/2] Revert "mei: vsc: Call wake_up() in the threaded IRQ handler" @ 2024-04-03 5:13 Wentong Wu 2024-04-03 5:13 ` [PATCH v2 2/2] mei: vsc: Unregister interrupt handler for system suspend Wentong Wu 2024-04-03 12:00 ` [PATCH v2 1/2] Revert "mei: vsc: Call wake_up() in the threaded IRQ handler" Winkler, Tomas 0 siblings, 2 replies; 4+ messages in thread From: Wentong Wu @ 2024-04-03 5:13 UTC (permalink / raw) To: sakari.ailus, tomas.winkler, gregkh; +Cc: linux-kernel, hao.yao, stable From: Sakari Ailus <sakari.ailus@linux.intel.com> This reverts commit 058a38acba15fd8e7b262ec6e17c4204cb15f984. It's not necessary to avoid a spinlock, a sleeping lock on PREEMPT_RT, in an interrupt handler as the interrupt handler itself would be called in a process context if PREEMPT_RT is enabled. So revert the patch. Cc: stable@vger.kernel.org # for 6.8 Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/misc/mei/vsc-tp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/misc/mei/vsc-tp.c b/drivers/misc/mei/vsc-tp.c index ecfb70cd057c..968a92a7425d 100644 --- a/drivers/misc/mei/vsc-tp.c +++ b/drivers/misc/mei/vsc-tp.c @@ -419,6 +419,8 @@ static irqreturn_t vsc_tp_isr(int irq, void *data) atomic_inc(&tp->assert_cnt); + wake_up(&tp->xfer_wait); + return IRQ_WAKE_THREAD; } @@ -426,8 +428,6 @@ static irqreturn_t vsc_tp_thread_isr(int irq, void *data) { struct vsc_tp *tp = data; - wake_up(&tp->xfer_wait); - if (tp->event_notify) tp->event_notify(tp->event_notify_context); -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] mei: vsc: Unregister interrupt handler for system suspend 2024-04-03 5:13 [PATCH v2 1/2] Revert "mei: vsc: Call wake_up() in the threaded IRQ handler" Wentong Wu @ 2024-04-03 5:13 ` Wentong Wu 2024-04-03 12:00 ` Winkler, Tomas 2024-04-03 12:00 ` [PATCH v2 1/2] Revert "mei: vsc: Call wake_up() in the threaded IRQ handler" Winkler, Tomas 1 sibling, 1 reply; 4+ messages in thread From: Wentong Wu @ 2024-04-03 5:13 UTC (permalink / raw) To: sakari.ailus, tomas.winkler, gregkh Cc: linux-kernel, hao.yao, stable, Dominik Brodowski, Wentong Wu From: Sakari Ailus <sakari.ailus@linux.intel.com> Unregister the MEI VSC interrupt handler before system suspend and re-register it at system resume time. This mirrors implementation of other MEI devices. This patch fixes the bug that causes continuous stream of MEI VSC errors after system resume. Fixes: 386a766c4169 ("mei: Add MEI hardware support for IVSC device") Cc: stable@vger.kernel.org # for 6.8 Reported-by: Dominik Brodowski <linux@dominikbrodowski.net> Signed-off-by: Wentong Wu <wentong.wu@intel.com> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/misc/mei/platform-vsc.c | 17 ++++++- drivers/misc/mei/vsc-tp.c | 84 +++++++++++++++++++++++---------- drivers/misc/mei/vsc-tp.h | 3 ++ 3 files changed, 78 insertions(+), 26 deletions(-) diff --git a/drivers/misc/mei/platform-vsc.c b/drivers/misc/mei/platform-vsc.c index 6c9f00bcb94b..b543e6b9f3cf 100644 --- a/drivers/misc/mei/platform-vsc.c +++ b/drivers/misc/mei/platform-vsc.c @@ -400,25 +400,40 @@ static void mei_vsc_remove(struct platform_device *pdev) static int mei_vsc_suspend(struct device *dev) { struct mei_device *mei_dev = dev_get_drvdata(dev); + struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev); mei_stop(mei_dev); + mei_disable_interrupts(mei_dev); + + vsc_tp_free_irq(hw->tp); + return 0; } static int mei_vsc_resume(struct device *dev) { struct mei_device *mei_dev = dev_get_drvdata(dev); + struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev); int ret; - ret = mei_restart(mei_dev); + ret = vsc_tp_request_irq(hw->tp); if (ret) return ret; + ret = mei_restart(mei_dev); + if (ret) + goto err_free; + /* start timer if stopped in suspend */ schedule_delayed_work(&mei_dev->timer_work, HZ); return 0; + +err_free: + vsc_tp_free_irq(hw->tp); + + return ret; } static DEFINE_SIMPLE_DEV_PM_OPS(mei_vsc_pm_ops, mei_vsc_suspend, mei_vsc_resume); diff --git a/drivers/misc/mei/vsc-tp.c b/drivers/misc/mei/vsc-tp.c index 968a92a7425d..e6a98dba8a73 100644 --- a/drivers/misc/mei/vsc-tp.c +++ b/drivers/misc/mei/vsc-tp.c @@ -94,6 +94,27 @@ static const struct acpi_gpio_mapping vsc_tp_acpi_gpios[] = { {} }; +static irqreturn_t vsc_tp_isr(int irq, void *data) +{ + struct vsc_tp *tp = data; + + atomic_inc(&tp->assert_cnt); + + wake_up(&tp->xfer_wait); + + return IRQ_WAKE_THREAD; +} + +static irqreturn_t vsc_tp_thread_isr(int irq, void *data) +{ + struct vsc_tp *tp = data; + + if (tp->event_notify) + tp->event_notify(tp->event_notify_context); + + return IRQ_HANDLED; +} + /* wakeup firmware and wait for response */ static int vsc_tp_wakeup_request(struct vsc_tp *tp) { @@ -383,6 +404,37 @@ int vsc_tp_register_event_cb(struct vsc_tp *tp, vsc_tp_event_cb_t event_cb, } EXPORT_SYMBOL_NS_GPL(vsc_tp_register_event_cb, VSC_TP); +/** + * vsc_tp_request_irq - request irq for vsc_tp device + * @tp: vsc_tp device handle + */ +int vsc_tp_request_irq(struct vsc_tp *tp) +{ + struct spi_device *spi = tp->spi; + struct device *dev = &spi->dev; + int ret; + + irq_set_status_flags(spi->irq, IRQ_DISABLE_UNLAZY); + ret = request_threaded_irq(spi->irq, vsc_tp_isr, vsc_tp_thread_isr, + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, + dev_name(dev), tp); + if (ret) + return ret; + + return 0; +} +EXPORT_SYMBOL_NS_GPL(vsc_tp_request_irq, VSC_TP); + +/** + * vsc_tp_free_irq - free irq for vsc_tp device + * @tp: vsc_tp device handle + */ +void vsc_tp_free_irq(struct vsc_tp *tp) +{ + free_irq(tp->spi->irq, tp); +} +EXPORT_SYMBOL_NS_GPL(vsc_tp_free_irq, VSC_TP); + /** * vsc_tp_intr_synchronize - synchronize vsc_tp interrupt * @tp: vsc_tp device handle @@ -413,27 +465,6 @@ void vsc_tp_intr_disable(struct vsc_tp *tp) } EXPORT_SYMBOL_NS_GPL(vsc_tp_intr_disable, VSC_TP); -static irqreturn_t vsc_tp_isr(int irq, void *data) -{ - struct vsc_tp *tp = data; - - atomic_inc(&tp->assert_cnt); - - wake_up(&tp->xfer_wait); - - return IRQ_WAKE_THREAD; -} - -static irqreturn_t vsc_tp_thread_isr(int irq, void *data) -{ - struct vsc_tp *tp = data; - - if (tp->event_notify) - tp->event_notify(tp->event_notify_context); - - return IRQ_HANDLED; -} - static int vsc_tp_match_any(struct acpi_device *adev, void *data) { struct acpi_device **__adev = data; @@ -490,10 +521,9 @@ static int vsc_tp_probe(struct spi_device *spi) tp->spi = spi; irq_set_status_flags(spi->irq, IRQ_DISABLE_UNLAZY); - ret = devm_request_threaded_irq(dev, spi->irq, vsc_tp_isr, - vsc_tp_thread_isr, - IRQF_TRIGGER_FALLING | IRQF_ONESHOT, - dev_name(dev), tp); + ret = request_threaded_irq(spi->irq, vsc_tp_isr, vsc_tp_thread_isr, + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, + dev_name(dev), tp); if (ret) return ret; @@ -522,6 +552,8 @@ static int vsc_tp_probe(struct spi_device *spi) err_destroy_lock: mutex_destroy(&tp->mutex); + free_irq(spi->irq, tp); + return ret; } @@ -532,6 +564,8 @@ static void vsc_tp_remove(struct spi_device *spi) platform_device_unregister(tp->pdev); mutex_destroy(&tp->mutex); + + free_irq(spi->irq, tp); } static const struct acpi_device_id vsc_tp_acpi_ids[] = { diff --git a/drivers/misc/mei/vsc-tp.h b/drivers/misc/mei/vsc-tp.h index f9513ddc3e40..14ca195cbddc 100644 --- a/drivers/misc/mei/vsc-tp.h +++ b/drivers/misc/mei/vsc-tp.h @@ -37,6 +37,9 @@ int vsc_tp_xfer(struct vsc_tp *tp, u8 cmd, const void *obuf, size_t olen, int vsc_tp_register_event_cb(struct vsc_tp *tp, vsc_tp_event_cb_t event_cb, void *context); +int vsc_tp_request_irq(struct vsc_tp *tp); +void vsc_tp_free_irq(struct vsc_tp *tp); + void vsc_tp_intr_enable(struct vsc_tp *tp); void vsc_tp_intr_disable(struct vsc_tp *tp); void vsc_tp_intr_synchronize(struct vsc_tp *tp); -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2 2/2] mei: vsc: Unregister interrupt handler for system suspend 2024-04-03 5:13 ` [PATCH v2 2/2] mei: vsc: Unregister interrupt handler for system suspend Wentong Wu @ 2024-04-03 12:00 ` Winkler, Tomas 0 siblings, 0 replies; 4+ messages in thread From: Winkler, Tomas @ 2024-04-03 12:00 UTC (permalink / raw) To: Wu, Wentong, sakari.ailus, gregkh Cc: linux-kernel, Yao, Hao, stable, Dominik Brodowski > > From: Sakari Ailus <sakari.ailus@linux.intel.com> > > Unregister the MEI VSC interrupt handler before system suspend and re- > register it at system resume time. This mirrors implementation of other MEI > devices. > > This patch fixes the bug that causes continuous stream of MEI VSC errors > after system resume. > > Fixes: 386a766c4169 ("mei: Add MEI hardware support for IVSC device") > Cc: stable@vger.kernel.org # for 6.8 > Reported-by: Dominik Brodowski <linux@dominikbrodowski.net> > Signed-off-by: Wentong Wu <wentong.wu@intel.com> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Tomas Winkler <tomas.winkler@intel.com> > --- > drivers/misc/mei/platform-vsc.c | 17 ++++++- > drivers/misc/mei/vsc-tp.c | 84 +++++++++++++++++++++++---------- > drivers/misc/mei/vsc-tp.h | 3 ++ > 3 files changed, 78 insertions(+), 26 deletions(-) > > diff --git a/drivers/misc/mei/platform-vsc.c b/drivers/misc/mei/platform- > vsc.c index 6c9f00bcb94b..b543e6b9f3cf 100644 > --- a/drivers/misc/mei/platform-vsc.c > +++ b/drivers/misc/mei/platform-vsc.c > @@ -400,25 +400,40 @@ static void mei_vsc_remove(struct platform_device > *pdev) static int mei_vsc_suspend(struct device *dev) { > struct mei_device *mei_dev = dev_get_drvdata(dev); > + struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev); > > mei_stop(mei_dev); > > + mei_disable_interrupts(mei_dev); > + > + vsc_tp_free_irq(hw->tp); > + > return 0; > } > > static int mei_vsc_resume(struct device *dev) { > struct mei_device *mei_dev = dev_get_drvdata(dev); > + struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev); > int ret; > > - ret = mei_restart(mei_dev); > + ret = vsc_tp_request_irq(hw->tp); > if (ret) > return ret; > > + ret = mei_restart(mei_dev); > + if (ret) > + goto err_free; > + > /* start timer if stopped in suspend */ > schedule_delayed_work(&mei_dev->timer_work, HZ); > > return 0; > + > +err_free: > + vsc_tp_free_irq(hw->tp); > + > + return ret; > } > > static DEFINE_SIMPLE_DEV_PM_OPS(mei_vsc_pm_ops, mei_vsc_suspend, > mei_vsc_resume); diff --git a/drivers/misc/mei/vsc-tp.c > b/drivers/misc/mei/vsc-tp.c index 968a92a7425d..e6a98dba8a73 100644 > --- a/drivers/misc/mei/vsc-tp.c > +++ b/drivers/misc/mei/vsc-tp.c > @@ -94,6 +94,27 @@ static const struct acpi_gpio_mapping > vsc_tp_acpi_gpios[] = { > {} > }; > > +static irqreturn_t vsc_tp_isr(int irq, void *data) { > + struct vsc_tp *tp = data; > + > + atomic_inc(&tp->assert_cnt); > + > + wake_up(&tp->xfer_wait); > + > + return IRQ_WAKE_THREAD; > +} > + > +static irqreturn_t vsc_tp_thread_isr(int irq, void *data) { > + struct vsc_tp *tp = data; > + > + if (tp->event_notify) > + tp->event_notify(tp->event_notify_context); > + > + return IRQ_HANDLED; > +} > + > /* wakeup firmware and wait for response */ static int > vsc_tp_wakeup_request(struct vsc_tp *tp) { @@ -383,6 +404,37 @@ int > vsc_tp_register_event_cb(struct vsc_tp *tp, vsc_tp_event_cb_t event_cb, } > EXPORT_SYMBOL_NS_GPL(vsc_tp_register_event_cb, VSC_TP); > > +/** > + * vsc_tp_request_irq - request irq for vsc_tp device > + * @tp: vsc_tp device handle > + */ > +int vsc_tp_request_irq(struct vsc_tp *tp) { > + struct spi_device *spi = tp->spi; > + struct device *dev = &spi->dev; > + int ret; > + > + irq_set_status_flags(spi->irq, IRQ_DISABLE_UNLAZY); > + ret = request_threaded_irq(spi->irq, vsc_tp_isr, vsc_tp_thread_isr, > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > + dev_name(dev), tp); > + if (ret) > + return ret; > + > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(vsc_tp_request_irq, VSC_TP); > + > +/** > + * vsc_tp_free_irq - free irq for vsc_tp device > + * @tp: vsc_tp device handle > + */ > +void vsc_tp_free_irq(struct vsc_tp *tp) { > + free_irq(tp->spi->irq, tp); > +} > +EXPORT_SYMBOL_NS_GPL(vsc_tp_free_irq, VSC_TP); > + > /** > * vsc_tp_intr_synchronize - synchronize vsc_tp interrupt > * @tp: vsc_tp device handle > @@ -413,27 +465,6 @@ void vsc_tp_intr_disable(struct vsc_tp *tp) } > EXPORT_SYMBOL_NS_GPL(vsc_tp_intr_disable, VSC_TP); > > -static irqreturn_t vsc_tp_isr(int irq, void *data) -{ > - struct vsc_tp *tp = data; > - > - atomic_inc(&tp->assert_cnt); > - > - wake_up(&tp->xfer_wait); > - > - return IRQ_WAKE_THREAD; > -} > - > -static irqreturn_t vsc_tp_thread_isr(int irq, void *data) -{ > - struct vsc_tp *tp = data; > - > - if (tp->event_notify) > - tp->event_notify(tp->event_notify_context); > - > - return IRQ_HANDLED; > -} > - > static int vsc_tp_match_any(struct acpi_device *adev, void *data) { > struct acpi_device **__adev = data; > @@ -490,10 +521,9 @@ static int vsc_tp_probe(struct spi_device *spi) > tp->spi = spi; > > irq_set_status_flags(spi->irq, IRQ_DISABLE_UNLAZY); > - ret = devm_request_threaded_irq(dev, spi->irq, vsc_tp_isr, > - vsc_tp_thread_isr, > - IRQF_TRIGGER_FALLING | > IRQF_ONESHOT, > - dev_name(dev), tp); > + ret = request_threaded_irq(spi->irq, vsc_tp_isr, vsc_tp_thread_isr, > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > + dev_name(dev), tp); > if (ret) > return ret; > > @@ -522,6 +552,8 @@ static int vsc_tp_probe(struct spi_device *spi) > err_destroy_lock: > mutex_destroy(&tp->mutex); > > + free_irq(spi->irq, tp); > + > return ret; > } > > @@ -532,6 +564,8 @@ static void vsc_tp_remove(struct spi_device *spi) > platform_device_unregister(tp->pdev); > > mutex_destroy(&tp->mutex); > + > + free_irq(spi->irq, tp); > } > > static const struct acpi_device_id vsc_tp_acpi_ids[] = { diff --git > a/drivers/misc/mei/vsc-tp.h b/drivers/misc/mei/vsc-tp.h index > f9513ddc3e40..14ca195cbddc 100644 > --- a/drivers/misc/mei/vsc-tp.h > +++ b/drivers/misc/mei/vsc-tp.h > @@ -37,6 +37,9 @@ int vsc_tp_xfer(struct vsc_tp *tp, u8 cmd, const void > *obuf, size_t olen, int vsc_tp_register_event_cb(struct vsc_tp *tp, > vsc_tp_event_cb_t event_cb, > void *context); > > +int vsc_tp_request_irq(struct vsc_tp *tp); void vsc_tp_free_irq(struct > +vsc_tp *tp); > + > void vsc_tp_intr_enable(struct vsc_tp *tp); void vsc_tp_intr_disable(struct > vsc_tp *tp); void vsc_tp_intr_synchronize(struct vsc_tp *tp); > -- > 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2 1/2] Revert "mei: vsc: Call wake_up() in the threaded IRQ handler" 2024-04-03 5:13 [PATCH v2 1/2] Revert "mei: vsc: Call wake_up() in the threaded IRQ handler" Wentong Wu 2024-04-03 5:13 ` [PATCH v2 2/2] mei: vsc: Unregister interrupt handler for system suspend Wentong Wu @ 2024-04-03 12:00 ` Winkler, Tomas 1 sibling, 0 replies; 4+ messages in thread From: Winkler, Tomas @ 2024-04-03 12:00 UTC (permalink / raw) To: Wu, Wentong, sakari.ailus, gregkh; +Cc: linux-kernel, Yao, Hao, stable > handler" > > From: Sakari Ailus <sakari.ailus@linux.intel.com> > > This reverts commit 058a38acba15fd8e7b262ec6e17c4204cb15f984. > > It's not necessary to avoid a spinlock, a sleeping lock on PREEMPT_RT, in an > interrupt handler as the interrupt handler itself would be called in a process > context if PREEMPT_RT is enabled. So revert the patch. > > Cc: stable@vger.kernel.org # for 6.8 > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Tomas Winkler <tomas.winkler@intel.com> > --- > drivers/misc/mei/vsc-tp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/misc/mei/vsc-tp.c b/drivers/misc/mei/vsc-tp.c index > ecfb70cd057c..968a92a7425d 100644 > --- a/drivers/misc/mei/vsc-tp.c > +++ b/drivers/misc/mei/vsc-tp.c > @@ -419,6 +419,8 @@ static irqreturn_t vsc_tp_isr(int irq, void *data) > > atomic_inc(&tp->assert_cnt); > > + wake_up(&tp->xfer_wait); > + > return IRQ_WAKE_THREAD; > } > > @@ -426,8 +428,6 @@ static irqreturn_t vsc_tp_thread_isr(int irq, void > *data) { > struct vsc_tp *tp = data; > > - wake_up(&tp->xfer_wait); > - > if (tp->event_notify) > tp->event_notify(tp->event_notify_context); > > -- > 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-03 12:01 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-03 5:13 [PATCH v2 1/2] Revert "mei: vsc: Call wake_up() in the threaded IRQ handler" Wentong Wu 2024-04-03 5:13 ` [PATCH v2 2/2] mei: vsc: Unregister interrupt handler for system suspend Wentong Wu 2024-04-03 12:00 ` Winkler, Tomas 2024-04-03 12:00 ` [PATCH v2 1/2] Revert "mei: vsc: Call wake_up() in the threaded IRQ handler" Winkler, Tomas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®