On Sun, Nov 11, 2018 at 4:22 AM Pavel Machek <pavel@xxxxxx> wrote: > > On Sun 2018-11-11 12:57:12, Hans de Goede wrote: > > Hi, > > > > On 11/7/18 5:53 AM, Daniel Drake wrote: > > >On Mon, Nov 5, 2018 at 1:19 AM Pavel Machek <pavel@xxxxxx> wrote: > > >>Plus, I don't think "100% charge" is right test for "battery full". At > > >>least on thinkpads, there's configuration option, and it is common > > >>_not_ to charge batterry above 95% or so (to increase its lifetime). > > > > > >Hans also touched on this area in his response: > > > > > >>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. > > > > > >I'm not fully convinced by this argument though. > > > > > >The ACPI spec is not very clear on what conditions you should apply to > > >decide when the battery is full. Instead, ACPI seems to provide a > > >pretty decent amount of data, and the decision about whether to > > >interpret that as "battery full" is left for consumers. > > > > Right, but in this case the "discharging" status bit is explicitly > > set, to me it feels wrong to report "full", when the firmware > > is reporting "discharging" IMHO, at best we are "not charging" > > (on AC, below the threshold where a new charge cycle starts) and > > that is what we are currently reporting. > > > > Anu heurstics to decide that "not charging" is close enough to full > > to report it as full to the user belongs in userspace IMHO. > > > > Anyways this ultimately is Rafael's call. If Rafael is ok with this > > patch then I would like to see Pavel's comment addressed and otherwise > > it is fine with me. > > If we can get to an agreement on this I'll send a v2 without division. > > Note that we will still often get the case where a laptop is charged, > > reports full, is unplugged for 5 minutes and then replugged and then > > reports a capacity of 97% combined with "not charging", so we will > > still need to fix userspace to handle this. > Yes, I agree that should be addressed in userspace, as it is pretty much a policy decision. > For the record, I don't think I'm okay with this. > > There's nothing special about 100% charge. > I don't agree there is nothing special about 100% charge. There is a separate state to represent battery full for a reason, which is the user wanting to know when their battery is 100% charged and not being discharged. > This changes userland ABI and I don't think it has good enough reasons to do that. > This only changes which state will be reported when the battery is 100% charged and not discharging, it does not introduce / remove any values. I don't think that is considered ABI change, and on other hardware like the Dell Latitude 5480, /sys/class/power_supply/<supply_name>/status already reports "Full" under these conditions. I still believe it is a bug that makes the ABI inconsistent across different hardware. -- João Paulo Rechi Vita http://about.me/jprvita
