[PATCH 1/2] ACPI / battery: call ACPI notifier chain in acpi_battery_notify

[PATCH 1/2] ACPI / battery: call ACPI notifier chain in acpi_battery_notify

[Alexander Mezin]

Allow other drivers to subscribe to battery status notifications.
Just like AC driver does.

Signed-off-by: Alexander Mezin <mezin.alexander@gmail.com>
---
 drivers/acpi/battery.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 797a693..8c98fd6 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -736,6 +736,7 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
 	acpi_bus_generate_netlink_event(device->pnp.device_class,
 					dev_name(&device->dev), event,
 					acpi_battery_present(battery));
+	acpi_notifier_call_chain(device, event, acpi_battery_present(battery));
 	/* acpi_battery_update could remove power_supply object */
 	if (old && battery->bat.dev)
 		power_supply_changed(&battery->bat);
-- 
1.9.0

[PATCH 2/2] ACPI / AC: recheck adapter status upon battery notifications

[Alexander Mezin]

On HP Pavilion dv6-6179er there are no notifications when AC adapter
is plugged/unplugged.
However, when AC status is read (acpi_ac_get_state), and if AC status
has changed, AML code triggers the notification.

This patch solves the problem by re-reading AC adapter status upon
battery notifications.

Signed-off-by: Alexander Mezin <mezin.alexander@gmail.com>
---
 drivers/acpi/ac.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 6f190bc..1d3903e 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -44,6 +44,8 @@
 #define ACPI_AC_STATUS_ONLINE		0x01
 #define ACPI_AC_STATUS_UNKNOWN		0xFF
 
+#define ACPI_BATTERY_CLASS		"battery"
+
 #define _COMPONENT		ACPI_AC_COMPONENT
 ACPI_MODULE_NAME("ac");
 
@@ -57,6 +59,7 @@ struct acpi_ac {
 	struct power_supply charger;
 	struct platform_device *pdev;
 	unsigned long long state;
+	struct notifier_block battery_nb;
 };
 
 #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger)
@@ -152,6 +155,18 @@ static void acpi_ac_notify_handler(acpi_handle handle, u32 event, void *data)
 	return;
 }
 
+static int acpi_ac_battery_notify(struct notifier_block *nb,
+				  unsigned long action, void *data)
+{
+	struct acpi_ac *ac = container_of(nb, struct acpi_ac, battery_nb);
+	struct acpi_bus_event *event = (struct acpi_bus_event *)data;
+
+	if (strcmp(event->device_class, ACPI_BATTERY_CLASS) == 0)
+		acpi_ac_get_state(ac);
+
+	return NOTIFY_OK;
+}
+
 static int thinkpad_e530_quirk(const struct dmi_system_id *d)
 {
 	ac_sleep_before_get_state_ms = 1000;
@@ -215,6 +230,8 @@ static int acpi_ac_probe(struct platform_device *pdev)
 	       acpi_device_name(adev), acpi_device_bid(adev),
 	       ac->state ? "on-line" : "off-line");
 
+	ac->battery_nb.notifier_call = acpi_ac_battery_notify;
+	register_acpi_notifier(&ac->battery_nb);
 end:
 	if (result)
 		kfree(ac);
@@ -261,6 +278,7 @@ static int acpi_ac_remove(struct platform_device *pdev)
 	ac = platform_get_drvdata(pdev);
 	if (ac->charger.dev)
 		power_supply_unregister(&ac->charger);
+	unregister_acpi_notifier(&ac->battery_nb);
 
 	kfree(ac);
 
-- 
1.9.0

Re: [PATCH 2/2] ACPI / AC: recheck adapter status upon battery notifications

[Rafael J. Wysocki]

On Monday, March 10, 2014 05:37:32 AM Alexander Mezin wrote:
> On HP Pavilion dv6-6179er there are no notifications when AC adapter
> is plugged/unplugged.
> However, when AC status is read (acpi_ac_get_state), and if AC status
> has changed, AML code triggers the notification.
> 
> This patch solves the problem by re-reading AC adapter status upon
> battery notifications.
> 
> Signed-off-by: Alexander Mezin <mezin.alexander@gmail.com>
> ---
>  drivers/acpi/ac.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
> index 6f190bc..1d3903e 100644
> --- a/drivers/acpi/ac.c
> +++ b/drivers/acpi/ac.c
> @@ -44,6 +44,8 @@
>  #define ACPI_AC_STATUS_ONLINE		0x01
>  #define ACPI_AC_STATUS_UNKNOWN		0xFF
>  
> +#define ACPI_BATTERY_CLASS		"battery"

Can you please put that definition into a header file and make all .c files
referring to ACPI_BATTERY_CLASS include that header?

