[PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Shaohua Li]

On Asus A6VC laptop, PCI config space read of slot 01:01.* after ACPI
_PTS is exectued always returns 0xFFFFFFFF.  Force _PTS is executed
after suspending devices

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
index d2f71a5..ebc20a5 100644
--- a/drivers/acpi/sleep/main.c
+++ b/drivers/acpi/sleep/main.c
@@ -272,12 +272,32 @@ static int __init init_ints_after_s1(const struct dmi_system_id *d)
 	return 0;
 }
 
+/*
+ * On Asus A6VC laptop, PCI config space read of slot 01:01.* after ACPI _PTS
+ * is exectued always returns 0xFFFFFFFF.  Force _PTS is executed after
+ * suspending devices
+ */
+static int __init asus_suspend_order(const struct dmi_system_id *d)
+{
+	printk(KERN_WARNING PREFIX "%s detected, "
+		"acpi_new_pts_ordering is force enabled\n", d->ident);
+	new_pts_ordering = true;
+	return 0;
+}
+
 static struct dmi_system_id __initdata acpisleep_dmi_table[] = {
 	{
 	 .callback = init_ints_after_s1,
 	 .ident = "Toshiba Satellite 4030cdt",
 	 .matches = {DMI_MATCH(DMI_PRODUCT_NAME, "S4030CDT/4.3"),},
 	 },
+	{
+	 .callback = asus_suspend_order,
+	 .ident = "Asus A6VC",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer Inc."),
+		DMI_MATCH(DMI_PRODUCT_NAME, "A6VC"),},
+	 },
 	{},
 };
 #endif /* CONFIG_SUSPEND */

Re: [PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Carlos Corbacho]

> +static int __init asus_suspend_order(const struct dmi_system_id *d)
> +{
> +	printk(KERN_WARNING PREFIX "%s detected, "
> +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> +	new_pts_ordering = true;
> +	return 0;

Given all you're doing here is just changing the ordering,
'asus_suspend_order' doesn't strike me as the right name for this function
since:

1) This clearly isn't the case for all Asus boards.

2) Perhaps something like 'new_suspend_order' would be a better name for
this function (e.g. if others want to use DMI to do this for non-Asus
boards).

-Carlos
-- 
E-Mail: carlos@strangeworlds.co.uk
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

Re: [PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Rafael J. Wysocki]

On Tuesday, 25 of March 2008, Carlos Corbacho wrote:
> > +static int __init asus_suspend_order(const struct dmi_system_id *d)
> > +{
> > +	printk(KERN_WARNING PREFIX "%s detected, "
> > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > +	new_pts_ordering = true;
> > +	return 0;
> 
> Given all you're doing here is just changing the ordering,
> 'asus_suspend_order' doesn't strike me as the right name for this function
> since:
> 
> 1) This clearly isn't the case for all Asus boards.
> 
> 2) Perhaps something like 'new_suspend_order' would be a better name for
> this function (e.g. if others want to use DMI to do this for non-Asus
> boards).

Agreed.

Thanks,
Rafael

Re: [PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Shaohua Li]

On Tue, 2008-03-25 at 09:24 +0000, Carlos Corbacho wrote:
> > +static int __init asus_suspend_order(const struct dmi_system_id *d)
> > +{
> > +	printk(KERN_WARNING PREFIX "%s detected, "
> > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > +	new_pts_ordering = true;
> > +	return 0;
> 
> Given all you're doing here is just changing the ordering,
> 'asus_suspend_order' doesn't strike me as the right name for this function
> since:
> 
> 1) This clearly isn't the case for all Asus boards.
> 
> 2) Perhaps something like 'new_suspend_order' would be a better name for
> this function (e.g. if others want to use DMI to do this for non-Asus
> boards).
Makes sense. I changed it.

On Asus A6VC laptop, PCI config space read of slot 01:01.* after ACPI
_PTS is exectued always returns 0xFFFFFFFF.  Force _PTS is executed
after suspending devices

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
index d2f71a5..4dded01 100644
--- a/drivers/acpi/sleep/main.c
+++ b/drivers/acpi/sleep/main.c
@@ -272,12 +272,31 @@ static int __init init_ints_after_s1(const struct dmi_system_id *d)
 	return 0;
 }
 
