Re: [PATCH 1/1] power: supply: max17042_battery: use VFSOC for capacity when no rsns

From: Henrik Grimler <henrik@grimler.se>
To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc: sre@kernel.org, linux-pm@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org,
	~postmarketos/upstreaming@lists.sr.ht, wolfgit@wiedmeyer.de,
	Sebastian Krzyszkowiak <sebastian.krzyszkowiak@puri.sm>,
	Hans de Goede <hdegoede@redhat.com>,
	Nikita Travkin <nikita@trvn.ru>
Subject: Re: [PATCH 1/1] power: supply: max17042_battery: use VFSOC for capacity when no rsns
Date: Mon, 27 Sep 2021 21:05:47 +0200	[thread overview]
Message-ID: <YVIWC5gehfh3TXX/@grimlerstat.localdomain> (raw)
In-Reply-To: <17ba5aaa-c456-2bb9-1680-ff0a302b412f@canonical.com>

On Fri, Sep 24, 2021 at 01:45:29PM +0200, Krzysztof Kozlowski wrote:
> Thanks for the patch. I found also my comments to Wolfgang's patch in
> 2016, which you resolve here but I have more. :)
> 
> I think my previous message about current sense are not correct. What is
> important is whether ModelGauge is being used/configured. For example
> none of DT platforms support it but ACPI might.
> There is incoming patch around it:
> https://lore.kernel.org/lkml/5702731.UytLkSCjyO@pliszka/
> 
> If you switch to VSSoc, I think you need to modify the SOC Alert Config
> in MiscCFG register (bits 0:1 to 0x1). Otherwise the alerts will be
> generated on different value.

So, 0x1 should correspond to AvSOC (i.e. non-filtered RepSOC), while
right now we write 0x3 (VFSOC) to MiscCFG for devices without current
sense [1]. Could you elaborate on why AvSOC should be used for alert
if we use VFSOC to get PROP_CAPACITY?

On this particular device it does not seem to make a difference what I
use for the SOC alert, the alert triggers all the time in any case
since RepSOC does not give an accurate value. Supposedly this happens
because ModelGauge configuration is incomplete, as you said. Looking
at the registers used by the ModelGauge it seems that only the
"characterization table" at 0x80 - 0xAF is missing. The rest (FullCap,
DesignCap, ICHGTerm, ..) are set to the same values as with vendor
kernel.

> Different topic:
> When touching Exynos-based boards (like Galaxy S3), please Cc me as
> well, even if I don't pop up in the maintainers.
> 
> For max17042 we need to Cc broader group of users, for example using it
> in ACPI platforms. The best is to pick the contributors.

Thanks for taking the time to explain this.

> Best regards,
> Krzysztof

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/power/supply/max17042_battery.c#n1092

Best regards,
Henrik Grimler