[char-misc v3] mei: me: reduce the scope on unexpected reset

[char-misc v3] mei: me: reduce the scope on unexpected reset

[Alexander Usyskin]

After commit 2cedb296988c ("mei: me: trigger link reset if hw ready is unexpected")
some devices started to show long resume times (5-7 seconds).
This happens as mei falsely detects unready hardware,
starts parallel link reset flow and triggers link reset timeouts
in the resume callback.

Address it by performing detection of unready hardware only
when driver is in the ENABLED state instead of blacklisting
states as done in the original patch.

Reported-by: Todd Brandt <todd.e.brandt@linux.intel.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221023
Tested-by: Todd Brandt <todd.e.brandt@linux.intel.com>
Fixes: 2cedb296988c ("mei: me: trigger link reset if hw ready is unexpected")
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---

V3: reword commit message, add Rafael and PM list

V2: rebase over v7.0-rc4

 drivers/misc/mei/hw-me.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index d4612c659784..1e4a41ac428f 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -1337,19 +1337,13 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 	/*  check if we need to start the dev */
 	if (!mei_host_is_ready(dev)) {
 		if (mei_hw_is_ready(dev)) {
-			/* synchronized by dev mutex */
-			if (waitqueue_active(&dev->wait_hw_ready)) {
-				dev_dbg(&dev->dev, "we need to start the dev.\n");
-				dev->recvd_hw_ready = true;
-				wake_up(&dev->wait_hw_ready);
-			} else if (dev->dev_state != MEI_DEV_UNINITIALIZED &&
-				   dev->dev_state != MEI_DEV_POWERING_DOWN &&
-				   dev->dev_state != MEI_DEV_POWER_DOWN) {
+			if (dev->dev_state == MEI_DEV_ENABLED) {
 				dev_dbg(&dev->dev, "Force link reset.\n");
 				schedule_work(&dev->reset_work);
 			} else {
-				dev_dbg(&dev->dev, "Ignore this interrupt in state = %d\n",
-					dev->dev_state);
+				dev_dbg(&dev->dev, "we need to start the dev.\n");
+				dev->recvd_hw_ready = true;
+				wake_up(&dev->wait_hw_ready);
 			}
 		} else {
 			dev_dbg(&dev->dev, "Spurious Interrupt\n");
-- 
2.43.0

Re: [char-misc v3] mei: me: reduce the scope on unexpected reset

[Rafael J. Wysocki]

On Thu, Mar 26, 2026 at 4:14 PM Alexander Usyskin
<alexander.usyskin@intel.com> wrote:
>
> After commit 2cedb296988c ("mei: me: trigger link reset if hw ready is unexpected")
> some devices started to show long resume times (5-7 seconds).
> This happens as mei falsely detects unready hardware,
> starts parallel link reset flow and triggers link reset timeouts
> in the resume callback.
>
> Address it by performing detection of unready hardware only
> when driver is in the ENABLED state instead of blacklisting
> states as done in the original patch.
>
> Reported-by: Todd Brandt <todd.e.brandt@linux.intel.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221023
> Tested-by: Todd Brandt <todd.e.brandt@linux.intel.com>
> Fixes: 2cedb296988c ("mei: me: trigger link reset if hw ready is unexpected")
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>
> V3: reword commit message, add Rafael and PM list
>
> V2: rebase over v7.0-rc4
>
>  drivers/misc/mei/hw-me.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
> index d4612c659784..1e4a41ac428f 100644
> --- a/drivers/misc/mei/hw-me.c
> +++ b/drivers/misc/mei/hw-me.c
> @@ -1337,19 +1337,13 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
>         /*  check if we need to start the dev */
>         if (!mei_host_is_ready(dev)) {
>                 if (mei_hw_is_ready(dev)) {
> -                       /* synchronized by dev mutex */

I think that the comment above is still valid, isn't it?  If so, why remove it?

> -                       if (waitqueue_active(&dev->wait_hw_ready)) {
> -                               dev_dbg(&dev->dev, "we need to start the dev.\n");
> -                               dev->recvd_hw_ready = true;
> -                               wake_up(&dev->wait_hw_ready);
> -                       } else if (dev->dev_state != MEI_DEV_UNINITIALIZED &&
> -                                  dev->dev_state != MEI_DEV_POWERING_DOWN &&
> -                                  dev->dev_state != MEI_DEV_POWER_DOWN) {
> +                       if (dev->dev_state == MEI_DEV_ENABLED) {
>                                 dev_dbg(&dev->dev, "Force link reset.\n");
>                                 schedule_work(&dev->reset_work);
>                         } else {

Isn't the waitqueue_active() check needed any more?

> -                               dev_dbg(&dev->dev, "Ignore this interrupt in state = %d\n",
> -                                       dev->dev_state);
> +                               dev_dbg(&dev->dev, "we need to start the dev.\n");
> +                               dev->recvd_hw_ready = true;
> +                               wake_up(&dev->wait_hw_ready);
>                         }
>                 } else {
>                         dev_dbg(&dev->dev, "Spurious Interrupt\n");
> --

RE: [char-misc v3] mei: me: reduce the scope on unexpected reset

[Usyskin, Alexander]

> Subject: Re: [char-misc v3] mei: me: reduce the scope on unexpected reset
> 
> On Thu, Mar 26, 2026 at 4:14 PM Alexander Usyskin
> <alexander.usyskin@intel.com> wrote:
> >
> > After commit 2cedb296988c ("mei: me: trigger link reset if hw ready is
> unexpected")
> > some devices started to show long resume times (5-7 seconds).
> > This happens as mei falsely detects unready hardware,
> > starts parallel link reset flow and triggers link reset timeouts
> > in the resume callback.
> >
> > Address it by performing detection of unready hardware only
> > when driver is in the ENABLED state instead of blacklisting
> > states as done in the original patch.
> >
> > Reported-by: Todd Brandt <todd.e.brandt@linux.intel.com>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221023
> > Tested-by: Todd Brandt <todd.e.brandt@linux.intel.com>
> > Fixes: 2cedb296988c ("mei: me: trigger link reset if hw ready is unexpected")
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > ---
> >
> > V3: reword commit message, add Rafael and PM list
> >
> > V2: rebase over v7.0-rc4
> >
> >  drivers/misc/mei/hw-me.c | 14 ++++----------
> >  1 file changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
> > index d4612c659784..1e4a41ac428f 100644
> > --- a/drivers/misc/mei/hw-me.c
> > +++ b/drivers/misc/mei/hw-me.c
> > @@ -1337,19 +1337,13 @@ irqreturn_t mei_me_irq_thread_handler(int
> irq, void *dev_id)
> >         /*  check if we need to start the dev */
> >         if (!mei_host_is_ready(dev)) {
> >                 if (mei_hw_is_ready(dev)) {
> > -                       /* synchronized by dev mutex */
> 
> I think that the comment above is still valid, isn't it?  If so, why remove it?
> 
> > -                       if (waitqueue_active(&dev->wait_hw_ready)) {
> > -                               dev_dbg(&dev->dev, "we need to start the dev.\n");
> > -                               dev->recvd_hw_ready = true;
> > -                               wake_up(&dev->wait_hw_ready);
> > -                       } else if (dev->dev_state != MEI_DEV_UNINITIALIZED &&
> > -                                  dev->dev_state != MEI_DEV_POWERING_DOWN &&
> > -                                  dev->dev_state != MEI_DEV_POWER_DOWN) {
> > +                       if (dev->dev_state == MEI_DEV_ENABLED) {
> >                                 dev_dbg(&dev->dev, "Force link reset.\n");
> >                                 schedule_work(&dev->reset_work);
> >                         } else {
> 
> Isn't the waitqueue_active() check needed any more?
> 

The fix simplifies original patch as in MEI_DEV_ENABLED state there will be no active waitqueue.
This also removes need for comment about waitqueue synchronization.

- - 
Thanks,
Sasha

Re: [char-misc v3] mei: me: reduce the scope on unexpected reset

[Rafael J. Wysocki]

On Sun, Mar 29, 2026 at 4:30 PM Usyskin, Alexander
<alexander.usyskin@intel.com> wrote:
>
> > Subject: Re: [char-misc v3] mei: me: reduce the scope on unexpected reset
> >
> > On Thu, Mar 26, 2026 at 4:14 PM Alexander Usyskin
> > <alexander.usyskin@intel.com> wrote:
> > >
> > > After commit 2cedb296988c ("mei: me: trigger link reset if hw ready is
> > unexpected")
> > > some devices started to show long resume times (5-7 seconds).
> > > This happens as mei falsely detects unready hardware,
> > > starts parallel link reset flow and triggers link reset timeouts
> > > in the resume callback.
> > >
> > > Address it by performing detection of unready hardware only
> > > when driver is in the ENABLED state instead of blacklisting
> > > states as done in the original patch.
> > >
> > > Reported-by: Todd Brandt <todd.e.brandt@linux.intel.com>
> > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221023
> > > Tested-by: Todd Brandt <todd.e.brandt@linux.intel.com>
> > > Fixes: 2cedb296988c ("mei: me: trigger link reset if hw ready is unexpected")
> > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > ---
> > >
> > > V3: reword commit message, add Rafael and PM list
> > >
> > > V2: rebase over v7.0-rc4
> > >
> > >  drivers/misc/mei/hw-me.c | 14 ++++----------
> > >  1 file changed, 4 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
> > > index d4612c659784..1e4a41ac428f 100644
> > > --- a/drivers/misc/mei/hw-me.c
> > > +++ b/drivers/misc/mei/hw-me.c
> > > @@ -1337,19 +1337,13 @@ irqreturn_t mei_me_irq_thread_handler(int
> > irq, void *dev_id)
> > >         /*  check if we need to start the dev */
> > >         if (!mei_host_is_ready(dev)) {
> > >                 if (mei_hw_is_ready(dev)) {
> > > -                       /* synchronized by dev mutex */
> >
> > I think that the comment above is still valid, isn't it?  If so, why remove it?
> >
> > > -                       if (waitqueue_active(&dev->wait_hw_ready)) {
> > > -                               dev_dbg(&dev->dev, "we need to start the dev.\n");
> > > -                               dev->recvd_hw_ready = true;
> > > -                               wake_up(&dev->wait_hw_ready);
> > > -                       } else if (dev->dev_state != MEI_DEV_UNINITIALIZED &&
> > > -                                  dev->dev_state != MEI_DEV_POWERING_DOWN &&
> > > -                                  dev->dev_state != MEI_DEV_POWER_DOWN) {
> > > +                       if (dev->dev_state == MEI_DEV_ENABLED) {
> > >                                 dev_dbg(&dev->dev, "Force link reset.\n");
> > >                                 schedule_work(&dev->reset_work);
> > >                         } else {
> >
> > Isn't the waitqueue_active() check needed any more?
> >
>
> The fix simplifies original patch as in MEI_DEV_ENABLED state there will be no active waitqueue.
> This also removes need for comment about waitqueue synchronization.

I see, thanks.

It would be good to add the above information to the patch changelog.

With that, please feel free to add

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

to the patch.