RE: mei driver breaks suspend on 3.0git

From: Winkler, Tomas
Date: Sun Jun 12 2011 - 15:42:12 EST




> -----Original Message-----
> From: Arkadiusz Miskiewicz [mailto:a.miskiewicz@xxxxxxxxx]
> Sent: Sunday, June 12, 2011 9:40 PM
> To: Winkler, Tomas
> Cc: Weil, Oren jer; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: mei driver breaks suspend on 3.0git
>
> On Sunday 12 of June 2011, Arkadiusz Miskiewicz wrote:
> > On Sunday 12 of June 2011, Arkadiusz Miskiewicz wrote:
> > > On Sunday 12 of June 2011, Winkler, Tomas wrote:
> > > > > -----Original Message-----
> > > > > From: Weil, Oren jer
> > > > > Sent: Sunday, June 12, 2011 8:36 PM
> > > > > To: Arkadiusz Miskiewicz
> > > > > Cc: Winkler, Tomas; linux-kernel@xxxxxxxxxxxxxxx
> > > > > Subject: RE: mei driver breaks suspend on 3.0git
> > > > >
> > > > > >It's thinkpad t400 with
> > > > > >00:03.0 Communication controller [0780]: Intel Corporation
> > > > > >Mobile 4 Series Chipset MEI Controller [8086:2a44] (rev 07)
> > > > >
> > > > > Can you send us more details? Does it happened in stress? How do
> > > > > I reproduce it?
> > >
> > > No stress. This is my daily use notebook and I simply suspend to ram
> > > when sitting under X in kde4.
> > >
> > > > > Can you send us the debug prints?
> > >
> > > Attached. suspend starts, tries to resume devices, MEI suspend fails
> > > causing whole suspend to be aborted.
> >
> > Shouldn't ret > 0 in mei_wd_stop to be "success"?
>
> I meant this one:
>
> ret = wait_event_interruptible_timeout(dev->wait_stop_wd,
> dev->wd_stopped, 10 * HZ);
>
>
> And that was possible untested solution that relies on wd being stopped and
> not anything else.
>
> >
> > diff --git a/drivers/staging/mei/wd.c b/drivers/staging/mei/wd.c index
> > 2564b03..608245a 100644
> > --- a/drivers/staging/mei/wd.c
> > +++ b/drivers/staging/mei/wd.c
> > @@ -171,8 +171,10 @@ int mei_wd_stop(struct mei_device *dev, bool
> preserve)
> > mutex_lock(&dev->device_lock);
> > if (!dev->wd_stopped)
> > dev_dbg(&dev->pdev->dev, "stop wd failed to complete.\n");
> > - else
> > + else {
> > + ret = 0;
> > dev_dbg(&dev->pdev->dev, "stop wd complete.\n");
> > + }
> >
> > if (preserve)
> > dev->wd_timeout = wd_timeout;
>

Looks like your patch is correct
wait_event_interruptible_timeout() returns >0 on success unlike wait_even_interruptable()

I wil check all the instances we have and issue a patch.
Thanks for your effort
Tomas

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸ ¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðà nÆàþY&—