[PATCH v2 1/4] ACPI: battery: Add acpi_battery_unregister() function

[PATCH v2 1/4] ACPI: battery: Add acpi_battery_unregister() function

[Hans de Goede]

On some systems we have a native PMIC driver which provides battery
monitoring, while the ACPI battery driver is broken on these systems
due to bad DSTDs or because of missing vendor specific ACPI opregion
(e.g. BMOP opregion) support, which the ACPI battery device in the
dsdt relies on.

This leads to there being 2 battery power_supply-s registed like this:

~$ acpi
Battery 0: Charging, 84%, 00:49:39 until charged
Battery 1: Unknown, 0%, rate information unavailable

Even if the ACPI battery where to function fine (which on systems
where we have a native PMIC driver it often doesn't) we still do not
want to export the same battery to userspace twice.

This commit adds an acpi_battery_unregister() function which native
PMIC drivers can call to tell the ACPI-battery driver to unregister
itself so that we do not end up with 2 power_supply-s for the same
battery device.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=194811
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Sergei Trusov <t.rus76@ya.ru>
---
Changes in v2:
-Fix capitalization in commit msg
-Minor style cleanups
---
 drivers/acpi/battery.c     | 33 ++++++++++++++++++++++++++++++++-
 include/linux/power/acpi.h | 18 ++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/power/acpi.h

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 4ef1e46..b26a2b7 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -29,6 +29,7 @@
 #include <linux/async.h>
 #include <linux/dmi.h>
 #include <linux/delay.h>
+#include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
 #include <asm/unaligned.h>
@@ -40,6 +41,7 @@
 #endif
 
 #include <linux/acpi.h>
+#include <linux/power/acpi.h>
 #include <linux/power_supply.h>
 
 #include "battery.h"
@@ -66,6 +68,10 @@ MODULE_AUTHOR("Alexey Starikovskiy <astarikovskiy@suse.de>");
 MODULE_DESCRIPTION("ACPI Battery Driver");
 MODULE_LICENSE("GPL");
 
+enum init_state_enum { BAT_NONE, BAT_INITIALIZED, BAT_EXITED };
+
+static enum init_state_enum init_state;
+static DEFINE_MUTEX(init_state_mutex);
 static async_cookie_t async_cookie;
 static int battery_bix_broken_package;
 static int battery_notification_delay_ms;
@@ -1336,17 +1342,42 @@ static int __init acpi_battery_init(void)
 	if (acpi_disabled)
 		return -ENODEV;
 
+	/* Check if acpi_battery_unregister() got called before _init() */
+	mutex_lock(&init_state_mutex);
+	if (init_state != BAT_NONE)
+		goto out_unlock;
+
 	async_cookie = async_schedule(acpi_battery_init_async, NULL);
+	init_state = BAT_INITIALIZED;
+
+out_unlock:
+	mutex_unlock(&init_state_mutex);
+
 	return 0;
 }
 
-static void __exit acpi_battery_exit(void)
+void acpi_battery_unregister(void)
 {
+	/* Check if _init() is done and only do unregister once */
+	mutex_lock(&init_state_mutex);
+	if (init_state != BAT_INITIALIZED)
+		goto out_exit;
+
 	async_synchronize_cookie(async_cookie + 1);
 	acpi_bus_unregister_driver(&acpi_battery_driver);
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_unlock_battery_dir(acpi_battery_dir);
 #endif
+
+out_exit:
+	init_state = BAT_EXITED;
+	mutex_unlock(&init_state_mutex);
+}
+EXPORT_SYMBOL_GPL(acpi_battery_unregister);
+
+static void __exit acpi_battery_exit(void)
+{
+	acpi_battery_unregister();
 }
 
 module_init(acpi_battery_init);