+/*
+ * Laptop needs _PTS is executed after suspending devices. 
+ * Asus A6VC - PCI config read of slot 01:01.* after _PTS returns 0xFFFFFFFF.
+ */
+static int __init new_suspend_order(const struct dmi_system_id *d)
+{
+	printk(KERN_WARNING PREFIX "%s detected, "
+		"acpi_new_pts_ordering is force enabled\n", d->ident);
+	new_pts_ordering = true;
+	return 0;
+}
+
 static struct dmi_system_id __initdata acpisleep_dmi_table[] = {
 	{
 	 .callback = init_ints_after_s1,
 	 .ident = "Toshiba Satellite 4030cdt",
 	 .matches = {DMI_MATCH(DMI_PRODUCT_NAME, "S4030CDT/4.3"),},
 	 },
+	{
+	 .callback = new_suspend_order,
+	 .ident = "Asus A6VC",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer Inc."),
+		DMI_MATCH(DMI_PRODUCT_NAME, "A6VC"),},
+	 },
 	{},
 };
 #endif /* CONFIG_SUSPEND */

Re: [PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Rafael J. Wysocki]

On Wednesday, 26 of March 2008, Shaohua Li wrote:
> 
> On Tue, 2008-03-25 at 09:24 +0000, Carlos Corbacho wrote:
> > > +static int __init asus_suspend_order(const struct dmi_system_id *d)
> > > +{
> > > +	printk(KERN_WARNING PREFIX "%s detected, "
> > > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > > +	new_pts_ordering = true;
> > > +	return 0;
> > 
> > Given all you're doing here is just changing the ordering,
> > 'asus_suspend_order' doesn't strike me as the right name for this function
> > since:
> > 
> > 1) This clearly isn't the case for all Asus boards.
> > 
> > 2) Perhaps something like 'new_suspend_order' would be a better name for
> > this function (e.g. if others want to use DMI to do this for non-Asus
> > boards).
> Makes sense. I changed it.
> 
> On Asus A6VC laptop, PCI config space read of slot 01:01.* after ACPI
> _PTS is exectued always returns 0xFFFFFFFF.  Force _PTS is executed
> after suspending devices
> 
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>

I don't think we can do anything smarter about it.

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>


> diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
> index d2f71a5..4dded01 100644
> --- a/drivers/acpi/sleep/main.c
> +++ b/drivers/acpi/sleep/main.c
> @@ -272,12 +272,31 @@ static int __init init_ints_after_s1(const struct dmi_system_id *d)
>  	return 0;
>  }
>  
> +/*
> + * Laptop needs _PTS is executed after suspending devices. 
> + * Asus A6VC - PCI config read of slot 01:01.* after _PTS returns 0xFFFFFFFF.
> + */
> +static int __init new_suspend_order(const struct dmi_system_id *d)
> +{
> +	printk(KERN_WARNING PREFIX "%s detected, "
> +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> +	new_pts_ordering = true;
> +	return 0;
> +}
> +
>  static struct dmi_system_id __initdata acpisleep_dmi_table[] = {
>  	{
>  	 .callback = init_ints_after_s1,
>  	 .ident = "Toshiba Satellite 4030cdt",
>  	 .matches = {DMI_MATCH(DMI_PRODUCT_NAME, "S4030CDT/4.3"),},
>  	 },
> +	{
> +	 .callback = new_suspend_order,
> +	 .ident = "Asus A6VC",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer Inc."),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "A6VC"),},
> +	 },
>  	{},
>  };
>  #endif /* CONFIG_SUSPEND */
> 
> 
> 
> 



-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Pavel Machek]

On Wed 2008-03-26 22:29:37, Rafael J. Wysocki wrote:
> On Wednesday, 26 of March 2008, Shaohua Li wrote:
> > 
> > On Tue, 2008-03-25 at 09:24 +0000, Carlos Corbacho wrote:
> > > > +static int __init asus_suspend_order(const struct dmi_system_id *d)
> > > > +{
> > > > +	printk(KERN_WARNING PREFIX "%s detected, "
> > > > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > > > +	new_pts_ordering = true;
> > > > +	return 0;
> > > 
> > > Given all you're doing here is just changing the ordering,
> > > 'asus_suspend_order' doesn't strike me as the right name for this function
> > > since:
> > > 
> > > 1) This clearly isn't the case for all Asus boards.
> > > 
> > > 2) Perhaps something like 'new_suspend_order' would be a better name for
> > > this function (e.g. if others want to use DMI to do this for non-Asus
> > > boards).
> > Makes sense. I changed it.
> > 
> > On Asus A6VC laptop, PCI config space read of slot 01:01.* after ACPI
> > _PTS is exectued always returns 0xFFFFFFFF.  Force _PTS is executed
> > after suspending devices
> > 
> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> 
> I don't think we can do anything smarter about it.
> 
> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

This will clash with your patch to remove the option...

