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: <65xrumpt7ug5mqd7mkcknwyqmljrn4sofrqymg46bwvcmjoarr@wmt5fhsj3viz>
Date: Fri, 19 Sep 2025 00:32:18 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: André Draszik <andre.draszik@...aro.org>
Cc: Thomas Antoine <t.antoine@...ouvain.be>, 
	Conor Dooley <conor@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Peter Griffin <peter.griffin@...aro.org>, linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH v6 2/2] dt-bindings: power: supply: add support for
 MAX77759 fuel gauge

Hi,

On Thu, Sep 18, 2025 at 02:02:55PM +0100, André Draszik wrote:
> > On 9/15/25 7:31 PM, Conor Dooley wrote:
> > > On Mon, Sep 15, 2025 at 12:14:11PM +0200, Thomas Antoine via B4 Relay wrote:
> > > > From: Thomas Antoine <t.antoine@...ouvain.be>
> > > > 
> > > > The Maxim MAX77759 is a companion PMIC for USB Type-C. It contains
> > > > Battery Charger, Fuel Gauge, temperature sensors, USB Type-C Port
> > > > Controller (TCPC), NVMEM, and additional GPIO interfaces
> > > > 
> > > > Use max77759-fg compatible to avoid conflict with drivers for other
> > > > functions.
> > > > 
> > > > The battery node is used to pass the REPCAP and ICHGTERM values
> > > > needed for the initialization of the fuel gauge.
> > > > 
> > > > The nvmem cells are used to get initialization values and to backup
> > > > the learning and the number of cycles. It should work out of the box
> > > > with gs101-oriole and gs101-raven which were previously running
> > > > Android.
> > > > 
> > > > Signed-off-by: Thomas Antoine <t.antoine@...ouvain.be>
> > > > ---
> > > >  .../bindings/power/supply/maxim,max77759.yaml      | 78 ++++++++++++++++++++++
> > > >  1 file changed, 78 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max77759.yaml
> > > > b/Documentation/devicetree/bindings/power/supply/maxim,max77759.yaml
> > > > new file mode 100644
> > > > index 0000000000000000000000000000000000000000..4d45739fcaf26273ec57b60049d6d0421df38efb
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/power/supply/maxim,max77759.yaml
> > > > @@ -0,0 +1,78 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/power/supply/maxim,max77759.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Maxim Integrated MAX77759 fuel gauge
> > > > +
> > > > +maintainers:
> > > > +  - Thomas Antoine <t.antoine@...ouvain.be>
> > > > +
> > > > +allOf:
> > > > +  - $ref: power-supply.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: maxim,max77759-fg
> > > 
> > > Compatible doesn't match the filename, why?
> > > I assume the "fg" is fuel-gauge, but can this device be anything else?
> > 
> > The max77759 is a multifunction chip.
> > The following compatibles are already used for some of those functions:
> > - maxim,max77759 (for the pmic)
> > - maxim,max77759-gpio
> > - maxim,max77759-nvmem
> > - maxim,max77759-tcpci
> > 
> > The fuel gauge functionality that is added with this patch is very similar
> > to the functionality of the max1720x which is why the filename was chosen
> > to fit other maxim fuel gauge chips pattern.
> > 
> > Maybe it would be better to use the maxim,max77759-battery compatible to
> > match the filename? It would also fit with the already existing
> > maxim,max77705-battery and maxim,max77849-battery compatibles.
> 
> It also has a (battery) charger, a -battery compatible could be misleading.
> The datasheet refers to these subblocks as FG (for fuelgauge) and CHARGER.
> I'd suggest keeping those terms.
> 
> Additionally, the FG block can also measure temperature and battery ID. For
> those, a combination of (top-level) PMIC and FG registers are needed
> unfortunately. Which means that the FG should probably be an MFD child
> device, even though the FG itself doesn't depend on the top-level. Otherwise
> it'd be hard to access the top-level PMIC register.

My understanding is, that the FG has a dedicated I2C device address
and thus cannot be a simple MFD child of the PMIC. Did I misunderstood
that part?

Assuming I understood things correctly, I think I suggest to model
things like this for the battery temperature/ID:

i2c {
    pmic: pmic@42 {
        compatible = "maxim,max77759";
        ...

        pmic_adc: adc {
            compatible = "maxim,max77759-adc";
            ...
        };
    };

    fuel-gauge@43 {
        compatible = "maxim,max77759-fg";
        ...
        io-channels = <&pmic_adc MAX77759_ADC_BAT_TEMP>, <&pmic_adc MAX77759_ADC_BAT_ID>;
        io-channel-names = "temperature", "ID";
    };
};

Greetings,

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