Randy Fishel wrote:
> 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.
>
I am having trouble understanding this idea.... one ISR for multiple
devices? If this hypothetical device is a nexus driver (it should
be!), then its children are pretty much guaranteed to have suspended
first, right?
One hopes that such a device has some level of independent control over
functions (the ability to shutdown one function so that it doesn't
generate interrupts, for example.) If not, then probably the idea of
"multiple" instances is the wrong model to use for the driver, and
instead it should use a single devinfo node with multiple minor numbers
for the different "functions".
>
> 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.
>
There is some information in WDD, and *example* code is correct. Where
we hit pitfalls is where driver developers try to take short-cuts.
Folks that have worked with suspend/resume on SPARC hardware should
understand the rules. But there are a lot of folks who are new to
suspend/resume in the x86 world, and it will take some time to educate
everyone properly.
>
>>> 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.
>
Thanks. Let me know if you need help with it. I consider this a fairly
urgent issue as well.
-- Garrett
_______________________________________________
driver-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/driver-discuss
