Skip to contentSkip to conversation

ACPI battery driver: odd usage of generic power supply class on battery removal

RFC In review Alan Jenkins 2 participants 3 messages May 10, 2009 – May 11, 2009 ACPI

lore
Share & tools
lore
#0 lore raw

RFC 1/1 ACPI battery driver: odd usage of generic power supply class on battery removal

Hi,

The ACPI battery driver does something strange with the generic power supply class, when the battery is removed from my laptop. Instead of clearing the "present" attribute of the class device, it removes the entire class device.

Can this be corrected, or is there a specific reason for it?

Patch follows for illustration purposes.

If this can be changed, I'd also try to cleanup acpi_battery_update(). I don't think it's right that CONFIG_ACPI_SYSFS_POWER affects the control flow. When it's disabled, acpi_battery_present() is not tested before calling acpi_battery_get_state(). It might not cause any problems, but it is evil.

drivers/acpi/battery.c +0 -0
index 69cbc57..050444a 100644
@@ -482,6 +482,11 @@ static int sysfs_add_battery(struct acpi_battery *battery)
	return device_create_file(battery->bat.dev, &alarm_attr);
}

+static void sysfs_update_battery(struct acpi_battery *battery) +{
+ power_supply_changed(&battery->bat);
+}
+
static void sysfs_remove_battery(struct acpi_battery *battery)

{
	if (!battery->bat.dev)
@@ -500,7 +505,7 @@ static int acpi_battery_update(struct acpi_battery *battery)
		return result;
#ifdef CONFIG_ACPI_SYSFS_POWER
	if (!acpi_battery_present(battery)) {
-		sysfs_remove_battery(battery);
+		sysfs_update_battery(battery);
		battery->update_time = 0;
		return 0;
	}
@@ -513,8 +518,7 @@ static int acpi_battery_update(struct acpi_battery *battery)
		acpi_battery_init_alarm(battery);
	}

#ifdef CONFIG_ACPI_SYSFS_POWER
- if (!battery->bat.dev)
- sysfs_add_battery(battery);
+ sysfs_update_battery(battery);

#endif
	return acpi_battery_get_state(battery);
}
#1 lore raw

I also agree with that.

For example what would happen if I remove the battery, and then boot the laptop?

In this case nobody could know my system has a battery. Currently that makes gpm to hide 'battery' tab.

Best regards,
Maxim Levitsky

#2 lore raw
In reply to Alan Jenkins
Quoted 9 lines · Alan Jenkins

Hi,

The ACPI battery driver does something strange with the generic power supply class, when the battery is removed from my laptop. Instead of clearing the "present" attribute of the class device, it removes the entire class device.

Can this be corrected, or is there a specific reason for it?

Quoted 9 lines

Patch follows for illustration purposes.

If this can be changed, I'd also try to cleanup acpi_battery_update(). I don't think it's right that CONFIG_ACPI_SYSFS_POWER affects the control flow. When it's disabled, acpi_battery_present() is not tested before calling acpi_battery_get_state(). It might not cause any problems, but it is evil.

Ignore that patch, it won't work :-). I have correct code prepared now though.

ACPI: battery: register power_supply subdevice only when battery is
present

Make sure no power_supply object is present unless we actualy detect presence of battery. This fixes ghost batteries detected by HAL

Ok, I found the original discussion
<http://lkml.indiana.edu/hypermail/linux/kernel/0710.3/2001.html>. At that point, the battery driver tried to change the set of properties dynamically. If the battery was not present, there was a bug that caused the set of properties to be left empty.

But it doesn't do that right now. When I tested my (corrected) patches, the set of properties remained constant, "present" returned "0", and all the other attributes returned -ENODEV.

So I think we can revisit this and fix it the right way this time :-). I just need to write the descriptions, then I'll submit the patches for review.

Thanks
Alan