* [FIX PATCH] ACPI/AC: Add sleep quirk for Thinkpad e530
@ 2013-05-06 12:27 Lan Tianyu
2013-05-07 23:54 ` Rafael J. Wysocki
0 siblings, 1 reply; 3+ messages in thread
From: Lan Tianyu @ 2013-05-06 12:27 UTC (permalink / raw)
To: rjw, lenb; +Cc: Lan Tianyu, linux-acpi
Thinkpad e530 bios notify ac device first and then sleep
a specific time before doing actual operations in the
EC event handler(_Qxx). This will cause the AC state
reported by ACPI event.
Method (_Q27, 0, NotSerialized)
{
Notify (AC, 0x80)
Sleep (0x03E8)
Store (Zero, PWRS)
PNOT ()
}
This patch is to add a 1s sleep in the ac driver's
notify handler before acpi_ac_get_state() to make
sure get right state.
https://bugzilla.kernel.org/show_bug.cgi?id=45221
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
drivers/acpi/ac.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 6d5bf64..0292cbb 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -28,6 +28,8 @@
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/types.h>
+#include <linux/dmi.h>
+#include <linux/delay.h>
#ifdef CONFIG_ACPI_PROCFS_POWER
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
@@ -74,6 +76,8 @@ static int acpi_ac_resume(struct device *dev);
#endif
static SIMPLE_DEV_PM_OPS(acpi_ac_pm, NULL, acpi_ac_resume);
+static int ac_flag_sleep_for_get_state;
+
static struct acpi_driver acpi_ac_driver = {
.name = "ac",
.class = ACPI_AC_CLASS,
@@ -252,6 +256,15 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
case ACPI_AC_NOTIFY_STATUS:
case ACPI_NOTIFY_BUS_CHECK:
case ACPI_NOTIFY_DEVICE_CHECK:
+ /* Some buggy bios notify ac device first and then sleep
+ * a specific time before doing actual operations in the
+ * EC event handler(_Qxx). This will cause the AC state
+ * reported by ACPI event wrong. So add a 1s sleep here
+ * to ensure get correct state.
+ */
+ if (ac_flag_sleep_for_get_state)
+ msleep(1000);
+
acpi_ac_get_state(ac);
acpi_bus_generate_proc_event(device, event, (u32) ac->state);
acpi_bus_generate_netlink_event(device->pnp.device_class,
@@ -264,6 +277,24 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
return;
}
+static int ac_sleep_for_get_state(const struct dmi_system_id *d)
+{
+ ac_flag_sleep_for_get_state = 1;
+ return 0;
+}
+
+static struct dmi_system_id __initdata ac_dmi_table[] = {
+ {
+ .callback = ac_sleep_for_get_state,
+ .ident = "thinkpad e530",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "32597CG"),
+ },
+ },
+ {},
+};
+
static int acpi_ac_add(struct acpi_device *device)
{
int result = 0;
@@ -312,6 +343,7 @@ static int acpi_ac_add(struct acpi_device *device)
kfree(ac);
}
+ dmi_check_system(ac_dmi_table);
return result;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [FIX PATCH] ACPI/AC: Add sleep quirk for Thinkpad e530
2013-05-06 12:27 [FIX PATCH] ACPI/AC: Add sleep quirk for Thinkpad e530 Lan Tianyu
@ 2013-05-07 23:54 ` Rafael J. Wysocki
2013-05-08 1:32 ` Lan Tianyu
0 siblings, 1 reply; 3+ messages in thread
From: Rafael J. Wysocki @ 2013-05-07 23:54 UTC (permalink / raw)
To: Lan Tianyu; +Cc: lenb, linux-acpi
On Monday, May 06, 2013 08:27:33 PM Lan Tianyu wrote:
> Thinkpad e530 bios notify ac device first and then sleep
> a specific time before doing actual operations in the
> EC event handler(_Qxx). This will cause the AC state
> reported by ACPI event.
>
> Method (_Q27, 0, NotSerialized)
> {
> Notify (AC, 0x80)
> Sleep (0x03E8)
> Store (Zero, PWRS)
> PNOT ()
> }
>
> This patch is to add a 1s sleep in the ac driver's
> notify handler before acpi_ac_get_state() to make
> sure get right state.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=45221
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> drivers/acpi/ac.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
> index 6d5bf64..0292cbb 100644
> --- a/drivers/acpi/ac.c
> +++ b/drivers/acpi/ac.c
> @@ -28,6 +28,8 @@
> #include <linux/slab.h>
> #include <linux/init.h>
> #include <linux/types.h>
> +#include <linux/dmi.h>
> +#include <linux/delay.h>
> #ifdef CONFIG_ACPI_PROCFS_POWER
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> @@ -74,6 +76,8 @@ static int acpi_ac_resume(struct device *dev);
> #endif
> static SIMPLE_DEV_PM_OPS(acpi_ac_pm, NULL, acpi_ac_resume);
>
> +static int ac_flag_sleep_for_get_state;
Hmm. Why don't you replace ac_flag_sleep_for_get_state with the time
to sleep in acpi_ac_notify(), something like ac_sleep_before_get_state_ms
and then do something like this ->
> +
> static struct acpi_driver acpi_ac_driver = {
> .name = "ac",
> .class = ACPI_AC_CLASS,
> @@ -252,6 +256,15 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
> case ACPI_AC_NOTIFY_STATUS:
> case ACPI_NOTIFY_BUS_CHECK:
> case ACPI_NOTIFY_DEVICE_CHECK:
> + /* Some buggy bios notify ac device first and then sleep
> + * a specific time before doing actual operations in the
> + * EC event handler(_Qxx). This will cause the AC state
> + * reported by ACPI event wrong. So add a 1s sleep here
> + * to ensure get correct state.
> + */
> + if (ac_flag_sleep_for_get_state)
> + msleep(1000);
-> if (ac_sleep_before_get_state_ms > 0)
msleep(ac_sleep_before_get_state_ms);
and *then* set ac_sleep_before_get_state_ms to 1000 in the Thinkpad e530
specific quirk?
If we find another machine having this problem in the future, but needing
a different sleep time, it will be easier to add it to the blacklist in that
case.
Thanks,
Rafael
> +
> acpi_ac_get_state(ac);
> acpi_bus_generate_proc_event(device, event, (u32) ac->state);
> acpi_bus_generate_netlink_event(device->pnp.device_class,
> @@ -264,6 +277,24 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
> return;
> }
>
> +static int ac_sleep_for_get_state(const struct dmi_system_id *d)
> +{
> + ac_flag_sleep_for_get_state = 1;
> + return 0;
> +}
> +
> +static struct dmi_system_id __initdata ac_dmi_table[] = {
> + {
> + .callback = ac_sleep_for_get_state,
> + .ident = "thinkpad e530",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "32597CG"),
> + },
> + },
> + {},
> +};
> +
> static int acpi_ac_add(struct acpi_device *device)
> {
> int result = 0;
> @@ -312,6 +343,7 @@ static int acpi_ac_add(struct acpi_device *device)
> kfree(ac);
> }
>
> + dmi_check_system(ac_dmi_table);
> return result;
> }
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FIX PATCH] ACPI/AC: Add sleep quirk for Thinkpad e530
2013-05-07 23:54 ` Rafael J. Wysocki
@ 2013-05-08 1:32 ` Lan Tianyu
0 siblings, 0 replies; 3+ messages in thread
From: Lan Tianyu @ 2013-05-08 1:32 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: lenb, linux-acpi
On 2013年05月08日 07:54, Rafael J. Wysocki wrote:
> On Monday, May 06, 2013 08:27:33 PM Lan Tianyu wrote:
>> Thinkpad e530 bios notify ac device first and then sleep
>> a specific time before doing actual operations in the
>> EC event handler(_Qxx). This will cause the AC state
>> reported by ACPI event.
>>
>> Method (_Q27, 0, NotSerialized)
>> {
>> Notify (AC, 0x80)
>> Sleep (0x03E8)
>> Store (Zero, PWRS)
>> PNOT ()
>> }
>>
>> This patch is to add a 1s sleep in the ac driver's
>> notify handler before acpi_ac_get_state() to make
>> sure get right state.
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=45221
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>> drivers/acpi/ac.c | 32 ++++++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
>> index 6d5bf64..0292cbb 100644
>> --- a/drivers/acpi/ac.c
>> +++ b/drivers/acpi/ac.c
>> @@ -28,6 +28,8 @@
>> #include <linux/slab.h>
>> #include <linux/init.h>
>> #include <linux/types.h>
>> +#include <linux/dmi.h>
>> +#include <linux/delay.h>
>> #ifdef CONFIG_ACPI_PROCFS_POWER
>> #include <linux/proc_fs.h>
>> #include <linux/seq_file.h>
>> @@ -74,6 +76,8 @@ static int acpi_ac_resume(struct device *dev);
>> #endif
>> static SIMPLE_DEV_PM_OPS(acpi_ac_pm, NULL, acpi_ac_resume);
>>
>> +static int ac_flag_sleep_for_get_state;
>
> Hmm. Why don't you replace ac_flag_sleep_for_get_state with the time
> to sleep in acpi_ac_notify(), something like ac_sleep_before_get_state_ms
> and then do something like this ->
>
>> +
>> static struct acpi_driver acpi_ac_driver = {
>> .name = "ac",
>> .class = ACPI_AC_CLASS,
>> @@ -252,6 +256,15 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
>> case ACPI_AC_NOTIFY_STATUS:
>> case ACPI_NOTIFY_BUS_CHECK:
>> case ACPI_NOTIFY_DEVICE_CHECK:
>> + /* Some buggy bios notify ac device first and then sleep
>> + * a specific time before doing actual operations in the
>> + * EC event handler(_Qxx). This will cause the AC state
>> + * reported by ACPI event wrong. So add a 1s sleep here
>> + * to ensure get correct state.
>> + */
>> + if (ac_flag_sleep_for_get_state)
>> + msleep(1000);
>
> -> if (ac_sleep_before_get_state_ms > 0)
> msleep(ac_sleep_before_get_state_ms);
>
> and *then* set ac_sleep_before_get_state_ms to 1000 in the Thinkpad e530
> specific quirk?
>
> If we find another machine having this problem in the future, but needing
> a different sleep time, it will be easier to add it to the blacklist in that
> case.
Yes, that will be more flexible. Thanks for your advice. I will update soon.
>
> Thanks,
> Rafael
>
>
>> +
>> acpi_ac_get_state(ac);
>> acpi_bus_generate_proc_event(device, event, (u32) ac->state);
>> acpi_bus_generate_netlink_event(device->pnp.device_class,
>> @@ -264,6 +277,24 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
>> return;
>> }
>>
>> +static int ac_sleep_for_get_state(const struct dmi_system_id *d)
>> +{
>> + ac_flag_sleep_for_get_state = 1;
>> + return 0;
>> +}
>> +
>> +static struct dmi_system_id __initdata ac_dmi_table[] = {
>> + {
>> + .callback = ac_sleep_for_get_state,
>> + .ident = "thinkpad e530",
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "32597CG"),
>> + },
>> + },
>> + {},
>> +};
>> +
>> static int acpi_ac_add(struct acpi_device *device)
>> {
>> int result = 0;
>> @@ -312,6 +343,7 @@ static int acpi_ac_add(struct acpi_device *device)
>> kfree(ac);
>> }
>>
>> + dmi_check_system(ac_dmi_table);
>> return result;
>> }
>>
>>
--
Best regards
Tianyu Lan
--
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-05-08 1:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-06 12:27 [FIX PATCH] ACPI/AC: Add sleep quirk for Thinkpad e530 Lan Tianyu
2013-05-07 23:54 ` Rafael J. Wysocki
2013-05-08 1:32 ` Lan Tianyu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox