2003-05-15 02:10:37

by Felipe Alfaro Solana

[permalink] [raw]
Subject: 2.5.69-mm5: reverting i8259-shutdown.patch

Hi again, Andrew,

Besides the "make_KOBJ_NAME-match_BUS_ID_SIZE.patch" causing "pccard"
oopses, I've also found that, with 2.5.69-mm5 compiled with ACPI
support, my laptop is unable to power off. The kernel invokes
"acpi_power_off" and stays there forever.

I've found that reverting the "i8259-shutdown.patch" fixes the problem
and my laptop is able to shutdown properly (init 0) when using ACPI.

A hardware bug? A kernel bug?

Thanks!



2003-05-15 02:18:54

by Andrew Morton

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

Felipe Alfaro Solana <[email protected]> wrote:
>
> Hi again, Andrew,
>
> Besides the "make_KOBJ_NAME-match_BUS_ID_SIZE.patch" causing "pccard"
> oopses, I've also found that, with 2.5.69-mm5 compiled with ACPI
> support, my laptop is unable to power off. The kernel invokes
> "acpi_power_off" and stays there forever.
>
> I've found that reverting the "i8259-shutdown.patch" fixes the problem
> and my laptop is able to shutdown properly (init 0) when using ACPI.
>
> A hardware bug? A kernel bug?

And thanks again, again.

That's the below patch. It looks pretty innocuous. I'd be assuming that
there's something in the shutdown sequence which needs 8259 functionality
after the thing has been turned off.

This could well depend upon .config contents and linkage order.

Eric, maybe we need to turn it off by hand at the right time rather than
relying on driver model shutdown ordering?


arch/i386/kernel/i8259.c | 11 +++++++++++
1 files changed, 11 insertions(+)

diff -puN arch/i386/kernel/i8259.c~i8259-shutdown arch/i386/kernel/i8259.c
--- 25/arch/i386/kernel/i8259.c~i8259-shutdown 2003-05-11 18:48:58.000000000 -0700
+++ 25-akpm/arch/i386/kernel/i8259.c 2003-05-11 18:48:58.000000000 -0700
@@ -245,10 +245,21 @@ static int i8259A_resume(struct device *
return 0;
}

+static void i8259A_shutdown(struct device *dev)
+{
+ /* Put the i8259A into a quiescent state that
+ * the kernel initialization code can get it
+ * out of.
+ */
+ outb(0xff, 0x21); /* mask all of 8259A-1 */
+ outb(0xff, 0xA1); /* mask all of 8259A-1 */
+}
+
static struct device_driver i8259A_driver = {
.name = "pic",
.bus = &system_bus_type,
.resume = i8259A_resume,
+ .shutdown = i8259A_shutdown,
};