> +
>  #define _COMPONENT		ACPI_AC_COMPONENT
>  ACPI_MODULE_NAME("ac");
>  
> @@ -57,6 +59,7 @@ struct acpi_ac {
>  	struct power_supply charger;
>  	struct platform_device *pdev;
>  	unsigned long long state;
> +	struct notifier_block battery_nb;
>  };
>  
>  #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger)
> @@ -152,6 +155,18 @@ static void acpi_ac_notify_handler(acpi_handle handle, u32 event, void *data)
>  	return;
>  }
>  
> +static int acpi_ac_battery_notify(struct notifier_block *nb,
> +				  unsigned long action, void *data)
> +{
> +	struct acpi_ac *ac = container_of(nb, struct acpi_ac, battery_nb);
> +	struct acpi_bus_event *event = (struct acpi_bus_event *)data;
> +
> +	if (strcmp(event->device_class, ACPI_BATTERY_CLASS) == 0)
> +		acpi_ac_get_state(ac);
> +
> +	return NOTIFY_OK;
> +}
> +
>  static int thinkpad_e530_quirk(const struct dmi_system_id *d)
>  {
>  	ac_sleep_before_get_state_ms = 1000;
> @@ -215,6 +230,8 @@ static int acpi_ac_probe(struct platform_device *pdev)
>  	       acpi_device_name(adev), acpi_device_bid(adev),
>  	       ac->state ? "on-line" : "off-line");
>  
> +	ac->battery_nb.notifier_call = acpi_ac_battery_notify;
> +	register_acpi_notifier(&ac->battery_nb);
>  end:
>  	if (result)
>  		kfree(ac);
> @@ -261,6 +278,7 @@ static int acpi_ac_remove(struct platform_device *pdev)
>  	ac = platform_get_drvdata(pdev);
>  	if (ac->charger.dev)
>  		power_supply_unregister(&ac->charger);
> +	unregister_acpi_notifier(&ac->battery_nb);
>  
>  	kfree(ac);
>  
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 2/2] ACPI / AC: recheck adapter status upon battery notifications

[Alexander Mezin]

2014-03-10 6:11 GMT+07:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> On Monday, March 10, 2014 05:37:32 AM Alexander Mezin wrote:
>> On HP Pavilion dv6-6179er there are no notifications when AC adapter
>> is plugged/unplugged.
>> However, when AC status is read (acpi_ac_get_state), and if AC status
>> has changed, AML code triggers the notification.
>>
>> This patch solves the problem by re-reading AC adapter status upon
>> battery notifications.
>>
>> Signed-off-by: Alexander Mezin <mezin.alexander@gmail.com>
>> ---
>>  drivers/acpi/ac.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
>> index 6f190bc..1d3903e 100644
>> --- a/drivers/acpi/ac.c
>> +++ b/drivers/acpi/ac.c
>> @@ -44,6 +44,8 @@
>>  #define ACPI_AC_STATUS_ONLINE                0x01
>>  #define ACPI_AC_STATUS_UNKNOWN               0xFF
>>
>> +#define ACPI_BATTERY_CLASS           "battery"
>
> Can you please put that definition into a header file and make all .c files
> referring to ACPI_BATTERY_CLASS include that header?
At least this definition is copied in sbs.c too. And ACPI_AC_CLASS is
used even in radeon_acpi.c. And ACPI_PROCESSOR_CLASS in a lot of
places too.
Maybe it makes sense to move all these class definitions (and maybe
notification codes) into single (public?) header file? But as a
separate patch series.
>
>> +
>>  #define _COMPONENT           ACPI_AC_COMPONENT
>>  ACPI_MODULE_NAME("ac");
>>
>> @@ -57,6 +59,7 @@ struct acpi_ac {
>>       struct power_supply charger;
>>       struct platform_device *pdev;
>>       unsigned long long state;
>> +     struct notifier_block battery_nb;
>>  };
>>
>>  #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger)
>> @@ -152,6 +155,18 @@ static void acpi_ac_notify_handler(acpi_handle handle, u32 event, void *data)
>>       return;
>>  }
>>
>> +static int acpi_ac_battery_notify(struct notifier_block *nb,
>> +                               unsigned long action, void *data)
>> +{
>> +     struct acpi_ac *ac = container_of(nb, struct acpi_ac, battery_nb);
>> +     struct acpi_bus_event *event = (struct acpi_bus_event *)data;
>> +
>> +     if (strcmp(event->device_class, ACPI_BATTERY_CLASS) == 0)
>> +             acpi_ac_get_state(ac);
>> +
>> +     return NOTIFY_OK;
>> +}
>> +
>>  static int thinkpad_e530_quirk(const struct dmi_system_id *d)
>>  {
>>       ac_sleep_before_get_state_ms = 1000;
>> @@ -215,6 +230,8 @@ static int acpi_ac_probe(struct platform_device *pdev)
>>              acpi_device_name(adev), acpi_device_bid(adev),
>>              ac->state ? "on-line" : "off-line");
>>
>> +     ac->battery_nb.notifier_call = acpi_ac_battery_notify;
>> +     register_acpi_notifier(&ac->battery_nb);
>>  end:
>>       if (result)
>>               kfree(ac);
>> @@ -261,6 +278,7 @@ static int acpi_ac_remove(struct platform_device *pdev)
>>       ac = platform_get_drvdata(pdev);
>>       if (ac->charger.dev)
>>               power_supply_unregister(&ac->charger);
>> +     unregister_acpi_notifier(&ac->battery_nb);
>>
>>       kfree(ac);
>>
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 2/2] ACPI / AC: recheck adapter status upon battery notifications