diff --git a/include/linux/power/acpi.h b/include/linux/power/acpi.h
new file mode 100644
index 0000000..83bdfb9
--- /dev/null
+++ b/include/linux/power/acpi.h
@@ -0,0 +1,18 @@
+/*
+ * Functions exported by the acpi power_supply drivers
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX_POWER_ACPI_H_
+#define __LINUX_POWER_ACPI_H_
+
+#if IS_ENABLED(CONFIG_ACPI_BATTERY)
+void acpi_battery_unregister(void);
+#else
+static inline void acpi_battery_unregister(void) {}
+#endif
+
+#endif
-- 
2.9.3

[PATCH v2 2/4] ACPI: ac: Add acpi_ac_unregister() function

[Hans de Goede]

On some systems we have a native PMIC driver which provides Mains
monitoring, while the ACPI ac driver is broken on these systems
due to bad DSTDs or because of missing vendor specific ACPI opregion
(e.g. BMOP opregion) support, which the ACPI ac device in the dsdt
relies on. This leads for example to a ADP1 power_supply which reports
itself as always online even if no mains are connected.

This commit adds an acpi_ac_unregister() function which native PMIC
drivers can call after successfully registering their own power_supply
to unregister the (potentially broken) ACPI-ac power_supply.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Fix capitalization in commit msg
-Minor style cleanups
---
 drivers/acpi/ac.c          | 43 ++++++++++++++++++++++++++++++++++++++-----
 include/linux/power/acpi.h |  6 ++++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index f71b756..b42affb 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -26,11 +26,13 @@
 #include <linux/types.h>
 #include <linux/dmi.h>
 #include <linux/delay.h>
+#include <linux/mutex.h>
 #ifdef CONFIG_ACPI_PROCFS_POWER
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #endif
 #include <linux/platform_device.h>
+#include <linux/power/acpi.h>
 #include <linux/power_supply.h>
 #include <linux/acpi.h>
 #include "battery.h"
@@ -422,17 +424,27 @@ static int acpi_ac_remove(struct acpi_device *device)
 	return 0;
 }
 
+enum init_state_enum { AC_NONE, AC_INITIALIZED, AC_EXITED };
+
+static enum init_state_enum init_state;
+static DEFINE_MUTEX(init_state_mutex);
+
 static int __init acpi_ac_init(void)
 {
-	int result;
+	int result, ret = -ENODEV;
 
 	if (acpi_disabled)
 		return -ENODEV;
 
+	/* Check if acpi_ac_unregister() got called before _init() */
+	mutex_lock(&init_state_mutex);
+	if (init_state != AC_NONE)
+		goto out_unlock;
+
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_ac_dir = acpi_lock_ac_dir();
 	if (!acpi_ac_dir)
-		return -ENODEV;
+		goto out_unlock;
 #endif
 
 
@@ -441,18 +453,39 @@ static int __init acpi_ac_init(void)
 #ifdef CONFIG_ACPI_PROCFS_POWER
 		acpi_unlock_ac_dir(acpi_ac_dir);
 #endif
-		return -ENODEV;
+		goto out_unlock;
 	}
 
-	return 0;
+	init_state = AC_INITIALIZED;
+	ret = 0;
+
+out_unlock:
+	mutex_unlock(&init_state_mutex);
+	return ret;
 }
 
-static void __exit acpi_ac_exit(void)
+void acpi_ac_unregister(void)
 {
+	/* Check if _init() is done and only do unregister once */
+	mutex_lock(&init_state_mutex);
+	if (init_state != AC_INITIALIZED)
+		goto out_exit;
+
 	acpi_bus_unregister_driver(&acpi_ac_driver);
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_unlock_ac_dir(acpi_ac_dir);
 #endif
+
+out_exit:
+	init_state = AC_EXITED;
+	mutex_unlock(&init_state_mutex);
 }
+EXPORT_SYMBOL_GPL(acpi_ac_unregister);
+
+static void __exit acpi_ac_exit(void)
+{
+	acpi_ac_unregister();
+}
+
 module_init(acpi_ac_init);
 module_exit(acpi_ac_exit);
diff --git a/include/linux/power/acpi.h b/include/linux/power/acpi.h
index 83bdfb9..b50ae69 100644
--- a/include/linux/power/acpi.h
+++ b/include/linux/power/acpi.h
@@ -15,4 +15,10 @@ void acpi_battery_unregister(void);
 static inline void acpi_battery_unregister(void) {}
 #endif
 
