Re: [PATCH]: ACPI : Set 32bit and 64bit waking vector in FCAS table

[Zhang Rui <rui.zhang@intel.com>]

On Thu, 2008-09-04 at 10:10 +0200, Rafael J. Wysocki wrote:
> On Thursday, 4 of September 2008, Zhao Yakui wrote:
> > Subject: ACPI : Set 32bit and 64bit waking vector in FCAS table
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> >     On some laptops only the 64bit waking vecotr is set for ACPI 2.0 FACS.
> > But when the system is resumed, BIOS will transfer control directly to 
> > 32bit waking vector. In such case the system can't be resumed correctly.
> >     Maybe it will be more appropriate that both the 32bit and 64bit waking 
> > vector will be set for the ACPI 2.0 FACS when the system enters the S3 
> > sleeping state.
> > 
> >  http://bugzilla.kernel.org/show_bug.cgi?id=11368
> 
> Well, the spec (2.0c) says we should use facs->firmware_waking_vector only
> if facs->xfirmware_waking_vector is zero, so I'm afraid this change may cause
> regressions to happen.
> 
> Moreover, both 1.0b and 2.0c say that facs->length should be at least 64 bytes,
> so the (facs->length >= 32) check is actually redundant (unless there is a
> quirk setting this value below 32 for some broken systems I'm not aware of).
> 
> So, the question is what happens if we ignore the facs->length check on systems
> this patch is supposed to fix.
this should have no impact.
facs->length is also 64 bytes.
without the patch, xfirmware_waking_vector is set.
with this patch applied, both xfirmware and firmware_waking_vector are
set.
so it seems that the BIOS sets facs->xfirmware_waking_vector during
POST, but uses facs->firmware_waking_vector to get back during resume.

thanks,
rui

>   If they _still_ require
> facs->firmware_waking_vector to be set even if
> facs->xfirmware_waking_vector is non-zero, I'd prefer to add quirks for them
> instead of affecting everybody else.
> 
> > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/hardware/hwsleep.c |   14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> > +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> > @@ -80,14 +80,14 @@ acpi_set_firmware_waking_vector(acpi_phy
> >  
> >  	/* Set the vector */
> >  
> > -	if ((facs->length < 32) || (!(facs->xfirmware_waking_vector))) {
> > -		/*
> > -		 * ACPI 1.0 FACS or short table or optional X_ field is zero
> > -		 */
> > -		facs->firmware_waking_vector = (u32) physical_address;
> > -	} else {
> > +	/* 32 Bit wakeing vector is always set */
> > +	facs->firmware_waking_vector = (u32) physical_address;
> > +
> > +	if (facs->length >= 32) {
> >  		/*
> > -		 * ACPI 2.0 FACS with valid X_ field
> > +		 * ACPI 2.0 FACS with the valid X_filed. Its length
> > +		 * will be more than 32 bytes. In such case the 64 bit
> > +		 * waking vector is also set.
> >  		 */
> >  		facs->xfirmware_waking_vector = physical_address;
> >  	}
> > 
> > 
> > --
> > 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
> > 
> > 
> 
> 
> --
> 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

--
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