On Mon, Aug 14, 2017 at 5:17 PM, Hao Wei Tee <angelsl@xxxxxxx> wrote: > This exposes the battery conservation mode present on some (?) IdeaPads. > The mode is set by calling ACPI method SBMC with argument 3 (on) or > 5 (off). Status is reported in bit 5 of the return value of ACPI method > GBMD. Thanks for an update, this looks pretty much good! Still some kind of nitpicks below. > +#define GBMD_CONSERVATION_BIT (5) > +enum { > + SBMC_CONSERVATION_ON = 3, > + SBMC_CONSERVATION_OFF = 5, Looks to me like GBMD = Get BM Data SBMC = Send BM Command Thus, I would leave BM only in the above (do you know by the way what BM stands for? > +static int method_gbmd(acpi_handle handle, unsigned long *ret) > +{ > + int val; > + int result = read_method_int(handle, "GBMD", &val); Reversed X-mas tree order in new code, please. > +static ssize_t show_ideapad_conservation(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + unsigned long result; > + struct ideapad_private *priv = dev_get_drvdata(dev); Ditto. > +} > + > +static ssize_t store_ideapad_conservation(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + int ret; > + bool state; > + struct ideapad_private *priv = dev_get_drvdata(dev); Ditto. > + > + ret = kstrtobool(buf, &state); > + if (ret) > + return ret; > + > + ret = method_sbmc(priv->adev->handle, state ? > + SBMC_CONSERVATION_ON : > + SBMC_CONSERVATION_OFF); > + if (ret < 0) > + return -EIO; > + return count; > +} > + > +static DEVICE_ATTR(conservation_mode, 0644, show_ideapad_conservation, > + store_ideapad_conservation); DEVICE_ATTR_RW() ? -- With Best Regards, Andy Shevchenko