+#if IS_ENABLED(CONFIG_ACPI_AC)
+void acpi_ac_unregister(void);
+#else
+static inline void acpi_ac_unregister(void) {}
+#endif
+
 #endif
-- 
2.9.3

[PATCH v2 3/4] power: supply: axp288_fuel_gauge: Unregister duplicate ACPI battery supply

[Hans de Goede]

On some systems with axp288 PMIC the dsdt also exports an ACPI battery
device (PNP0C0A device). This leads to there being 2 battery
power_supply-s registed like this:

~$ acpi
Battery 0: Charging, 84%, 00:49:39 until charged
Battery 1: Unknown, 0%, rate information unavailable

Note that the ACPI battery device does not work properly this is due
to Linux missing support for the vendor specific BMOP ACPI opregion.
But even if the ACPI battery where to function fine we still do not
want to export the same battery to userspace twice.

Therefor this commit calls acpi_battery_unregister() after successfully
registering the axp288-fuel-gauge power_supply to remove the duplicate
(and often broken) ACPI battery power_supply.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=194811
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Sergei Trusov <t.rus76@ya.ru>
---
 drivers/power/supply/Kconfig             | 2 ++
 drivers/power/supply/axp288_fuel_gauge.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index d978d75..ae8363b 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -242,6 +242,8 @@ config AXP288_CHARGER
 config AXP288_FUEL_GAUGE
 	tristate "X-Powers AXP288 Fuel Gauge"
 	depends on MFD_AXP20X && IIO
+	# if ACPI_BATTERY=m, this can't be 'y'
+	depends on ACPI_BATTERY || !ACPI_BATTERY
 	help
 	  Say yes here to have support for X-Power power management IC (PMIC)
 	  Fuel Gauge. The device provides battery statistics and status
diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index a8dcabc..b6efe7d 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -25,6 +25,7 @@
 #include <linux/workqueue.h>
 #include <linux/mfd/axp20x.h>
 #include <linux/platform_device.h>
+#include <linux/power/acpi.h>
 #include <linux/power_supply.h>
 #include <linux/iio/consumer.h>
 #include <linux/debugfs.h>
@@ -754,6 +755,8 @@ static int axp288_fuel_gauge_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	acpi_battery_unregister();
+
 	fuel_gauge_create_debugfs(info);
 	fuel_gauge_init_irq(info);
 	schedule_delayed_work(&info->status_monitor, STATUS_MON_DELAY_JIFFIES);
-- 
2.9.3

[PATCH v2 4/4] power: supply: axp288_charger: Unregister duplicate ACPI ac supply

[Hans de Goede]

On some systems with an axp288 PMIC the dsdt exports a non functional (*)
ACPI AC device (ACPI0003 device), which results in a Mains power_supply
which reports itself as being always online.

This commit calls acpi_ac_unregister() after successfully registering the
axp288_charger power_supply to remove the broken Mains power_supply.

*) It depends on a vendor specific BMOP ACPI opregion we do not implement

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/Kconfig          | 2 ++
 drivers/power/supply/axp288_charger.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index ae8363b..fe53edd 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -235,6 +235,8 @@ config CHARGER_AXP20X
 config AXP288_CHARGER
 	tristate "X-Powers AXP288 Charger"
 	depends on MFD_AXP20X && EXTCON_AXP288
+	# if ACPI_AC=m, this can't be 'y'
+	depends on ACPI_AC || !ACPI_AC
 	help
 	  Say yes here to have support X-Power AXP288 power management IC (PMIC)
 	  integrated charger.
diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index 5037d98..b3b5680 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/usb/otg.h>
 #include <linux/notifier.h>
+#include <linux/power/acpi.h>
 #include <linux/power_supply.h>
 #include <linux/property.h>
 #include <linux/mfd/axp20x.h>
@@ -882,6 +883,7 @@ static int axp288_charger_probe(struct platform_device *pdev)
 		}
 	}
 
+	acpi_ac_unregister();
 	return 0;
 }
 
-- 
2.9.3