static struct sys_device device_i8259A = {

_

2003-05-15 02:25:59

by Patrick Mochel

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch


On Wed, 14 May 2003, Andrew Morton wrote:

> Felipe Alfaro Solana <[email protected]> wrote:
> >
> > Hi again, Andrew,
> >
> > Besides the "make_KOBJ_NAME-match_BUS_ID_SIZE.patch" causing "pccard"
> > oopses, I've also found that, with 2.5.69-mm5 compiled with ACPI
> > support, my laptop is unable to power off. The kernel invokes
> > "acpi_power_off" and stays there forever.
> >
> > I've found that reverting the "i8259-shutdown.patch" fixes the problem
> > and my laptop is able to shutdown properly (init 0) when using ACPI.
> >
> > A hardware bug? A kernel bug?
>
> And thanks again, again.
>
> That's the below patch. It looks pretty innocuous. I'd be assuming that
> there's something in the shutdown sequence which needs 8259 functionality
> after the thing has been turned off.
>
> This could well depend upon .config contents and linkage order.
>
> Eric, maybe we need to turn it off by hand at the right time rather than
> relying on driver model shutdown ordering?

Interesting. This is yet more proof that system-level devices cannot be
treated as common, everyday devices. Sure, it's nice to see them show up
in sysfs with little overhead, and very nice not to have to work about
them during shutdown or power transitions. But there are just too many
special cases (like getting the ordering right ;) that you have to worry
about.

So, what do we do with them?


-pat

2003-05-15 02:42:12

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Wed, 14 May 2003, Patrick Mochel wrote:

> Interesting. This is yet more proof that system-level devices cannot be
> treated as common, everyday devices. Sure, it's nice to see them show up
> in sysfs with little overhead, and very nice not to have to work about
> them during shutdown or power transitions. But there are just too many
> special cases (like getting the ordering right ;) that you have to worry
> about.
>
> So, what do we do with them?

Does the PIC shutdown callback get called _just_ before acpi_power_off?

Zwane
--
function.linuxpower.ca

2003-05-15 02:46:55

by Patrick Mochel

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch


On Wed, 14 May 2003, Zwane Mwaikambo wrote:

> On Wed, 14 May 2003, Patrick Mochel wrote:
>
> > Interesting. This is yet more proof that system-level devices cannot be
> > treated as common, everyday devices. Sure, it's nice to see them show up
> > in sysfs with little overhead, and very nice not to have to work about
> > them during shutdown or power transitions. But there are just too many
> > special cases (like getting the ordering right ;) that you have to worry
> > about.
> >
> > So, what do we do with them?
>
> Does the PIC shutdown callback get called _just_ before acpi_power_off?

It may or may not be. It depends on which order it gets registered in,
which is arbitrary (*). One can enable DEBUG in drivers/base/power.c and
crank up the console log level to see the order that devices get shutdown
in on their system.


-pat

(*) - Ok, we can control the registration via link order, but that's
something we'll be tweaking until our dying days in absence of a good
solution.

2003-05-15 06:00:02

by Andrew Morton

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

Patrick Mochel <[email protected]> wrote:
>
> > Eric, maybe we need to turn it off by hand at the right time rather than
> > relying on driver model shutdown ordering?
>
> Interesting. This is yet more proof that system-level devices cannot be
> treated as common, everyday devices. Sure, it's nice to see them show up
> in sysfs with little overhead, and very nice not to have to work about
> them during shutdown or power transitions. But there are just too many
> special cases (like getting the ordering right ;) that you have to worry
> about.
>
> So, what do we do with them?

I'd say that as long as the shutdown routines are executed in reverse
order of startup, then the core driver stuff has fulfilled its
obligations.

In this case we need to understand why the lockup is happening - what
code is requiring 8259 services after the thing has been turned off?
Could be that the bug lies there.

Felipe, please send your .config. (again - in fact you may as well do
cp .config ~/.signature)



2003-05-15 06:10:41

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Wed, 14 May 2003, Andrew Morton wrote:

> I'd say that as long as the shutdown routines are executed in reverse
> order of startup, then the core driver stuff has fulfilled its
> obligations.
>
> In this case we need to understand why the lockup is happening - what
> code is requiring 8259 services after the thing has been turned off?
> Could be that the bug lies there.

The registration is somewhat unfair here, it depends on device_initcall
and we initialise the 8259 in init_IRQ.

Zwane
--
function.linuxpower.ca

2003-05-15 07:52:56

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Thu, 15 May 2003, Zwane Mwaikambo wrote:

> On Wed, 14 May 2003, Andrew Morton wrote:
>
> > I'd say that as long as the shutdown routines are executed in reverse
> > order of startup, then the core driver stuff has fulfilled its
> > obligations.
> >
> > In this case we need to understand why the lockup is happening - what
> > code is requiring 8259 services after the thing has been turned off?
> > Could be that the bug lies there.
>
> The registration is somewhat unfair here, it depends on device_initcall
> and we initialise the 8259 in init_IRQ.

Pat what do you say to some late shutdown callbacks? I'll drop you a
patch sometime tommorrow.

Zwane
--
function.linuxpower.ca

2003-05-15 11:50:09

by Felipe Alfaro Solana

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Thu, 2003-05-15 at 05:00, Patrick Mochel wrote:
> On Wed, 14 May 2003, Zwane Mwaikambo wrote:
>
> > On Wed, 14 May 2003, Patrick Mochel wrote:
> >
> > > Interesting. This is yet more proof that system-level devices cannot be
> > > treated as common, everyday devices. Sure, it's nice to see them show up
> > > in sysfs with little overhead, and very nice not to have to work about
> > > them during shutdown or power transitions. But there are just too many
> > > special cases (like getting the ordering right ;) that you have to worry
> > > about.
> > >
> > > So, what do we do with them?
> >
> > Does the PIC shutdown callback get called _just_ before acpi_power_off?
>
> It may or may not be. It depends on which order it gets registered in,
> which is arbitrary (*). One can enable DEBUG in drivers/base/power.c and
> crank up the console log level to see the order that devices get shutdown
> in on their system.

OK, I've changed "#undef DEBUG" to "#define DEBUG" in
drivers/base/power.c, but during shutdown, I can see no extra debug
messages. What am I doing wrong?

2003-05-15 11:54:54

by Felipe Alfaro Solana

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Thu, 2003-05-15 at 08:14, Andrew Morton wrote:
> Patrick Mochel <[email protected]> wrote:
> >
> > > Eric, maybe we need to turn it off by hand at the right time rather than
> > > relying on driver model shutdown ordering?
> >
> > Interesting. This is yet more proof that system-level devices cannot be
> > treated as common, everyday devices. Sure, it's nice to see them show up
> > in sysfs with little overhead, and very nice not to have to work about
> > them during shutdown or power transitions. But there are just too many
> > special cases (like getting the ordering right ;) that you have to worry
> > about.
> >
> > So, what do we do with them?
>
> I'd say that as long as the shutdown routines are executed in reverse
> order of startup, then the core driver stuff has fulfilled its
> obligations.
>
> In this case we need to understand why the lockup is happening - what
> code is requiring 8259 services after the thing has been turned off?
> Could be that the bug lies there.
>
> Felipe, please send your .config. (again - in fact you may as well do
> cp .config ~/.signature)

Config attached...
Don't understand what do you mean with "cp .config ~/.signature" :-?


Attachments:
config-2.5.69-mm5 (18.74 kB)

2003-05-15 17:21:31

by Patrick Mochel

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch


> Pat what do you say to some late shutdown callbacks? I'll drop you a
> patch sometime tommorrow.

Bah. It would work, but it's a hack. We'll get caught in a game similar to
the leap-frogging initcalls. In fact, we could just get really twisted and
define various levels of exitcalls. [ Or, do them implicitly with some
linkser-section-fu by calling the modules' exit functions in the reverse
order in which they were initialized, but that's another story. ]

I just think that system level devices need to be treated specially in
every case. They just don't work as normal devices because of the ordering
issue. We can keep a separate list of them and deal with them explicitly
after regular devices. It's not that bad of a change, but will take a few
days, unless someone wants to take a stab at it..


-pat


2003-05-15 17:22:42

by Patrick Mochel

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch


> OK, I've changed "#undef DEBUG" to "#define DEBUG" in
> drivers/base/power.c, but during shutdown, I can see no extra debug
> messages. What am I doing wrong?

Nothing. No one has shutdown methods in their drivers. This patch should
give you a little more info. For me, it says the i8259 is getting shutdown
very early in the process, which surely can't be good..


-pat

===== drivers/base/power.c 1.18 vs edited =====
--- 1.18/drivers/base/power.c Mon Jan 6 09:56:05 2003
+++ edited/drivers/base/power.c Thu May 15 10:30:25 2003
@@ -8,7 +8,7 @@
*
*/

-#undef DEBUG
+#define DEBUG

#include <linux/device.h>
#include <linux/module.h>
@@ -88,10 +88,12 @@
down_write(&devices_subsys.rwsem);
list_for_each(entry,&devices_subsys.kset.list) {
struct device * dev = to_dev(entry);
+ pr_debug("shutting down %s: ",dev->name);
if (dev->driver && dev->driver->shutdown) {
- pr_debug("shutting down %s\n",dev->name);
+ pr_debug("Ok\n");
dev->driver->shutdown(dev);
- }
+ } else
+ pr_debug("Ignored.\n");
}
up_write(&devices_subsys.rwsem);
}

