Hi,
On 9/2/22 20:00, Arvid Norlander wrote:
> This commit adds the ACPI battery hook which in turns adds the sysfs
> entries.
>
> Because the Toshiba laptops only support two modes (eco or normal), which
> in testing correspond to 80% and 100% we simply round to the nearest
> possible level when set.
>
> It is possible that Toshiba laptops other than the Z830 has different set
> points for the charging. If so, a quirk table could be introduced in the
> future for this. For now, assume that all laptops that support this feature
> work the same way.
>
> Tested on a Toshiba Satellite Z830.
>
> Signed-off-by: Arvid Norlander <lkml@xxxxxxxxx>
> ---
> drivers/platform/x86/toshiba_acpi.c | 97 +++++++++++++++++++++++++++++
> 1 file changed, 97 insertions(+)
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index c927d5d0f8cd..fc953d6bcb93 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -44,6 +44,7 @@
> #include <linux/rfkill.h>
> #include <linux/iio/iio.h>
> #include <linux/toshiba.h>
> +#include <acpi/battery.h>
> #include <acpi/video.h>
>
> MODULE_AUTHOR("John Belmonte");
> @@ -2981,6 +2982,92 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
> return 0;
> }
>
> +
> +/* ACPI battery hooking */
> +static ssize_t charge_control_end_threshold_show(struct device *device,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + u32 state;
> + int status;
> +
> + if (toshiba_acpi == NULL) {
> + pr_err("Toshiba ACPI object invalid\n");
> + return -ENODEV;
> + }
These and the other (toshiba_acpi == NULL) checks are not necessary,
battery_hook_register() is only called after setting toshiba_acpi to non NULL
and battery_hook_unregister() is called before setting it NULL again,
so toshiba_acpi can never be NULL when the callbacks run.
I have removed all the NULL checks while merging this.
> +
> + status = toshiba_battery_charge_mode_get(toshiba_acpi, &state);
> +
> + if (status != 0)
> + return status;
> +
> + if (state == 1)
> + return sprintf(buf, "80\n");
> + else
> + return sprintf(buf, "100\n");
> +}
> +
> +static ssize_t charge_control_end_threshold_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + u32 value;
> + int rval;
> +
> + if (toshiba_acpi == NULL) {
> + pr_err("Toshiba ACPI object invalid\n");
> + return -ENODEV;
> + }
> +
> + rval = kstrtou32(buf, 10, &value);
> + if (rval)
> + return rval;
> +
> + if (value < 1 || value > 100)
> + return -EINVAL;
> + rval = toshiba_battery_charge_mode_set(toshiba_acpi,
> + (value < 90) ? 1 : 0);
> + if (rval < 0)
> + return rval;
> + else
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(charge_control_end_threshold);
> +
> +static struct attribute *toshiba_acpi_battery_attrs[] = {
> + &dev_attr_charge_control_end_threshold.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(toshiba_acpi_battery);
> +
> +static int toshiba_acpi_battery_add(struct power_supply *battery)
> +{
> + if (toshiba_acpi == NULL) {
> + pr_err("Init order issue\n");
> + return -ENODEV;
> + }
> + if (!toshiba_acpi->battery_charge_mode_supported)
> + return -ENODEV;
> + if (device_add_groups(&battery->dev, toshiba_acpi_battery_groups))
> + return -ENODEV;
> + return 0;
> +}
> +
> +static int toshiba_acpi_battery_remove(struct power_supply *battery)
> +{
> + device_remove_groups(&battery->dev, toshiba_acpi_battery_groups);
> + return 0;
> +}
> +
> +static struct acpi_battery_hook battery_hook = {
> + .add_battery = toshiba_acpi_battery_add,
> + .remove_battery = toshiba_acpi_battery_remove,
> + .name = "Toshiba Battery Extension",
> +};
> +
> static void print_supported_features(struct toshiba_acpi_dev *dev)
> {
> pr_info("Supported laptop features:");
> @@ -3063,6 +3150,9 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
> rfkill_destroy(dev->wwan_rfk);
> }
>
> + if (dev->battery_charge_mode_supported)
> + battery_hook_unregister(&battery_hook);
> +
battery_hook_[un]register() call code from the acpi_battery
kernel code/module. To make sure those symbols are actually available
we need to add: "depends on ACPI_BATTERY" to config ACPI_TOSHIBA
in Kconfig. I have done this while merging this.
Regards,
Hans
> if (toshiba_acpi)
> toshiba_acpi = NULL;
>
> @@ -3246,6 +3336,13 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>
> toshiba_acpi = dev;
>
> + /*
> + * As the battery hook relies on the static variable toshiba_acpi being
> + * set, this must be done after toshiba_acpi is assigned.
> + */
> + if (dev->battery_charge_mode_supported)
> + battery_hook_register(&battery_hook);
> +
> return 0;
>
> error:
