Re: [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting

From: "Arthur D." <spinal.by@gmail.com>
To: "Tony Lindgren" <tony@atomide.com>
Cc: "Pavel Machek" <pavel@ucw.cz>,
	"Merlijn Wajer" <merlijn@wizzup.org>,
	sre@kernel.org, linux-pm@vger.kernel.org,
	linux-omap@vger.kernel.org
Subject: Re: [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting
Date: Mon, 16 Mar 2020 04:30:10 +0300	[thread overview]
Message-ID: <op.0hjs4kk2hxa7s4@supervisor.net28> (raw)
In-Reply-To: <20200315215949.GK37466@atomide.com>

Hi, Tony.

It seems like a misunderstanding here. There's no problem in detecting
if the charging is in progress. The green led is switched off and
the battery current sign is changed from "-" to "+" (which means
that the battery is being discharged). So there's no need in additional
checks. For cpcap-battery this situation seems like a battery stopped
charging. And it doesn't matter if that was a user who disconnected
the charger or it was done somewhere in a driver/firmware/hardware.

The problem is that the charging current cant get to the point <100 mA,
not talking about <50 mA. And that's why I set the value of 112 mA for
the end of charge current: to help the kernel to detect this plateau and
to stop the calibration cycle, so the userspace can get all the battery
parameters I mentioned in the previous mail.

Please note, that the behaviour I mentioned was observed only when the
conditions written in my last mail were met. The important one was:
> 2) the display backlight is off

Because when I unlocked the display the charging current was able
to go below 112 mA. Of course I couldn't rely on something like this:
the user should stay with backlight on to have the battery calibrated.
Think about it: waiting for the charging current to drop from 100 mA
to 50 mA can take dozens of minutes (it depends on the age of battery -
the older the battery the longer it will take), and the user should
force somehow the device to not switch off the display hightlight
until the battery is calibrated.

Of course it's unacceptable, so I decided to set the end of charge
current limit to 112 mA. Which allows the user to just put the device
on a table and to wait until it's fully charged without a need
to interfere the charging process with some action from the user.

--
Best regards, Spinal

>> Now about the chosen limits. For some reason the charging is
>> interrupted (and restarted after a while) when the following
>> conditions are met:
>> 1) the charging current is < 112 mA
>> 2) the display backlight is off
>>
>> This behaviour was observed in Maemo Leste with hildon-desktop
>> running. I tested these patches for several days, so I picked up
>> the parameters for optimal (from my point of view) work in practice
>> taking into account the current "features" of Droid 4 drivers.
>>
>> If we could somehow fix this behaviour (charging interruption),
>> I'd reconsider the end of charge current value to be 50 mA.
>
> Hmm well we do have two chargers, the usb charger and the
> unknown inductive charger for the pins on the back.
>
> It would be best to keep cpcap-battery.c independent of the
> chargers to avoid depndencies as the chargers do usually start
> charging briefly always when connected.
>
> Maybe just adding something like below would be enough of a check:
>
> static int
> cpcap_battery_get_counter_rate(struct cpcap_battery_ddata *ddata,
> 			       int poll_time_ms);
>
> And then based on the value being negative or positive you
> would know if it's charging or not. I guess we could then
> use this also for POWER_SUPPLY_PROP_CHARGE_NOW with poll_time_ms
> value of 0. I think the charge counter is configure to poll
> at 250 ms right now.