On Fri, 9 Nov 2007, Garrett D'Amore wrote:
> Juergen Keil wrote:
> > Garrett wrote:
> >
> >> Dan Mick wrote:
> >>
> >>> Juergen Keil wrote:
> >>>
> >>>
> >>>
> >>>> How is a driver supposed to behave in such a case, when it has
> >>>> sucessfully suspended the device but the interrupt handler is
> >>>> still invoked because it is sharing the interrupt with some
> >>>> other device that is not yet suspended?
One of the biggest mistakes made by driver writers for Solaris
suspend/resume, is that after it returns DDI_SUCCESS, it won't *ever*
be called again till their attach() is called. In many cases this
doesn't happen, but it can so the driver needs to be aware and protect
itself. And their isn't a lot of documentation surrounding this and
many other suspend/resume issues (but I am working on that).
> >>>>
> >>>> Shouldn't ohci set some flag in its softstate when
> >>>> ohci_detach(DDI_SUSPEND) has run, and just return with
> >>>> DDI_INTR_UNCLAIMED from ohci_intr() when ohci_intr is invoked
> >>>> for such a suspended device?
Device drivers that have interrupt routines *MUST* understand they
are suspended so they don't update their hardware, and return
DDI_INTR_UNCLAIMED, even if they have disabled their own interrupts.
The only exception, is if there is a single routine for several
hardware devices, and in that case it is important for the isr to know
which devices have been suspended, and service interrupts for those
that have not.
> >>>>
> >>>>
> >>> Certainly return without doing damage, yes, and
> >>> DDI_INTR_UNCLAIMED seems like the best possible way...
> >>>
> >>> I'm surprised suspendable drivers don't do this as a matter of
> >>> course.
> >>>
> >
> > That is, removing the interrupt handler on suspend and reinstalling on
> > resume isn't necessary?
> >
>
> Correct. You *do* need to turn off interrupts on the *device*, though.
This may not always be correct, but likely so for most devices.
Multi-function devices may well share a single interrupt mask, and
turning it off could well prevent other devices from suspending. But
as far as I can tell, it is sufficient to just return
DDI_INTR_UNCLAIMED if the driver is already suspended.
>
>
> >
> >
> >> Most of them do, I think.
> >>
Again, I think too many driver writers assume that they won't ever
be called again till attach time. And we havn't' helped much as we
don't _really_ document the possible pitfalls.
> >
> >
> > ohci doesn't.
> >
> > And ehci apparently has the same problem.
> >
> > uhci has no suspend/resume support at this time.
> >
> >
> > Changing ohci like this enables suspend for ohci on my ASUS P2B-LS box,
> > so that the box does not hang any more on a suspend attempt:
> >
> > diff -r 5e2f30c0e8dc usr/src/uts/common/io/usb/hcd/openhci/ohci.c
> > --- a/usr/src/uts/common/io/usb/hcd/openhci/ohci.c Tue Nov 06 02:08:12
> > 2007
> > -0800
> > +++ b/usr/src/uts/common/io/usb/hcd/openhci/ohci.c Thu Nov 08 18:14:30
> > 2007
> > +0100
> > @@ -7625,6 +7634,12 @@ ohci_intr(caddr_t arg1, caddr_t arg2)
> > "ohci_intr: Interrupt occurred, arg1 0x%p arg2 0x%p", arg1,
> > arg2);
> >
> > mutex_enter(&ohcip->ohci_int_mutex);
> > +
> > + /* Don't touch a suspended device */
> > + if (ohcip->ohci_hc_soft_state == OHCI_CTLR_SUSPEND_STATE) {
> > + mutex_exit(&ohcip->ohci_int_mutex);
> > + return (DDI_INTR_UNCLAIMED);
> > + }
> >
> > /*
> > * Suppose if we switched to the polled mode from the normal
> >
> >
> This looks like the right fix for ohci.
>
> Randy, do you want to take this?
On one side, I would like the owner of the driver to do this fix so
that they undersand the relevance of this fix for other drivers. On
the other, I would want this fix (and possibly ehci) in the sxde
release, so it needs to make the next build. I will take on the
responsibility of making sure it happens.
Cheers!
---- Randy
>
> -- Garrett
>
_______________________________________________
driver-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/driver-discuss