2003-05-16 19:52:40

by Pavel Machek

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

Hi!

> > > Hi again, Andrew,
> > >
> > > Besides the "make_KOBJ_NAME-match_BUS_ID_SIZE.patch" causing "pccard"
> > > oopses, I've also found that, with 2.5.69-mm5 compiled with ACPI
> > > support, my laptop is unable to power off. The kernel invokes
> > > "acpi_power_off" and stays there forever.
> > >
> > > I've found that reverting the "i8259-shutdown.patch" fixes the problem
> > > and my laptop is able to shutdown properly (init 0) when using ACPI.
> > >
> > > A hardware bug? A kernel bug?
> >
> > And thanks again, again.
> >
> > That's the below patch. It looks pretty innocuous. I'd be assuming that
> > there's something in the shutdown sequence which needs 8259 functionality
> > after the thing has been turned off.
> >
> > This could well depend upon .config contents and linkage order.
> >
> > Eric, maybe we need to turn it off by hand at the right time rather than
> > relying on driver model shutdown ordering?
>
> Interesting. This is yet more proof that system-level devices cannot be
> treated as common, everyday devices. Sure, it's nice to see them show up
> in sysfs with little overhead, and very nice not to have to work about
> them during shutdown or power transitions. But there are just too many
> special cases (like getting the ordering right ;) that you have to worry
> about.
>
> So, what do we do with them?

I guess shutdown needs to be treated like suspend, and needs to have
"level". There should be no shutdown, you should do suspend(5, ) and
go through all levels properly.
Pavel

--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

2003-05-17 19:22:37

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Thu, 15 May 2003, Felipe Alfaro Solana wrote:

> > In this case we need to understand why the lockup is happening - what
> > code is requiring 8259 services after the thing has been turned off?
> > Could be that the bug lies there.
> >
> > Felipe, please send your .config. (again - in fact you may as well do
> > cp .config ~/.signature)
>
> Config attached...
> Don't understand what do you mean with "cp .config ~/.signature" :-?
>

Unable to reproduce, appears to be machine specific, 1 laptop and 2 test
systems both managed to power off with APM or ACPI. Also tried with
Felipe's config

Zwane
--
function.linuxpower.ca

2003-05-17 19:34:26

by Felipe Alfaro Solana

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Sat, 2003-05-17 at 21:25, Zwane Mwaikambo wrote:
> On Thu, 15 May 2003, Felipe Alfaro Solana wrote:
>
> > > In this case we need to understand why the lockup is happening - what
> > > code is requiring 8259 services after the thing has been turned off?
> > > Could be that the bug lies there.
> > >
> > > Felipe, please send your .config. (again - in fact you may as well do
> > > cp .config ~/.signature)
> >
> > Config attached...
> > Don't understand what do you mean with "cp .config ~/.signature" :-?
> >
> Unable to reproduce, appears to be machine specific, 1 laptop and 2 test
> systems both managed to power off with APM or ACPI. Also tried with
> Felipe's config

The machine is a NEC (Packard Bell) Chrom@. Could an lspci be of
interest?

2003-05-17 20:34:55

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Sat, 17 May 2003, Felipe Alfaro Solana wrote:

> The machine is a NEC (Packard Bell) Chrom@. Could an lspci be of
> interest?

Hmm perhaps /var/log/dmesg, also can you try this ugly patch?