> > index d2f71a5..4dded01 100644
> > --- a/drivers/acpi/sleep/main.c
> > +++ b/drivers/acpi/sleep/main.c
> > @@ -272,12 +272,31 @@ static int __init init_ints_after_s1(const struct dmi_system_id *d)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * Laptop needs _PTS is executed after suspending devices. 
> > + * Asus A6VC - PCI config read of slot 01:01.* after _PTS returns 0xFFFFFFFF.
> > + */
> > +static int __init new_suspend_order(const struct dmi_system_id *d)
> > +{
> > +	printk(KERN_WARNING PREFIX "%s detected, "
> > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > +	new_pts_ordering = true;
> > +	return 0;
> > +}
> > +
> >  static struct dmi_system_id __initdata acpisleep_dmi_table[] = {
> >  	{
> >  	 .callback = init_ints_after_s1,
> >  	 .ident = "Toshiba Satellite 4030cdt",
> >  	 .matches = {DMI_MATCH(DMI_PRODUCT_NAME, "S4030CDT/4.3"),},
> >  	 },
> > +	{
> > +	 .callback = new_suspend_order,
> > +	 .ident = "Asus A6VC",
> > +	 .matches = {
> > +		DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer Inc."),
> > +		DMI_MATCH(DMI_PRODUCT_NAME, "A6VC"),},
> > +	 },
> >  	{},
> >  };
> >  #endif /* CONFIG_SUSPEND */
> > 
> > 
> > 
> > 
> 
> 
> 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Rafael J. Wysocki]

On Sunday, 30 of March 2008, Pavel Machek wrote:
> On Wed 2008-03-26 22:29:37, Rafael J. Wysocki wrote:
> > On Wednesday, 26 of March 2008, Shaohua Li wrote:
> > > 
> > > On Tue, 2008-03-25 at 09:24 +0000, Carlos Corbacho wrote:
> > > > > +static int __init asus_suspend_order(const struct dmi_system_id *d)
> > > > > +{
> > > > > +	printk(KERN_WARNING PREFIX "%s detected, "
> > > > > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > > > > +	new_pts_ordering = true;
> > > > > +	return 0;
> > > > 
> > > > Given all you're doing here is just changing the ordering,
> > > > 'asus_suspend_order' doesn't strike me as the right name for this function
> > > > since:
> > > > 
> > > > 1) This clearly isn't the case for all Asus boards.
> > > > 
> > > > 2) Perhaps something like 'new_suspend_order' would be a better name for
> > > > this function (e.g. if others want to use DMI to do this for non-Asus
> > > > boards).
> > > Makes sense. I changed it.
> > > 
> > > On Asus A6VC laptop, PCI config space read of slot 01:01.* after ACPI
> > > _PTS is exectued always returns 0xFFFFFFFF.  Force _PTS is executed
> > > after suspending devices
> > > 
> > > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > 
> > I don't think we can do anything smarter about it.
> > 
> > Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> This will clash with your patch to remove the option...

Well, it won't be needed at all if my patch is applied. :-)

The $subject patch was posted earlier than mine and the second system broken
by the ordering change (the MacBook) was reported in the meantime.  That's
why I decided to post that patch (please also see
http://bugzilla.kernel.org/show_bug.cgi?id=10340).

Thanks,
Rafael


> 
> > > index d2f71a5..4dded01 100644
> > > --- a/drivers/acpi/sleep/main.c
> > > +++ b/drivers/acpi/sleep/main.c
> > > @@ -272,12 +272,31 @@ static int __init init_ints_after_s1(const struct dmi_system_id *d)
> > >  	return 0;
> > >  }
> > >  
> > > +/*
> > > + * Laptop needs _PTS is executed after suspending devices. 
> > > + * Asus A6VC - PCI config read of slot 01:01.* after _PTS returns 0xFFFFFFFF.
> > > + */
> > > +static int __init new_suspend_order(const struct dmi_system_id *d)
> > > +{
> > > +	printk(KERN_WARNING PREFIX "%s detected, "
> > > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > > +	new_pts_ordering = true;
> > > +	return 0;
> > > +}
> > > +
> > >  static struct dmi_system_id __initdata acpisleep_dmi_table[] = {
> > >  	{
> > >  	 .callback = init_ints_after_s1,
> > >  	 .ident = "Toshiba Satellite 4030cdt",
> > >  	 .matches = {DMI_MATCH(DMI_PRODUCT_NAME, "S4030CDT/4.3"),},
> > >  	 },
> > > +	{
> > > +	 .callback = new_suspend_order,
> > > +	 .ident = "Asus A6VC",
> > > +	 .matches = {
> > > +		DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer Inc."),
> > > +		DMI_MATCH(DMI_PRODUCT_NAME, "A6VC"),},
> > > +	 },
> > >  	{},
> > >  };
> > >  #endif /* CONFIG_SUSPEND */
> > > 
> > > 
> > > 
> > > 
> > 
> > 
> > 
> 



-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Len Brown]

