Re: [PATCH] fix tulip suspend/resume

[Adam Belay]

On Thu, 2005-06-09 at 02:04 +0200, Pavel Machek wrote:
> Hi!
> 
> > > > I think we should also use the pm_message_t defines.  We will need to
> > > > add PMSG_FREEZE eventually.  I decided to default to the current state
> > > > rather than panic.  Does this patch look ok?
> > > 
> > > No.
> > 
> > Hrm... I don't follow you anymore here ...
> > 
> > >         case PM_EVENT_ON:
> > >                 return PCI_D0;
> > >         case PM_EVENT_FREEZE:
> > >         case PM_EVENT_SUSPEND:
> > >                 return PCI_D3hot;
> > 
> > What are these new PM_EVENT_* things ? I though we defined PMSG_* ?
> 
> PMSG_* are for struct; you can't case on struct.
> 
> > > You passed invalid argument; I see no reason why you should paper over
> > > it and risk continuing. This happens during system suspend; it is
> > > quite possible that user will not see your printk when machine powers
> > > off just after that; and remember that it will not be in syslog after
> > > resume.
> > 
> > Crap. I don't think a BUG() makes any useful help neither in this place,
> > and when I locally turn PMSG_FREEZE to something sane I suddenly blow up
> > in there (and I wonder in how many other places).
> 
> At least you can see & report that error... That would not be a case
> for simple printk.

Well not exactly.  The video device may have already been disabled, so
the user wouldn't see it anyway.  The reason I don't like BUG() here is
that it's very unlikely passing an invalid PMSG will have serious
consequences.  The problems are likely less significant than those
caused by calling BUG().  Many suspend routines don't set power at all
yet (e.g. cardbus) and it still works out fine.  Defaulting to the
current state seems like a very safe behavior.  Then, after resume, we
can see the messages.  I'd like to see PM just work, and we have to be
careful during an API change.

> 
> This is the patch I'd like to go in. I hope it makes it
> clear... Please base your development on top of this one...

Yeah, I wasn't sure where those flags were coming from...

Thanks,
Adam


> 									Pavel
> 
> ---
> 
> Turn pm_message_t into struct, so that it is typechecked properly (and
> so that we can add flags field in future). This should not go in
> before 2.6.12.

What do you have in mind for adding to the structure in the future?


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/