[patch 1/11]avoid check _STA method

[patch 1/11]avoid check _STA method

[Shaohua Li]

In some BIOSes, every _STA method call will send a notification again,
this cause freeze. And in some BIOSes, it appears _STA should be called
after _DCK. This tries to avoid calls _STA, and still keep the device
present check.
http://bugzilla.kernel.org/show_bug.cgi?id=10431

Signed-off-by: Shaohua Li <shaohua.li@intel.com> 
---
 drivers/acpi/dock.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: linux/drivers/acpi/dock.c
===================================================================
--- linux.orig/drivers/acpi/dock.c	2008-08-27 10:19:17.000000000 +0800
+++ linux/drivers/acpi/dock.c	2008-08-27 10:19:30.000000000 +0800
@@ -604,14 +604,16 @@ static int handle_eject_request(struct d
 static void dock_notify(acpi_handle handle, u32 event, void *data)
 {
 	struct dock_station *ds = data;
+	struct acpi_device *tmp;
 
 	switch (event) {
 	case ACPI_NOTIFY_BUS_CHECK:
-		if (!dock_in_progress(ds) && dock_present(ds)) {
+		if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle, &tmp)) {
 			begin_dock(ds);
 			dock(ds);
 			if (!dock_present(ds)) {
 				printk(KERN_ERR PREFIX "Unable to dock!\n");
+				complete_dock(ds);
 				break;
 			}
 			atomic_notifier_call_chain(&dock_notifier_list,


--
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 1/11]avoid check _STA method

[Andi Kleen]

On Wed, Aug 27, 2008 at 11:14:19AM +0800, Shaohua Li wrote:
> In some BIOSes, every _STA method call will send a notification again,
> this cause freeze. And in some BIOSes, it appears _STA should be called
> after _DCK. This tries to avoid calls _STA, and still keep the device
> present check.
> http://bugzilla.kernel.org/show_bug.cgi?id=10431

General comment before I review any code. Your mailer adds strange
characters to the comment log: (e.g. in this case before the "In"
and the "http"). My vim displays that as lots of &&&&&? Can you
remove that please?

-Andi
--
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 1/11]avoid check _STA method

[Li, Shaohua]

Let me check how to remove them, it appears I can't find them in vim.

Thanks,
Shaohua
>-----Original Message-----
>From: Andi Kleen [mailto:andi@firstfloor.org]
>Sent: Wednesday, August 27, 2008 3:53 PM
>To: Li, Shaohua
>Cc: linux acpi; Len Brown; Andi Kleen; Henrique de Moraes Holschuh; Holger
>Macht; mjg59; Tejun Heo
>Subject: Re: [patch 1/11]avoid check _STA method
>
>On Wed, Aug 27, 2008 at 11:14:19AM +0800, Shaohua Li wrote:
>> In some BIOSes, every _STA method call will send a notification again,
>> this cause freeze. And in some BIOSes, it appears _STA should be called
>> after _DCK. This tries to avoid calls _STA, and still keep the device
>> present check.
>> http://bugzilla.kernel.org/show_bug.cgi?id=10431
>
>General comment before I review any code. Your mailer adds strange
>characters to the comment log: (e.g. in this case before the "In"
>and the "http"). My vim displays that as lots of &&&&&? Can you
>remove that please?
>
>-Andi

Re: [patch 1/11]avoid check _STA method

[Andi Kleen]

On Wed, Aug 27, 2008 at 04:24:10PM +0800, Li, Shaohua wrote:
> Let me check how to remove them, it appears I can't find them in vim.

I hope it's not a fluke here, but I have a fairly ordinary postfix+mutt
setup and normally don't see weird characters in other mails.

-Andi

Re: [patch 1/11]avoid check _STA method

[Alan Jenkins]

Andi Kleen wrote:
> On Wed, Aug 27, 2008 at 04:24:10PM +0800, Li, Shaohua wrote:
>   
>> Let me check how to remove them, it appears I can't find them in vim.
>>     
>
> I hope it's not a fluke here, but I have a fairly ordinary postfix+mutt
> setup and normally don't see weird characters in other mails.
>   
I saw garbage characters too, in Thunderbird.  I didn't read the patch
but they were present in the email quoted above.

(these ones:)

"N�����r��y���b�X��ǧv�^�)޺{.n�+����{�i�b�{ay�\x1dʇڙ�,j\a��f���h���z�\x1e�w���"


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

[patch 1/11]avoid check _STA method

[Shaohua Li]

In some BIOSes, every _STA method call will send a notification again,
this cause freeze. And in some BIOSes, it appears _STA should be called
after _DCK. This tries to avoid calls _STA, and still keep the device
present check.
http://bugzilla.kernel.org/show_bug.cgi?id=10431

Signed-off-by: Shaohua Li <shaohua.li@intel.com> 
---
 drivers/acpi/dock.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux/drivers/acpi/dock.c
===================================================================
--- linux.orig/drivers/acpi/dock.c	2008-08-27 14:38:44.000000000 +0800
+++ linux/drivers/acpi/dock.c	2008-08-28 09:33:02.000000000 +0800
@@ -604,14 +604,17 @@ static int handle_eject_request(struct d
 static void dock_notify(acpi_handle handle, u32 event, void *data)
 {
 	struct dock_station *ds = data;
+	struct acpi_device *tmp;
 
 	switch (event) {
 	case ACPI_NOTIFY_BUS_CHECK:
-		if (!dock_in_progress(ds) && dock_present(ds)) {
+		if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle,
+		   &tmp)) {
 			begin_dock(ds);
 			dock(ds);
 			if (!dock_present(ds)) {
 				printk(KERN_ERR PREFIX "Unable to dock!\n");
+				complete_dock(ds);
 				break;
 			}
 			atomic_notifier_call_chain(&dock_notifier_list,

Re: [patch 1/11]avoid check _STA method

[Thomas Renninger]

Hi,

could this patchset eventually help here:
kacpi* eat a lot of cpu after s2disk
https://bugzilla.novell.com/show_bug.cgi?id=401740
This seem to happen on all recent Dells.

Shaohua Li, Andi: Can you keep me up to date about these patches, pls

If Andi thinks they are suitable for mainline, I'd like to
add them to our recent codebase.
Or is there a chance that these (or some critical ones) go into
.27-rcX?

Thanks,

      Thomas

On Thursday 28 August 2008 04:02:03 Shaohua Li wrote:
> In some BIOSes, every _STA method call will send a notification again,
> this cause freeze. And in some BIOSes, it appears _STA should be called
> after _DCK. This tries to avoid calls _STA, and still keep the device
> present check.
> http://bugzilla.kernel.org/show_bug.cgi?id=10431
>
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> ---
>  drivers/acpi/dock.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Index: linux/drivers/acpi/dock.c
> ===================================================================
> --- linux.orig/drivers/acpi/dock.c	2008-08-27 14:38:44.000000000 +0800
> +++ linux/drivers/acpi/dock.c	2008-08-28 09:33:02.000000000 +0800
> @@ -604,14 +604,17 @@ static int handle_eject_request(struct d
>  static void dock_notify(acpi_handle handle, u32 event, void *data)
>  {
>  	struct dock_station *ds = data;
> +	struct acpi_device *tmp;
>
>  	switch (event) {
>  	case ACPI_NOTIFY_BUS_CHECK:
> -		if (!dock_in_progress(ds) && dock_present(ds)) {
> +		if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle,
> +		   &tmp)) {
>  			begin_dock(ds);
>  			dock(ds);
>  			if (!dock_present(ds)) {
>  				printk(KERN_ERR PREFIX "Unable to dock!\n");
> +				complete_dock(ds);
>  				break;
>  			}
>  			atomic_notifier_call_chain(&dock_notifier_list,
>
>
> --
> 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 1/11]avoid check _STA method

[Holger Macht]

On Thu 28. Aug - 10:02:03, Shaohua Li wrote:
> In some BIOSes, every _STA method call will send a notification again,
> this cause freeze. And in some BIOSes, it appears _STA should be called
> after _DCK. This tries to avoid calls _STA, and still keep the device
> present check.
> http://bugzilla.kernel.org/show_bug.cgi?id=10431

Finally found some time to test this...

> 
> Signed-off-by: Shaohua Li <shaohua.li@intel.com> 
> ---
>  drivers/acpi/dock.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Index: linux/drivers/acpi/dock.c
> ===================================================================
> --- linux.orig/drivers/acpi/dock.c	2008-08-27 14:38:44.000000000 +0800
> +++ linux/drivers/acpi/dock.c	2008-08-28 09:33:02.000000000 +0800
> @@ -604,14 +604,17 @@ static int handle_eject_request(struct d
>  static void dock_notify(acpi_handle handle, u32 event, void *data)
>  {
>  	struct dock_station *ds = data;
> +	struct acpi_device *tmp;
>  
>  	switch (event) {
>  	case ACPI_NOTIFY_BUS_CHECK:
> -		if (!dock_in_progress(ds) && dock_present(ds)) {
> +		if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle,

On the Thinkpad X60, when booting undocked, the first time I dock,
acpi_bus_get_device() returns 0, so hotplug is not performed
properly. Second time (undock->dock), it works.

However, dock_present(ds) would return 1, so on this system, it works
without applying this patch.

Regards,
	Holger

> +		   &tmp)) {
>  			begin_dock(ds);
>  			dock(ds);
>  			if (!dock_present(ds)) {
>  				printk(KERN_ERR PREFIX "Unable to dock!\n");
> +				complete_dock(ds);
>  				break;
>  			}
>  			atomic_notifier_call_chain(&dock_notifier_list,
> 
> 

Re: [patch 1/11]avoid check _STA method

[Shaohua Li]

On Mon, 2008-09-22 at 18:57 +0800, Holger Macht wrote:
> On Thu 28. Aug - 10:02:03, Shaohua Li wrote:
> > In some BIOSes, every _STA method call will send a notification again,
> > this cause freeze. And in some BIOSes, it appears _STA should be called
> > after _DCK. This tries to avoid calls _STA, and still keep the device
> > present check.
> > http://bugzilla.kernel.org/show_bug.cgi?id=10431
> 
> Finally found some time to test this...
> 
> >
> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > ---
> >  drivers/acpi/dock.c |    5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > Index: linux/drivers/acpi/dock.c
> > ===================================================================
> > --- linux.orig/drivers/acpi/dock.c    2008-08-27 14:38:44.000000000 +0800
> > +++ linux/drivers/acpi/dock.c 2008-08-28 09:33:02.000000000 +0800
> > @@ -604,14 +604,17 @@ static int handle_eject_request(struct d
> >  static void dock_notify(acpi_handle handle, u32 event, void *data)
> >  {
> >       struct dock_station *ds = data;
> > +     struct acpi_device *tmp;
> >
> >       switch (event) {
> >       case ACPI_NOTIFY_BUS_CHECK:
> > -             if (!dock_in_progress(ds) && dock_present(ds)) {
> > +             if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle,
> 
> On the Thinkpad X60, when booting undocked, the first time I dock,
> acpi_bus_get_device() returns 0, so hotplug is not performed
> properly. Second time (undock->dock), it works.
So the patch breaks your X60? acpi_bus_get_device() returns 0 means the
device is already in ACPI device tree, but it shouldn't as it's not
present.

Thanks,
Shaohua

Re: [patch 1/11]avoid check _STA method

[Holger Macht]

On Wed 24. Sep - 10:51:05, Shaohua Li wrote:
> On Mon, 2008-09-22 at 18:57 +0800, Holger Macht wrote:
> > On Thu 28. Aug - 10:02:03, Shaohua Li wrote:
> > > In some BIOSes, every _STA method call will send a notification again,
> > > this cause freeze. And in some BIOSes, it appears _STA should be called
> > > after _DCK. This tries to avoid calls _STA, and still keep the device
> > > present check.
> > > http://bugzilla.kernel.org/show_bug.cgi?id=10431
> > 
> > Finally found some time to test this...
> > 
> > >
> > > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > > ---
> > >  drivers/acpi/dock.c |    5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > Index: linux/drivers/acpi/dock.c
> > > ===================================================================
> > > --- linux.orig/drivers/acpi/dock.c    2008-08-27 14:38:44.000000000 +0800
> > > +++ linux/drivers/acpi/dock.c 2008-08-28 09:33:02.000000000 +0800
> > > @@ -604,14 +604,17 @@ static int handle_eject_request(struct d
> > >  static void dock_notify(acpi_handle handle, u32 event, void *data)
> > >  {
> > >       struct dock_station *ds = data;
> > > +     struct acpi_device *tmp;
> > >
> > >       switch (event) {
> > >       case ACPI_NOTIFY_BUS_CHECK:
> > > -             if (!dock_in_progress(ds) && dock_present(ds)) {
> > > +             if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle,
> > 
> > On the Thinkpad X60, when booting undocked, the first time I dock,
> > acpi_bus_get_device() returns 0, so hotplug is not performed
> > properly. Second time (undock->dock), it works.
> So the patch breaks your X60? acpi_bus_get_device() returns 0 means the
> device is already in ACPI device tree, but it shouldn't as it's not
> present.

Well, it does not really break it, I just have to dock a second time :-)
This second time, acpi_bus_get_device() return -ENODEV.

So for this laptop, the old way with dock_present() (checking _STA) would
work just fine.

Regards,
	Holger

Re: [patch 1/11]avoid check _STA method

[Holger Macht]

On Wed 24. Sep - 10:51:05, Shaohua Li wrote:
> On Mon, 2008-09-22 at 18:57 +0800, Holger Macht wrote:
> > On Thu 28. Aug - 10:02:03, Shaohua Li wrote:
> > > In some BIOSes, every _STA method call will send a notification again,
> > > this cause freeze. And in some BIOSes, it appears _STA should be called
> > > after _DCK. This tries to avoid calls _STA, and still keep the device
> > > present check.
> > > http://bugzilla.kernel.org/show_bug.cgi?id=10431
> > 
> > Finally found some time to test this...
> > 
> > >
> > > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > > ---
> > >  drivers/acpi/dock.c |    5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > Index: linux/drivers/acpi/dock.c
> > > ===================================================================
> > > --- linux.orig/drivers/acpi/dock.c    2008-08-27 14:38:44.000000000 +0800
> > > +++ linux/drivers/acpi/dock.c 2008-08-28 09:33:02.000000000 +0800
> > > @@ -604,14 +604,17 @@ static int handle_eject_request(struct d
> > >  static void dock_notify(acpi_handle handle, u32 event, void *data)
> > >  {
> > >       struct dock_station *ds = data;
> > > +     struct acpi_device *tmp;
> > >
> > >       switch (event) {
> > >       case ACPI_NOTIFY_BUS_CHECK:
> > > -             if (!dock_in_progress(ds) && dock_present(ds)) {
> > > +             if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle,
> > 
> > On the Thinkpad X60, when booting undocked, the first time I dock,
> > acpi_bus_get_device() returns 0, so hotplug is not performed
> > properly. Second time (undock->dock), it works.
> So the patch breaks your X60? acpi_bus_get_device() returns 0 means the
> device is already in ACPI device tree, but it shouldn't as it's not
> present.

Do you have a model number of a system which needs this patch? If so,
maybe I could get my hands on one to do some more testing...maybe finding
another solution...

Regards,
	Holger

Re: [patch 1/11]avoid check _STA method

[Shaohua Li]

On Thu, Sep 25, 2008 at 10:49:45PM +0800, Holger Macht wrote:
> On Wed 24. Sep - 10:51:05, Shaohua Li wrote:
> > On Mon, 2008-09-22 at 18:57 +0800, Holger Macht wrote:
> > > On Thu 28. Aug - 10:02:03, Shaohua Li wrote:
> > > > In some BIOSes, every _STA method call will send a notification again,
> > > > this cause freeze. And in some BIOSes, it appears _STA should be called
> > > > after _DCK. This tries to avoid calls _STA, and still keep the device
> > > > present check.
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=10431
> > >
> > > Finally found some time to test this...
> > >
> > > >
> > > > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > > > ---
> > > >  drivers/acpi/dock.c |    5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > Index: linux/drivers/acpi/dock.c
> > > > ===================================================================
> > > > --- linux.orig/drivers/acpi/dock.c    2008-08-27 14:38:44.000000000 +0800
> > > > +++ linux/drivers/acpi/dock.c 2008-08-28 09:33:02.000000000 +0800
> > > > @@ -604,14 +604,17 @@ static int handle_eject_request(struct d
> > > >  static void dock_notify(acpi_handle handle, u32 event, void *data)
> > > >  {
> > > >       struct dock_station *ds = data;
> > > > +     struct acpi_device *tmp;
> > > >
> > > >       switch (event) {
> > > >       case ACPI_NOTIFY_BUS_CHECK:
> > > > -             if (!dock_in_progress(ds) && dock_present(ds)) {
> > > > +             if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle,
> > >
> > > On the Thinkpad X60, when booting undocked, the first time I dock,
> > > acpi_bus_get_device() returns 0, so hotplug is not performed
> > > properly. Second time (undock->dock), it works.
> > So the patch breaks your X60? acpi_bus_get_device() returns 0 means the
> > device is already in ACPI device tree, but it shouldn't as it's not
> > present.
> 
> Do you have a model number of a system which needs this patch? If so,
> maybe I could get my hands on one to do some more testing...maybe finding
> another solution...
please see the bugzilla above. I haven't a system which requires this at hand.
I'm quite strange why acpi_bus_get_device() returns 0 for a non-present device,
maybe you can dig into this. Is it possible the BIOS wrongly reported device
status?

Thanks,
Shaohua

Re: [patch 1/11]avoid check _STA method

[Holger Macht]

On Fri 26. Sep - 09:04:59, Shaohua Li wrote:
> On Thu, Sep 25, 2008 at 10:49:45PM +0800, Holger Macht wrote:
> > On Wed 24. Sep - 10:51:05, Shaohua Li wrote:
> > > On Mon, 2008-09-22 at 18:57 +0800, Holger Macht wrote:
> > > > On Thu 28. Aug - 10:02:03, Shaohua Li wrote:
> > > > > In some BIOSes, every _STA method call will send a notification again,
> > > > > this cause freeze. And in some BIOSes, it appears _STA should be called
> > > > > after _DCK. This tries to avoid calls _STA, and still keep the device
> > > > > present check.
> > > > > http://bugzilla.kernel.org/show_bug.cgi?id=10431
> > > >
> > > > Finally found some time to test this...
> > > >
> > > > >
> > > > > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > > > > ---
> > > > >  drivers/acpi/dock.c |    5 ++++-
> > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > >
> > > > > Index: linux/drivers/acpi/dock.c
> > > > > ===================================================================
> > > > > --- linux.orig/drivers/acpi/dock.c    2008-08-27 14:38:44.000000000 +0800
> > > > > +++ linux/drivers/acpi/dock.c 2008-08-28 09:33:02.000000000 +0800
> > > > > @@ -604,14 +604,17 @@ static int handle_eject_request(struct d
> > > > >  static void dock_notify(acpi_handle handle, u32 event, void *data)
> > > > >  {
> > > > >       struct dock_station *ds = data;
> > > > > +     struct acpi_device *tmp;
> > > > >
> > > > >       switch (event) {
> > > > >       case ACPI_NOTIFY_BUS_CHECK:
> > > > > -             if (!dock_in_progress(ds) && dock_present(ds)) {
> > > > > +             if (!dock_in_progress(ds) && acpi_bus_get_device(ds->handle,
> > > >
> > > > On the Thinkpad X60, when booting undocked, the first time I dock,
> > > > acpi_bus_get_device() returns 0, so hotplug is not performed
> > > > properly. Second time (undock->dock), it works.
> > > So the patch breaks your X60? acpi_bus_get_device() returns 0 means the
> > > device is already in ACPI device tree, but it shouldn't as it's not
> > > present.
> > 
> > Do you have a model number of a system which needs this patch? If so,
> > maybe I could get my hands on one to do some more testing...maybe finding
> > another solution...
> please see the bugzilla above. I haven't a system which requires this at hand.
> I'm quite strange why acpi_bus_get_device() returns 0 for a non-present device,
> maybe you can dig into this. Is it possible the BIOS wrongly reported device
> status?

No, it was a bug in the acpi subsystem:

commit 39a0ad871000d2a016a4fa113a6e53d22aabf25d
Author: Zhao Yakui <yakui.zhao@intel.com>
Date:   Mon Aug 11 13:40:22 2008 +0800

    ACPI : Load device driver according to the status of acpi device


Regards,
	Holger

Re: [patch 1/11]avoid check _STA method

[Len Brown]

1-11 applied to acpi-test

thanks,
-Len