| Message ID | 1242192968.14626.40.camel@minggr.sh.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wednesday 13 May 2009, Lin Ming wrote: > > > From: Bob Copeland <me@bobcopeland.com> > > Date: Wed, May 13, 2009 at 12:05 PM > > Subject: [BISECTED] Was Re: acpi/suspend - irq 9: nobody cared > > To: Bjorn Helgaas <bjorn.helgaas@hp.com> > > Cc: yakui_zhao <yakui.zhao@intel.com>, "linux-kernel@vger.kernel.org" > > <linux-kernel@vger.kernel.org>, "linux-acpi@vger.kernel.org" > > <linux-acpi@vger.kernel.org>, "Rafael J. Wysocki" <rjw@sisk.pl>, > > robert.moore@intel.com > > > > > > On Tue, May 12, 2009 at 01:01:06PM -0600, Bjorn Helgaas wrote: > > > Even if you can't bisect this, can you please open a bugzilla at > > > http://bugzilla.kernel.org? > > > > Done: http://bugzilla.kernel.org/show_bug.cgi?id=13289 > > > > I have bisected it. I don't know if it's related, but I have a > > "MacBook 1,1" which in sleep.c has a quirk related to SCI_EN, and I > > see that this patch changed ACPI_PM1_CONTROL_PRESERVED_BITS to include > > SCI_EN. I have verified that reverting the patch on top of latest -rc > > prevents irq 9 from being disabled. > > >From ACPI Spec 3.0a 4.7.3.2.1, > > "SCI_EN: Selects the power management event to be either an SCI or SMI > interrupt for the following events. When this bit is set, then power > management events will generate an SCI interrupt. When this bit is reset > power management events will generate an SMI interrupt. It is the > responsibility of the hardware to set or reset this bit. OSPM always > preserves this bit position." > > So we preserve SCI_EN bit per spec. In fact we do set SCI_EN directly in drivers/acpi/sleep.c in a couple of places exactly because the hardware is _known_ to wrong things here. So, we don't really preserve it, _exactly_ because at least some pieces of hardware out there don't follow the spec. Including Macs, IIRC. Bob, can you please verify if the Lin Ming's patch helps? Rafael > Anyway, would you please try below patch to see if the bug is caused by > SCI_EN bit preserved? > > diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h > index 772ee5c..3d082bb 100644 > --- a/drivers/acpi/acpica/aclocal.h > +++ b/drivers/acpi/acpica/aclocal.h > @@ -787,7 +787,7 @@ struct acpi_bit_register_info { > > /* For control registers, both ignored and reserved bits must be preserved */ > > -#define ACPI_PM1_CONTROL_IGNORED_BITS 0x0201 /* Bits 9, 0(SCI_EN) */ > +#define ACPI_PM1_CONTROL_IGNORED_BITS 0x0200 /* Bits 9 */ > #define ACPI_PM1_CONTROL_RESERVED_BITS 0xC1F8 /* Bits 14-15, 3-8 */ > #define ACPI_PM1_CONTROL_PRESERVED_BITS \ > (ACPI_PM1_CONTROL_IGNORED_BITS | ACPI_PM1_CONTROL_RESERVED_BITS) > > --- > Lin Ming > > > > > commit 20869dcfde204e1c21b642608d708d82472fee2b > > Author: Bob Moore <robert.moore@intel.com> > > Date: Fri Mar 13 09:10:46 2009 +0800 > > > > ACPICA: Preserve all PM control reserved and ignored bits > > > > As per the ACPI specification, preserve (read/modify/write) all > > bits that are defined as either reserved or ignored (PM control > > control registers only.) > > > > Signed-off-by: Bob Moore <robert.moore@intel.com> > > Signed-off-by: Lin Ming <ming.m.lin@intel.com> > > Signed-off-by: Len Brown <len.brown@intel.com> > > > > diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h > > index 18a8d96..f01e155 100644 > > --- a/drivers/acpi/acpica/aclocal.h > > +++ b/drivers/acpi/acpica/aclocal.h > > @@ -780,7 +780,15 @@ struct acpi_bit_register_info { > > * must be preserved. > > */ > > #define ACPI_PM1_STATUS_PRESERVED_BITS 0x0800 /* Bit 11 */ > > -#define ACPI_PM1_CONTROL_PRESERVED_BITS 0x0200 /* Bit 9 (whatever) */ > > + > > +/* For control registers, both ignored and reserved bits must be preserved */ > > + > > +#define ACPI_PM1_CONTROL_IGNORED_BITS 0x0201 /* Bits 9, 0(SCI_EN) */ > > +#define ACPI_PM1_CONTROL_RESERVED_BITS 0xC1F8 /* Bits 14-15, 3-8 */ > > +#define ACPI_PM1_CONTROL_PRESERVED_BITS \ > > + (ACPI_PM1_CONTROL_IGNORED_BITS | ACPI_PM1_CONTROL_RESERVED_BITS) > > + > > +#define ACPI_PM2_CONTROL_PRESERVED_BITS 0xFFFFFFFE /* All > > except bit 0 */ > > > > /* > > * Register IDs > > diff --git a/drivers/acpi/acpica/hwregs.c b/drivers/acpi/acpica/hwregs.c > > index fd7abe2..6117362 100644 > > --- a/drivers/acpi/acpica/hwregs.c > > +++ b/drivers/acpi/acpica/hwregs.c > > @@ -328,6 +328,21 @@ acpi_status acpi_hw_register_write(u32 > > register_id, u32 value) > > > > case ACPI_REGISTER_PM2_CONTROL: /* 8-bit access */ > > > > + /* > > + * For control registers, all reserved bits must be preserved, > > + * as per the ACPI spec. > > + */ > > + status = > > + acpi_read(&read_value, &acpi_gbl_FADT.xpm2_control_block); > > + if (ACPI_FAILURE(status)) { > > + goto exit; > > + } > > + > > + /* Insert the bits to be preserved */ > > + > > + ACPI_INSERT_BITS(value, ACPI_PM2_CONTROL_PRESERVED_BITS, > > + read_value); > > + > > status = acpi_write(value, &acpi_gbl_FADT.xpm2_control_block); > > break; > > > > > > > > -- > > Bob Copeland %% www.bobcopeland.com > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > >
2009/5/13 Rafael J. Wysocki <rjw@sisk.pl>: >> So we preserve SCI_EN bit per spec. > > In fact we do set SCI_EN directly in drivers/acpi/sleep.c in a couple of > places exactly because the hardware is _known_ to wrong things here. Â So, we > don't really preserve it, _exactly_ because at least some pieces of hardware > out there don't follow the spec. Â Including Macs, IIRC. > > Bob, can you please verify if the Lin Ming's patch helps? Yes, my laptop is one of those pieces of hardware. I can confirm that Lin Ming's patch works on my system.
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h index 772ee5c..3d082bb 100644 --- a/drivers/acpi/acpica/aclocal.h +++ b/drivers/acpi/acpica/aclocal.h @@ -787,7 +787,7 @@ struct acpi_bit_register_info { /* For control registers, both ignored and reserved bits must be preserved */ -#define ACPI_PM1_CONTROL_IGNORED_BITS 0x0201 /* Bits 9, 0(SCI_EN) */ +#define ACPI_PM1_CONTROL_IGNORED_BITS 0x0200 /* Bits 9 */ #define ACPI_PM1_CONTROL_RESERVED_BITS 0xC1F8 /* Bits 14-15, 3-8 */ #define ACPI_PM1_CONTROL_PRESERVED_BITS \ (ACPI_PM1_CONTROL_IGNORED_BITS | ACPI_PM1_CONTROL_RESERVED_BITS)