Re: [PATCH v2 1/2] acpi: battery: Query Battery charge time ("_BCT") method

[Rafael J. Wysocki (Intel)]

On Wed, Sep 23, 2026 at 10:28 AM Umang Jain <uajain@xxxxxxxxxx> wrote:
>
> Add plumbing for querying the Battery Charge Time ("_BCT") method.
> If present, set ACPI_BATTERY_BCT_PRESENT in battery flags to denote
> that the method is supported in ACPI.
>
> Signed-off-by: Umang Jain <uajain@xxxxxxxxxx>
> ---
>  drivers/acpi/battery.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 670853ec3a4d..a1b7a821abaf 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -76,6 +76,7 @@ MODULE_DEVICE_TABLE(acpi, battery_device_ids);
>  enum {
>         ACPI_BATTERY_ALARM_PRESENT,
>         ACPI_BATTERY_XINFO_PRESENT,
> +       ACPI_BATTERY_BCT_PRESENT,
>         ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY,
>         /* On Lenovo Thinkpad models from 2010 and 2011, the power unit
>          * switches between mWh and mAh depending on whether the system
> @@ -110,6 +111,7 @@ struct acpi_battery {
>
>         struct mutex property_lock; /* Protects properties below. */
>         unsigned long update_time;
> +       unsigned long long charge_time;

This doesn't need to be unsigned long long.  As per the spec, unsigned
int should be sufficient.

>         int revision;
>         int rate_now;
>         int capacity_now;
> @@ -613,6 +615,10 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
>         int result = 0;
>         acpi_status status = 0;
>         struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +       union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> +       struct acpi_object_list args = { 1, &arg0 };
> +
> +       arg0.integer.value = 100;

Please move the definitions of the variables related to _BCT
evaluation to the code block where it gets evaluated.

>
>         lockdep_assert_held(&battery->property_lock);
>
> @@ -638,6 +644,15 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
>         battery->update_time = jiffies;
>         kfree(buffer.pointer);
>
> +       if (test_bit(ACPI_BATTERY_BCT_PRESENT, &battery->flags)) {
> +               status = acpi_evaluate_integer(battery->device->handle, "_BCT",
> +                                              &args, &battery->charge_time);

Currently, you only need to evaluate _BCT to implement the quirk in
the second patch AFAICS, so it is not useful to evaluate _BCT if the
quirk is not going to be used.  Any chance to rearrange the code to
avoid evaluating _BCT when it is not needed?

> +       if (ACPI_FAILURE(status))
> +               acpi_handle_info(battery->device->handle,
> +                                "_BCT evaluation failed: %s",
> +                                acpi_format_exception(status));

Missing indentation.

> +       }
> +
>         /* For buggy DSDTs that report negative 16-bit values for either
>          * charging or discharging current and/or report 0 as 65536
>          * due to bad math.
> @@ -1083,6 +1098,8 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume)
>                         if (result)
>                                 return result;
>                         acpi_battery_init_alarm(battery);
> +                       if (acpi_has_method(battery->device->handle, "_BCT"))
> +                               set_bit(ACPI_BATTERY_BCT_PRESENT, &battery->flags);
>                 }
>
>                 result = acpi_battery_get_state(battery);
> --