Re: [PATCH 2/2] lg-laptop: Move setting of battery charge limit to common location

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 2/19/22 14:54, Matan Ziv-Av wrote:
> For now leave also the driver specific location,
> with deprecated warning in documentation.
> 
> Signed-off-by: Matan Ziv-Av <matan@svgalib.org>
> ---
>  .../ABI/testing/sysfs-platform-lg-laptop      |  2 +-
>  .../admin-guide/laptops/lg-laptop.rst         |  2 +-
>  drivers/platform/x86/lg-laptop.c              | 57 ++++++++++++++++---
>  3 files changed, 51 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-lg-laptop b/Documentation/ABI/testing/sysfs-platform-lg-laptop
> index cf47749b19df..d226058049ce 100644
> --- a/Documentation/ABI/testing/sysfs-platform-lg-laptop
> +++ b/Documentation/ABI/testing/sysfs-platform-lg-laptop
> @@ -16,7 +16,7 @@ What:		/sys/devices/platform/lg-laptop/battery_care_limit
>  Date:		October 2018
>  KernelVersion:	4.20
>  Contact:	"Matan Ziv-Av <matan@svgalib.org>
> -Description:
> +Description: <deprecated> use /sys/class/power_supply/CMB0/charge_control_end_threshold
>          Maximal battery charge level. Accepted values are 80 or 100.
>  
>  What:		/sys/devices/platform/lg-laptop/fan_mode

We have some automatic scripts parsing these files now; and I'm not sure
if those will like this form.

So I've changed this to the following while merging it:

@@ -17,6 +17,7 @@ Date:		October 2018
 KernelVersion:	4.20
 Contact:	"Matan Ziv-Av <matan@svgalib.org>
 Description:
+        Deprecated use /sys/class/power_supply/CMB0/charge_control_end_threshold
         Maximal battery charge level. Accepted values are 80 or 100.
 
 What:		/sys/devices/platform/lg-laptop/fan_mode




Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans





> diff --git a/Documentation/admin-guide/laptops/lg-laptop.rst b/Documentation/admin-guide/laptops/lg-laptop.rst
> index 6fbe165dcd27..67fd6932cef4 100644
> --- a/Documentation/admin-guide/laptops/lg-laptop.rst
> +++ b/Documentation/admin-guide/laptops/lg-laptop.rst
> @@ -38,7 +38,7 @@ FN lock.
>  Battery care limit
>  ------------------
>  
> -Writing 80/100 to /sys/devices/platform/lg-laptop/battery_care_limit
> +Writing 80/100 to /sys/class/power_supply/CMB0/charge_control_end_threshold
>  sets the maximum capacity to charge the battery. Limiting the charge
>  reduces battery capacity loss over time.
>  


> diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c
> index a91847a551a7..332868b140ed 100644
> --- a/drivers/platform/x86/lg-laptop.c
> +++ b/drivers/platform/x86/lg-laptop.c
> @@ -17,6 +17,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/types.h>
>  
> +#include <acpi/battery.h>
> +
>  #define LED_DEVICE(_name, max, flag) struct led_classdev _name = { \
>  	.name           = __stringify(_name),   \
>  	.max_brightness = max,                  \
> @@ -458,14 +460,14 @@ static ssize_t fn_lock_show(struct device *dev,
>  	return sysfs_emit(buffer, "%d\n", status);
>  }
>  
> -static ssize_t battery_care_limit_store(struct device *dev,
> -					struct device_attribute *attr,
> -					const char *buffer, size_t count)
> +static ssize_t charge_control_end_threshold_store(struct device *dev,
> +						  struct device_attribute *attr,
> +						  const char *buf, size_t count)
>  {
>  	unsigned long value;
>  	int ret;
>  
> -	ret = kstrtoul(buffer, 10, &value);
> +	ret = kstrtoul(buf, 10, &value);
>  	if (ret)
>  		return ret;
>  
> @@ -486,9 +488,9 @@ static ssize_t battery_care_limit_store(struct device *dev,
>  	return -EINVAL;
>  }
>  
> -static ssize_t battery_care_limit_show(struct device *dev,
> -				       struct device_attribute *attr,
> -				       char *buffer)
> +static ssize_t charge_control_end_threshold_show(struct device *device,
> +						 struct device_attribute *attr,
> +						 char *buf)
>  {
>  	unsigned int status;
>  	union acpi_object *r;
> @@ -520,15 +522,52 @@ static ssize_t battery_care_limit_show(struct device *dev,
>  	if (status != 80 && status != 100)
>  		status = 0;
>  
> -	return sysfs_emit(buffer, "%d\n", status);
> +	return sysfs_emit(buf, "%d\n", status);
> +}
> +
> +static ssize_t battery_care_limit_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buffer)
> +{
> +	return charge_control_end_threshold_show(dev, attr, buffer);
> +}
> +
> +static ssize_t battery_care_limit_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buffer, size_t count)
> +{
> +	return charge_control_end_threshold_store(dev, attr, buffer, count);
>  }
>  
>  static DEVICE_ATTR_RW(fan_mode);
>  static DEVICE_ATTR_RW(usb_charge);
>  static DEVICE_ATTR_RW(reader_mode);
>  static DEVICE_ATTR_RW(fn_lock);
> +static DEVICE_ATTR_RW(charge_control_end_threshold);
>  static DEVICE_ATTR_RW(battery_care_limit);
>  
> +static int lg_battery_add(struct power_supply *battery)
> +{
> +	if (device_create_file(&battery->dev,
> +			       &dev_attr_charge_control_end_threshold))
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static int lg_battery_remove(struct power_supply *battery)
> +{
> +	device_remove_file(&battery->dev,
> +			   &dev_attr_charge_control_end_threshold);
> +	return 0;
> +}
> +
> +static struct acpi_battery_hook battery_hook = {
> +	.add_battery = lg_battery_add,
> +	.remove_battery = lg_battery_remove,
> +	.name = "LG Battery Extension",
> +};
> +
>  static struct attribute *dev_attributes[] = {
>  	&dev_attr_fan_mode.attr,
>  	&dev_attr_usb_charge.attr,
> @@ -711,6 +750,7 @@ static int acpi_add(struct acpi_device *device)
>  	led_classdev_register(&pf_device->dev, &tpad_led);
>  
>  	wmi_input_setup();
> +	battery_hook_register(&battery_hook);
>  
>  	return 0;
>  
> @@ -728,6 +768,7 @@ static int acpi_remove(struct acpi_device *device)
>  	led_classdev_unregister(&tpad_led);
>  	led_classdev_unregister(&kbd_backlight);
>  
> +	battery_hook_unregister(&battery_hook);
>  	wmi_input_destroy();
>  	platform_device_unregister(pf_device);
>  	pf_device = NULL;