Hi,
Thank you for the reviews!
On 16-03-17 17:29, Andy Shevchenko wrote:
On Thu, 2017-03-16 at 17:15 +0100, Hans de Goede wrote:
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.
acpi -> ACPI
pmic -> PMIC
dsdt -> DSDT (perhaps not here, but just in case)
Will fix for v2, note for the rest of the series, to avoid
spamming everyone with a lot of comments I will just fix
everything and send a v2 unless I've a specific remark.
@@ -31,6 +31,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/suspend.h>
+#include <linux/mutex.h>
Keep in alphabetical order ?
I'm a fan of having headers in alphabetical order myself,
but if you look at the actual file, rather then the diff
context, you will see that this file uses random order.
#include <linux/acpi.h>
#include <linux/power_supply.h>
+#include <linux/power/acpi.h>
Ditto. (though I don't remember which is first _ or /)
+ /* Check if acpi_battery_unregister got called before _init()
*/
acpi_battery_unregister -> acpi_battery_unregister() ?
+ 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;
+ empty line here...
Both fixed for v2.
+out_unlock:
+ mutex_unlock(&init_state_mutex);
+
...instead of this ?
+++ b/include/linux/power/acpi.h
E.g. for GPIO we keep such things directly in linux/acpi.h. Does it make
sense to have separate one in this case?
I've taken include/acpi/video.h as example here. TBH I do not think
shoving everything acpi related into linux/acpi.h is a good idea.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
