linux-kernel - Re: [PATCH 2/4] power: supply: max17042_battery: use ModelCfg refresh on max17055

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]

Date:   Sun, 20 Mar 2022 13:18:49 +0100
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Sebastian Krzyszkowiak <sebastian.krzyszkowiak@...i.sm>,
        Hans de Goede <hdegoede@...hat.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Sebastian Reichel <sre@...nel.org>, linux-pm@...r.kernel.org
Cc:     Purism Kernel Team <kernel@...i.sm>, Rob Herring <robh@...nel.org>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/4] power: supply: max17042_battery: use ModelCfg refresh
 on max17055

On 18/03/2022 20:58, Sebastian Krzyszkowiak wrote:
> On piątek, 18 marca 2022 09:22:16 CET Krzysztof Kozlowski wrote:
>> On 18/03/2022 01:10, Sebastian Krzyszkowiak wrote:
>>> Unlike other models, max17055 doesn't require cell characterization
>>> data and operates on smaller amount of input variables (DesignCap,
>>> VEmpty, IChgTerm and ModelCfg). Input data can already be filled in
>>> by max17042_override_por_values, however model refresh bit has to be
>>> set after adjusting input variables in order to make them apply.
>>>
>>> Signed-off-by: Sebastian Krzyszkowiak <sebastian.krzyszkowiak@...i.sm>
>>> ---
>>>
>>>  drivers/power/supply/max17042_battery.c | 73 +++++++++++++++----------
>>>  include/linux/power/max17042_battery.h  |  3 +
>>>  2 files changed, 48 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/power/supply/max17042_battery.c
>>> b/drivers/power/supply/max17042_battery.c index
>>> c019d6c52363..c39250349a1d 100644
>>> --- a/drivers/power/supply/max17042_battery.c
>>> +++ b/drivers/power/supply/max17042_battery.c
>>> @@ -806,6 +806,13 @@ static inline void
>>> max17042_override_por_values(struct max17042_chip *chip)> 
>>>  	    (chip->chip_type == MAXIM_DEVICE_TYPE_MAX17055)) {
>>>  		
>>>  		max17042_override_por(map, MAX17047_V_empty, config-
>> vempty);
>>>  	
>>>  	}
>>>
>>> +
>>> +	if (chip->chip_type == MAXIM_DEVICE_TYPE_MAX17055) {
>>> +		max17042_override_por(map, MAX17055_ModelCfg, config-
>> model_cfg);
>>> +		// VChg is 1 by default, so allow it to be set to 0
>>
>> Consistent comment, so /* */
>>
>> I actually do not understand fully the comment and the code. You write
>> entire model_cfg to MAX17055_ModelCfg and then immediately do it again,
>> but with smaller mask. Why?
> 
> That's because VChg is 1 on POR, and max17042_override_por doesn't do anything 
> when value equals 0 - which means that if the whole config->model_cfg is 0, 
> VChg won't get unset (which is needed for 4.2V batteries).
> 
> This could actually be replaced with a single regmap_write.
> 

I got it now. But if config->model_cfg is 0, should VChg be unset?


Best regards,
Krzysztof