Index: linux-2.5.69-mm6/drivers/acpi/hardware/hwsleep.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.69/drivers/acpi/hardware/hwsleep.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 hwsleep.c
--- linux-2.5.69-mm6/drivers/acpi/hardware/hwsleep.c 6 May 2003 12:20:11 -0000 1.1.1.1
+++ linux-2.5.69-mm6/drivers/acpi/hardware/hwsleep.c 17 May 2003 20:33:20 -0000
@@ -306,7 +306,7 @@ acpi_enter_sleep_state (
* still read the right value. Ideally, this entire block would go
* away entirely.
*/
- acpi_os_stall (10000000);
+ /* acpi_os_stall (10000000); */

status = acpi_hw_register_write (ACPI_MTX_DO_NOT_LOCK, ACPI_REGISTER_PM1_CONTROL,
sleep_enable_reg_info->access_bit_mask);
Index: linux-2.5.69-mm6/drivers/acpi/sleep/poweroff.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.69/drivers/acpi/sleep/poweroff.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 poweroff.c
--- linux-2.5.69-mm6/drivers/acpi/sleep/poweroff.c 6 May 2003 12:20:12 -0000 1.1.1.1
+++ linux-2.5.69-mm6/drivers/acpi/sleep/poweroff.c 17 May 2003 20:35:11 -0000
@@ -15,6 +15,7 @@ acpi_power_off (void)
printk("%s called\n",__FUNCTION__);
acpi_enter_sleep_state_prep(ACPI_STATE_S5);
ACPI_DISABLE_IRQS();
+ printk("%s:%d\n", __FUNCTION__, __LINE__);
acpi_enter_sleep_state(ACPI_STATE_S5);
}

--
function.linuxpower.ca

2003-05-18 19:29:22

by Felipe Alfaro Solana

[permalink] [raw]
Subject: Re: 2.5.69-mm5: reverting i8259-shutdown.patch

On Sat, 2003-05-17 at 22:35, Zwane Mwaikambo wrote:
> On Sat, 17 May 2003, Felipe Alfaro Solana wrote:
>
> > The machine is a NEC (Packard Bell) Chrom@. Could an lspci be of
> > interest?
>
> Hmm perhaps /var/log/dmesg, also can you try this ugly patch?
>
> Index: linux-2.5.69-mm6/drivers/acpi/hardware/hwsleep.c
> ===================================================================
> RCS file: /build/cvsroot/linux-2.5.69/drivers/acpi/hardware/hwsleep.c,v
> retrieving revision 1.1.1.1
> diff -u -p -B -r1.1.1.1 hwsleep.c
> --- linux-2.5.69-mm6/drivers/acpi/hardware/hwsleep.c 6 May 2003 12:20:11 -0000 1.1.1.1
> +++ linux-2.5.69-mm6/drivers/acpi/hardware/hwsleep.c 17 May 2003 20:33:20 -0000
> @@ -306,7 +306,7 @@ acpi_enter_sleep_state (
> * still read the right value. Ideally, this entire block would go
> * away entirely.
> */
> - acpi_os_stall (10000000);
> + /* acpi_os_stall (10000000); */
>
> status = acpi_hw_register_write (ACPI_MTX_DO_NOT_LOCK, ACPI_REGISTER_PM1_CONTROL,
> sleep_enable_reg_info->access_bit_mask);
> Index: linux-2.5.69-mm6/drivers/acpi/sleep/poweroff.c
> ===================================================================
> RCS file: /build/cvsroot/linux-2.5.69/drivers/acpi/sleep/poweroff.c,v
> retrieving revision 1.1.1.1
> diff -u -p -B -r1.1.1.1 poweroff.c
> --- linux-2.5.69-mm6/drivers/acpi/sleep/poweroff.c 6 May 2003 12:20:12 -0000 1.1.1.1
> +++ linux-2.5.69-mm6/drivers/acpi/sleep/poweroff.c 17 May 2003 20:35:11 -0000
> @@ -15,6 +15,7 @@ acpi_power_off (void)
> printk("%s called\n",__FUNCTION__);
> acpi_enter_sleep_state_prep(ACPI_STATE_S5);
> ACPI_DISABLE_IRQS();
> + printk("%s:%d\n", __FUNCTION__, __LINE__);
> acpi_enter_sleep_state(ACPI_STATE_S5);
> }
>
With the previous patch applied, I can't make the kernel to printk
anything interesting after "Halting the computer". Now, I can't even see
the "acpi_power_off" message I was seen before.

Attached is "/var/log/dmesg" of a 2.5.69-mm6 kernel.
Thanks!


Attachments:
dmesg (7.14 kB)