lkml.org 
[lkml]   [2019]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH V2] cros_ec: Expose sysfile to force battery cut-off on shutdown.
Hi Guenter,

On Sun, Mar 3, 2019 at 5:13 PM Guenter Roeck <groeck@google.com> wrote:
>
> On Sun, Mar 3, 2019 at 4:54 PM RaviChandra Sadineni <ravisadineni@chromium.org> wrote:
>>
>> On chromebooks, power_manager daemon normally shutsdown(S5) the device
>> when the battery charge falls below 4% threshold. Chromeos EC then
>> normally spends an hour in S5 before hibernating. If the battery charge
>> falls below critical threshold in the mean time, EC does a battery cutoff
>> instead of hibernating. On some chromebooks, S5 is optimal enough
>> resulting in EC hibernating without battery cut-off. This results in
>> battery deep-discharging. This is a bad user experience as battery
>> has to trickle charge before booting when the A.C is plugged in the next
>> time.
>>
>> This patch exposes a sysfs file for an userland daemon to suggest EC if it
>> has to do a battery cut-off instead of hibernating when the system enters
>> S5 next time.
>>
>> Signed-off-by: RaviChandra Sadineni <ravisadineni@chromium.org>
>> ---
>> V2: Use kstrtobool() instead of kstrtou8() and add documentation.
>>
>> .../ABI/testing/sysfs-class-chromeos | 8 ++++
>> drivers/platform/chrome/cros_ec_sysfs.c | 37 +++++++++++++++++++
>> 2 files changed, 45 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-class-chromeos
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-chromeos b/Documentation/ABI/testing/sysfs-class-chromeos
>> new file mode 100644
>> index 000000000000..44d3cee6e7ae
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-chromeos
>> @@ -0,0 +1,8 @@
>> +What: /sys/class/chromeos/cros_ec/cutoff_at_shutdown
>> +Date: February 2019
>> +Contact: Ravi Chandra Sadineni <ravisadineni@chromium.org>
>> +Description:
>> + On writing '[Yy1]' to cutoff_at_shutdown property,
>> + kernel sends a host command to EC requesting battery
>> + cutoff on next shutdown. If AC is plugged in before
>> + next shutdown, EC resets this flag.
>> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
>> index f34a50121064..f5168ce8bfc7 100644
>> --- a/drivers/platform/chrome/cros_ec_sysfs.c
>> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
>> @@ -322,14 +322,51 @@ static ssize_t kb_wake_angle_store(struct device *dev,
>> return count;
>> }
>>
>> +/* Battery cut-off control */
>> +static ssize_t cutoff_at_shutdown_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct ec_params_battery_cutoff *param;
>> + struct cros_ec_command *msg;
>> + int ret;
>> + struct cros_ec_dev *ec = container_of(
>> + dev, struct cros_ec_dev, class_dev);
>> + bool cutoff_battery;
>> +
>> + if (kstrtobool(buf, &cutoff_battery))
>> + return -EINVAL;
>> +
>> + if (!cutoff_battery)
>> + return count;
>> +
>
>
> It is quite unusual to only be able to set this option, but not to be able to reset it. I think that warrants an explanation and reasoning. Also, if clearing the flag is not supported, I would expect an attempt to do so to return an error. There should also be an explanation in the code explaining why the attribute is write-only.
Added documentation for the reason behind making this flag write only.
Instead of taking a boolean value, made this attribute more generic
and took 'at-shutdown' as an option. This way it will be easy to
extend this sysfs api in the future (if EC exposes more attributes for
this host command).
>
> Guenter
>
>>
>> + msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
>> + if (!msg)
>> + return -ENOMEM;
>> +
>> + param = (struct ec_params_battery_cutoff *)msg->data;
>> + msg->command = EC_CMD_BATTERY_CUT_OFF + ec->cmd_offset;
>> + msg->version = 1;
>> + param->flags = EC_BATTERY_CUTOFF_FLAG_AT_SHUTDOWN;
>> + msg->outsize = sizeof(*param);
>> + msg->insize = 0;
>> + ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
>> + kfree(msg);
>> + if (ret < 0)
>> + return ret;
>> + return count;
>> +}
>> +
>> /* Module initialization */
>>
>> static DEVICE_ATTR_RW(reboot);
>> static DEVICE_ATTR_RO(version);
>> static DEVICE_ATTR_RO(flashinfo);
>> static DEVICE_ATTR_RW(kb_wake_angle);
>> +static DEVICE_ATTR_WO(cutoff_at_shutdown);
>>
>> static struct attribute *__ec_attrs[] = {
>> + &dev_attr_cutoff_battery_at_shutdown.attr,
>> &dev_attr_kb_wake_angle.attr,
>> &dev_attr_reboot.attr,
>> &dev_attr_version.attr,
>> --
>> 2.20.1
>>
Thanks,
Ravi

\
 
 \ /
  Last update: 2019-03-05 02:01    [W:0.065 / U:2.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site