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

[Krzysztof Kozlowski]

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@xxxxxxx>
>>> ---
>>>
>>>  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