[Rafael J. Wysocki]

On Monday, March 10, 2014 06:21:50 AM Alexander Mezin wrote:
> 2014-03-10 6:11 GMT+07:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> > On Monday, March 10, 2014 05:37:32 AM Alexander Mezin wrote:
> >> On HP Pavilion dv6-6179er there are no notifications when AC adapter
> >> is plugged/unplugged.
> >> However, when AC status is read (acpi_ac_get_state), and if AC status
> >> has changed, AML code triggers the notification.
> >>
> >> This patch solves the problem by re-reading AC adapter status upon
> >> battery notifications.
> >>
> >> Signed-off-by: Alexander Mezin <mezin.alexander@gmail.com>
> >> ---
> >>  drivers/acpi/ac.c | 18 ++++++++++++++++++
> >>  1 file changed, 18 insertions(+)
> >>
> >> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
> >> index 6f190bc..1d3903e 100644
> >> --- a/drivers/acpi/ac.c
> >> +++ b/drivers/acpi/ac.c
> >> @@ -44,6 +44,8 @@
> >>  #define ACPI_AC_STATUS_ONLINE                0x01
> >>  #define ACPI_AC_STATUS_UNKNOWN               0xFF
> >>
> >> +#define ACPI_BATTERY_CLASS           "battery"
> >
> > Can you please put that definition into a header file and make all .c files
> > referring to ACPI_BATTERY_CLASS include that header?
> At least this definition is copied in sbs.c too.

Yes, it is.  Is that a good enough reason to add one more copy?

> And ACPI_AC_CLASS is used even in radeon_acpi.c. And ACPI_PROCESSOR_CLASS
> in a lot of places too.
> Maybe it makes sense to move all these class definitions (and maybe
> notification codes) into single (public?) header file? But as a
> separate patch series.

Well, it would be good to do that, but at least no one is trying to add more
copies of those.

> >
> >> +
> >>  #define _COMPONENT           ACPI_AC_COMPONENT
> >>  ACPI_MODULE_NAME("ac");
> >>
> >> @@ -57,6 +59,7 @@ struct acpi_ac {
> >>       struct power_supply charger;
> >>       struct platform_device *pdev;
> >>       unsigned long long state;
> >> +     struct notifier_block battery_nb;
> >>  };
> >>
> >>  #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger)
> >> @@ -152,6 +155,18 @@ static void acpi_ac_notify_handler(acpi_handle handle, u32 event, void *data)
> >>       return;
> >>  }
> >>
> >> +static int acpi_ac_battery_notify(struct notifier_block *nb,
> >> +                               unsigned long action, void *data)
> >> +{
> >> +     struct acpi_ac *ac = container_of(nb, struct acpi_ac, battery_nb);
> >> +     struct acpi_bus_event *event = (struct acpi_bus_event *)data;
> >> +
> >> +     if (strcmp(event->device_class, ACPI_BATTERY_CLASS) == 0)
> >> +             acpi_ac_get_state(ac);
> >> +
> >> +     return NOTIFY_OK;
> >> +}
> >> +
> >>  static int thinkpad_e530_quirk(const struct dmi_system_id *d)
> >>  {
> >>       ac_sleep_before_get_state_ms = 1000;
> >> @@ -215,6 +230,8 @@ static int acpi_ac_probe(struct platform_device *pdev)
> >>              acpi_device_name(adev), acpi_device_bid(adev),
> >>              ac->state ? "on-line" : "off-line");
> >>
> >> +     ac->battery_nb.notifier_call = acpi_ac_battery_notify;
> >> +     register_acpi_notifier(&ac->battery_nb);
> >>  end:
> >>       if (result)
> >>               kfree(ac);
> >> @@ -261,6 +278,7 @@ static int acpi_ac_remove(struct platform_device *pdev)
> >>       ac = platform_get_drvdata(pdev);
> >>       if (ac->charger.dev)
> >>               power_supply_unregister(&ac->charger);
> >> +     unregister_acpi_notifier(&ac->battery_nb);
> >>
> >>       kfree(ac);
> >>
> >>

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.