Re: 2.6.0-test9 regression: panic on Fn+F7 hotkey

Ducrot Bruno
2003-10-28T13:57:46Z

On Tue, Oct 28, 2003 at 08:33:16PM +0800, Li, Shaohua wrote:
>
> > -----Original Message-----
> > From: Ducrot Bruno [mailto:ducrot-kk6yZipjEM5g9hUCZPvPmw@public.gmane.org]
> > Sent: 2003??10??28?? 18:04
> > To: Li, Shaohua
> > Cc: Sérgio Monteiro Basto; Karol Kozimor; acpi-devel
> > Subject: Re: [ACPI] 2.6.0-test9 regression: panic on Fn+F7 hotkey
> >
> > On Tue, Oct 28, 2003 at 04:14:56PM +0800, Li, Shaohua wrote:
> > > Hi,
> > >
> > > I missed non-ECDT situation. Try this one.
> > >
> >
> > Please don't apply. This is not OK. You missed the
> > point that an interrupt handler can not sleep. Anyway,
> > from my point-of-view, ec.c is not buggy, but
> > events/exviface.c::acpi_remove_.*().
> >
> > IMHO, we should have our own acpi workqueue, at first, and
> > then wait for completion when removing any events handlers.
> > Those, the correct fix should be in osl.c, and in events/evxface.c
> > calling this wait for completition implemented in osl.c.
>
> yes, I know it's a bad idea to execute GPE query in interrupt handler. I do such thing is because this situation is rare. The interrupts seldom occur before EC device is initialized.
>
> But even though acpi has own workqueue, When replace ECDT with EC, we still have a wrong context.

Yes, there is still a little window where unlikely this may happens (that why
I think that should go in osl.c, because anyway this is more generic than
the EC problem, and have to be done in OS fashion).

>
> I think a better solution is when replace ECDT with EC, we can change the context in the event queue. Or we can use indirectly accessed context. Like this:
>
> --- ec.c 2003-10-09 03:24:07.000000000 +0800
> +++ ec.c.new 2003-10-28 20:17:05.643232728 +0800
> @@ -93,7 +93,7 @@
>
> /* External interfaces use first EC only, so remember */
> static struct acpi_device *first_ec;
> -
> +static struct acpi_device *ec_context = NULL;
> /* --------------------------------------------------------------------------
> Transaction Management
> -------------------------------------------------------------------------- */
> @@ -386,7 +386,7 @@
> void *data)
> {
> acpi_status status = AE_OK;
> - struct acpi_ec *ec = (struct acpi_ec *) data;
> + struct acpi_ec *ec = ec_context;
>
> if (!ec)
> return;

...

What happens if you have more than one EC? Toshiba Satellite 190x
series have 2 EC IIRC, and only one will have an interrupt handler.

Cheers,

--
Ducrot Bruno

-- Which is worse: ignorance or apathy?
-- Don't know. Don't care.