linux-kernel - Re: [PATCH 1/3] power: supply: add battery driver for netronix ec

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] [day] [month] [year] [list]

Message-ID: <0087cd4f-4e85-4cd0-8732-cd5b175e9eaa@jm0.eu>
Date: Sun, 28 Dec 2025 00:34:20 +0100
From: "Ing. Josua Mayer" <josua.mayer@....eu>
To: Andreas Kemnade <andreas@...nade.info>
Cc: Jonathan Neuschäfer <j.neuschaefer@....net>,
 Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Sebastian Reichel <sre@...nel.org>,
 Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, imx@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/3] power: supply: add battery driver for netronix ec

Am 27.12.25 um 22:38 schrieb Andreas Kemnade:
> On Sat, 27 Dec 2025 17:28:13 +0100
> Josua Mayer <josua.mayer@....eu> wrote:
> 
>> Implement a simple battery driver for monitoring voltage with the
>> netronix embedded controller found in certain ebook readers.
>>
>> Signed-off-by: Josua Mayer <josua.mayer@....eu>
> 
> This also produces a value somehow depending on battery voltage
> on the Tolino vision.
Good!

> [...]
>> diff --git a/drivers/mfd/ntxec.c b/drivers/mfd/ntxec.c
>> index 08c68de0f01bc..d5059b8862aa8 100644
>> --- a/drivers/mfd/ntxec.c
>> +++ b/drivers/mfd/ntxec.c
>> @@ -139,6 +139,7 @@ static const struct regmap_config regmap_config = {
>> static const struct mfd_cell ntxec_subdev[] = {
>> 	{ .name = "ntxec-rtc" },
>> 	{ .name = "ntxec-pwm" },
>> +	{ .name = "ntxec-battery" },
>> };
>>
>> static const struct mfd_cell ntxec_subdev_pwm[] = {
> 
> I think that should be a separate patch for mfd.
Okay

> [...]
>> +	switch (psp) {
>> +		case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>> +			ret = regmap_read(priv->ec->regmap, NTXEC_REG_READ_BATTERY, &value);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			/* ec value to microvolt conversion:
>> +			 * vendor kernel source suggests linear behaviour from 3V to 4.2V
>> +			 * with readings 767 to 1023; each increment represents 4687,5uV.
>> +			 * adjust 3V boundary slightly to report exactly 4.2V when full.
>> +			 */
>> +			val->intval = 2999872 + (value - 767) * 4688;
>> +			break;
> I find this code both in some kobo 2.6.35.3 code and on the tolino 3.0.35:
> 
>          const unsigned short battGasgauge[] = {
>          //      3.0V, 3.1V, 3.2V, 3.3V, 3.4V, 3.5V, 3.6V, 3.7V, 3.8V, 3.9V, 4.0V, 4.1V, 4.2V,
> //               743,  767,  791,  812,  835,  860,  885,  909,  935,  960,  985, 1010, 1023,
>                   767,  791,  812,  833,  852,  877,  903,  928,  950,  979,  993, 1019, 1023,
>          };
> 
> This does not look very linear... We have offsets
> 24
> 21
> 21
> 19
> 25
> 26
> 25
> 22
> 29
> 14
> 26
> 4
> 
> Do you have something looking more sane?
No, I based on the same but simplified it.
> No idea what should produce such flaky offsets besides of
> improper measurements. At least that should be commented.
> And why do these tables exist at all?
> 
> Hmm, the more weird thing is that these voltages are translated linearly
> inot capacity.
Indeed - this is why I decided on a linear relationship ...
matching minimal and maixmal voltage as close as possible.

> So maybe they are just adjusted to have the capacity look
> more sane. That would explain the 4 units step between 4.1V and 4.2V.
4.1 is the full voltage of the lion battery if charger was disconnected.
However 4.2 I think is the final voltage reached while charging.

> Having linear adc result -> voltage and nonlinear voltage-> capcity would
> make more sense.
Indeed.
But if it was intended as percentage, then why would he register not 
just read from 0-100 :(
So I still guess it is some adc result.

> 
> looking at such code snippet like this:
> case POWER_SUPPLY_PROP_CAPACITY:
>                  if (POWER_SUPPLY_STATUS_NOT_CHARGING == g_ntx_bat_di->battery_status) {
>                          val->intval = 100;
>                          return 0;
>                  }
>                  value = ntx_up_battery_vol();
> [...]
>                                  val->intval  = 100 - ((4100000 - value)/7000);
> 
> 
> I am wondering whether we should just return capacity that way without
> calculating voltage...
I suppose it depends on whether it is more likely that the ec provides 
voltage, or a charge estimation.

br
Josua