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@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221023
> > > Tested-by: Todd Brandt <todd.e.brandt@xxxxxxxxxxxxxxx>
> > > Fixes: 2cedb296988c ("mei: me: trigger link reset if hw ready is unexpected")
> > > Signed-off-by: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
> > > ---
> > >
> > > 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@xxxxxxxxxx>

to the patch.