linux-kernel - Re: [PATCH v7 4/5] power: supply: max17040: Config alert SOC low level threshold from FDT

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: <20191129183340.hsjddxot7ocnxran@earth.universe>
Date:   Fri, 29 Nov 2019 19:33:40 +0100
From:   Sebastian Reichel <sebastian.reichel@...labora.com>
To:     Matheus Castello <matheus@...tello.eng.br>
Cc:     krzk@...nel.org, robh+dt@...nel.org, mark.rutland@....com,
        cw00.choi@...sung.com, b.zolnierkie@...sung.com,
        lee.jones@...aro.org, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 4/5] power: supply: max17040: Config alert SOC low
 level threshold from FDT

Hi,

On Wed, Nov 27, 2019 at 10:06:47PM -0300, Matheus Castello wrote:
> [...]
> > > @@ -256,14 +303,26 @@ static int max17040_probe(struct i2c_client *client,
> > > 
> > >   	/* check interrupt */
> > >   	if (client->irq) {
> > > -		int ret;
> > > -
> > > -		ret = max17040_enable_alert_irq(chip);
> > > -
> > > -		if (ret) {
> > > -			client->irq = 0;
> > > +		if (of_device_is_compatible(client->dev.of_node,
> > > +					    "maxim,max77836-battery")) {
> > > +			ret = max17040_set_low_soc_alert(client,
> > > +							 chip->low_soc_alert);
> > > +			if (ret) {
> > > +				dev_err(&client->dev,
> > > +					"Failed to set low SOC alert: err %d\n",
> > > +					ret);
> > > +				return ret;
> > > +			}
> > > +
> > > +			ret = max17040_enable_alert_irq(chip);
> > > +			if (ret) {
> > > +				client->irq = 0;
> > > +				dev_warn(&client->dev,
> > > +					 "Failed to get IRQ err %d\n", ret);
> > > +			}
> > > +		} else {
> > >   			dev_warn(&client->dev,
> > > -				 "Failed to get IRQ err %d\n", ret);
> > > +				 "Device not compatible for IRQ");
> > 
> > Something is odd here. Either this should be part of the first
> > patch ("max17040: Add IRQ handler for low SOC alert"), or both
> > device types support the IRQ and this check should be removed.
> 
> The first patch add the IRQ without the configuration of the low SoC alert,
> using the default state of charge level. This patch is working with
> registers to config the low state of charge level, so it was proposed to
> just try to write registers in the models compatible with that
> (maxim,max77836-battery).
> 
> Maybe join the first patch to this one, and let DT binding be the first
> patch on the series so we can already test compatible here, let me know what
> you think about it.

Assuming the !max77836 do not have any interrupt support, you can
just add the OF check in the first patch in "if (client->irq)", so
that it reads 

if (client->irq && of_device_is_compatible(...)) {
    ...
}

-- Sebastian

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