linux-kernel - Re: [PATCH] ACPI / battery: Fix reporting "Not charging" when capacity is 100%

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+A7VXUBCp3i18bz+LQ+gVpf_UdZALU=UpHXcCQ8V-eTSwOkJQ@mail.gmail.com>
Date:   Tue, 6 Nov 2018 12:14:08 -0800
From:   João Paulo Rechi Vita <jprvita@...il.com>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
        Daniel Drake <drake@...lessm.com>,
        Sebastian Reichel <sebastian.reichel@...labora.co.uk>,
        LKML <linux-kernel@...r.kernel.org>, linux@...lessm.com,
        João Paulo Rechi Vita <jprvita@...lessm.com>
Subject: Re: [PATCH] ACPI / battery: Fix reporting "Not charging" when
 capacity is 100%

Hello Hans, thanks for your quick response,

On Sat, Nov 3, 2018 at 4:28 AM Hans de Goede <hdegoede@...hat.com> wrote:
>
> Hi,
>
> On 03-11-18 07:57, João Paulo Rechi Vita wrote:
> > Commit 19fffc8450d4378580a8f019b195c4617083176f fixed reporting
> > "Discharging" on some machines when AC was connected but the battery was
> > not charging. But now on these machines the battery status is reported
> > as "Not charging" even when the battery is fully charged.
> >
> > This commit takes the battery capacity into consideration when checking
> > if "Not charging" should be returned and "Full" is returned when the
> > capacity is 100%.
> >
> > Signed-off-by: João Paulo Rechi Vita <jprvita@...lessm.com>
>
> acpi_battery_handle_discharging() only gets called if the ACPI_BATTERY_STATE_DISCHARGING
> bit is set by the firmware in that case if we are not actually discharging
> returning POWER_SUPPLY_STATUS_NOT_CHARGING is the only correct thing to do,
> we should never return POWER_SUPPLY_STATUS_FULL when the
> ACPI_BATTERY_STATE_DISCHARGING bit is set.
>

I understand the reason behind your original patch, but can you
elaborate on why would it be wrong to return POWER_SUPPLY_STATUS_FULL
if the battery is full and we know it is not actually discharging?
IIUC that is the state returned on hw / firmware that behaves
correctly when there is AC power and the battery has 100% of charge.

> I was about to point you to the upower bug for upower not
> handling POWER_SUPPLY_STATUS_NOT_CHARGING as well as it
> could atm, but I see you've already found that and are working
> on fixing that. That is great, thank you.
>

Yes, I'm trying to fix this problem, but it touches different points
across the stack and there are different possible approaches for how
to present the "Not charging" state to the user. This patch is not
necessarily part of, although closely related to, the bigger problem
of better handling "Not charging". It actually addresses the problem
that the kernel returns "Not charging" when AC is present and the
battery is fully charged, which seems wrong to me, despite of "Not
charging" not being strictly defined anywhere (at least that I know
of).

> As for this kernel-side fix I do not believe that fixing thus in
> the kernel is the right thing to do. We try to stay away from
> heuristics using full_charge_capacity in the kernel since that
> is not really reliable / deterministic.
>

At first I thought that returning POWER_SUPPLY_STATUS_FULL when the
battery is full and AC is present was deterministic enough to have it
in the kernel. But I don't have any prior experience working with this
kind of hardware, so I may very well be wrong on this assumption. It
would be great to get some extra clarification though.

Thanks,
--
João Paulo Rechi Vita
http://about.me/jprvita