linux-kernel - Re: [PATCH 1/4] power: supply: add support for max77759 fuel gauge

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: <16f9b5f8aed79e9313b4638512896743fa5a8d6d.camel@linaro.org>
Date: Tue, 03 Dec 2024 11:02:20 +0000
From: André Draszik <andre.draszik@...aro.org>
To: Thomas Antoine <t.antoine@...ouvain.be>, Sebastian Reichel
 <sre@...nel.org>,  Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,  Dimitri Fedrau
 <dima.fedrau@...il.com>, Catalin Marinas <catalin.marinas@....com>, Will
 Deacon <will@...nel.org>,  Peter Griffin <peter.griffin@...aro.org>, Alim
 Akhtar <alim.akhtar@...sung.com>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-samsung-soc@...r.kernel.org
Subject: Re: [PATCH 1/4] power: supply: add support for max77759 fuel gauge

On Tue, 2024-12-03 at 11:11 +0100, Thomas Antoine wrote:
> On 12/3/24 10:31, André Draszik wrote:
> > On Tue, 2024-12-03 at 10:08 +0100, Thomas Antoine wrote:
> > > On 12/3/24 07:47, André Draszik wrote:
> > > > > From: Thomas Antoine <t.antoine@...ouvain.be>
> > > > > 
[...]

> > > > >  /*
> > > > >   * Model Gauge M5 Algorithm output register
> > > > >   * Volatile data (must not be cached)
> > > > > @@ -369,6 +387,8 @@ static int max1720x_battery_get_property(struct
> > > > > power_supply *psy,
> > > > >                       val->strval = max17201_model;
> > > > >               else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205)
> > > > >                       val->strval = max17205_model;
> > > > > +             else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX77759)
> > > > > +                     val->strval = max77759_model;
> > > > >               else
> > > > 
> > > > This is a 16 bit register, and while yes, MAX172XX_DEV_NAME_TYPE_MASK only
> > > > cares about the bottom 4 bits, the register is described as 'Firmware
> > > > Version Information'.
> > > > 
> > > > But maybe it's ok to do it like that, at least for now.
> > > 
> > > I thought this method would be ok as long as there is no collision on
> > > values. I hesitated to change the model evaluation method based on chip
> > > model, where the max77759 would thus have an hard-coded value and the
> > > max1720x would still evaluate the register value. I did not do it because
> > > it led to a lot more changes for no difference.
> > 
> > Downstream uses the upper bits for max77759:
> > https://android.googlesource.com/kernel/google-modules/bms/+/refs/heads/android-gs-raviole-mainline/max_m5.h#135
> > 
> > I don't know what the original max17201/5 report in this register
> > for those bits, though. Given for max77759 this register returns
> > the firmware version, I assume the lower bits can change.
> 
> Based on this datasheet of the max1720x, the upper bits are the revision
> and the four lower bits are device. So it could change.
> https://www.analog.com/media/en/technical-documentation/data-sheets/MAX17201-MAX17215.pdf#MAX17201%20DS.indd%3A.213504%3A15892
> 
> If the four lower bits are not always 0 for the max77759 then I guess it
> is necessary to change this as it wouldn't work with all max77759.

Maybe the best way forward is to go by the compatible (from DT), and
if max77759 to then print a warning if the upper bits are != 0x62 and
!= 0x63. And maybe even refuse to load in that case.

Cheers,
Andre'