On Tuesday 25 March 2008, Shaohua Li wrote:
> 
> On Tue, 2008-03-25 at 09:24 +0000, Carlos Corbacho wrote:
> > > +static int __init asus_suspend_order(const struct dmi_system_id *d)
> > > +{
> > > +	printk(KERN_WARNING PREFIX "%s detected, "
> > > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > > +	new_pts_ordering = true;
> > > +	return 0;
> > 
> > Given all you're doing here is just changing the ordering,
> > 'asus_suspend_order' doesn't strike me as the right name for this function
> > since:
> > 
> > 1) This clearly isn't the case for all Asus boards.
> > 
> > 2) Perhaps something like 'new_suspend_order' would be a better name for
> > this function (e.g. if others want to use DMI to do this for non-Asus
> > boards).
> Makes sense. I changed it.

There doesn't appear to be anyting particularly "new" about the ACPI implementation on this laptop

[000h 000  4]                    Signature : "FACP"    /* Fixed ACPI Description Table */
[004h 004  4]                 Table Length : 00000084
[008h 008  1]                     Revision : 02

FADT revision 2 is from _before_ ACPI 2.0.

(However, MCFG, which is present, is reserved starting at ACPI 3.0)

-Len


> On Asus A6VC laptop, PCI config space read of slot 01:01.* after ACPI
> _PTS is exectued always returns 0xFFFFFFFF.  Force _PTS is executed
> after suspending devices
> 
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> 
> diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
> index d2f71a5..4dded01 100644
> --- a/drivers/acpi/sleep/main.c
> +++ b/drivers/acpi/sleep/main.c
> @@ -272,12 +272,31 @@ static int __init init_ints_after_s1(const struct dmi_system_id *d)
>  	return 0;
>  }
>  
> +/*
> + * Laptop needs _PTS is executed after suspending devices. 
> + * Asus A6VC - PCI config read of slot 01:01.* after _PTS returns 0xFFFFFFFF.
> + */
> +static int __init new_suspend_order(const struct dmi_system_id *d)
> +{
> +	printk(KERN_WARNING PREFIX "%s detected, "
> +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> +	new_pts_ordering = true;
> +	return 0;
> +}
> +
>  static struct dmi_system_id __initdata acpisleep_dmi_table[] = {
>  	{
>  	 .callback = init_ints_after_s1,
>  	 .ident = "Toshiba Satellite 4030cdt",
>  	 .matches = {DMI_MATCH(DMI_PRODUCT_NAME, "S4030CDT/4.3"),},
>  	 },
> +	{
> +	 .callback = new_suspend_order,
> +	 .ident = "Asus A6VC",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer Inc."),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "A6VC"),},
> +	 },
>  	{},
>  };
>  #endif /* CONFIG_SUSPEND */
> 
> 
> --
> 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
> 

Re: [PATCH] Force acpi_new_pts_ordering for ASUS A6VC laptop

[Rafael J. Wysocki]

On Friday, 28 of March 2008, Len Brown wrote:
> On Tuesday 25 March 2008, Shaohua Li wrote:
> > 
> > On Tue, 2008-03-25 at 09:24 +0000, Carlos Corbacho wrote:
> > > > +static int __init asus_suspend_order(const struct dmi_system_id *d)
> > > > +{
> > > > +	printk(KERN_WARNING PREFIX "%s detected, "
> > > > +		"acpi_new_pts_ordering is force enabled\n", d->ident);
> > > > +	new_pts_ordering = true;
> > > > +	return 0;
> > > 
> > > Given all you're doing here is just changing the ordering,
> > > 'asus_suspend_order' doesn't strike me as the right name for this function
> > > since:
> > > 
> > > 1) This clearly isn't the case for all Asus boards.
> > > 
> > > 2) Perhaps something like 'new_suspend_order' would be a better name for
> > > this function (e.g. if others want to use DMI to do this for non-Asus
> > > boards).
> > Makes sense. I changed it.
> 
> There doesn't appear to be anyting particularly "new" about the ACPI implementation on this laptop
> 
> [000h 000  4]                    Signature : "FACP"    /* Fixed ACPI Description Table */
> [004h 004  4]                 Table Length : 00000084
> [008h 008  1]                     Revision : 02
> 
> FADT revision 2 is from _before_ ACPI 2.0.

So in theory it should work with the suspend ordering we use by default right
now. :-)

Thanks,
Rafael