Re: [PATCH v3 5/6] power: supply: Add support for mp2733 battery charger

[saravanan sekar <sravanhome@gmail.com>]

Hi Sebastian,

On 11/09/22 15:31, Sebastian Reichel wrote:
> Hi,
> 
> On Wed, Jun 15, 2022 at 04:53:56PM +0200, Saravanan Sekar wrote:
>> mp2733 is updated version of mp2629 battery charge management
>> which supports USB fast-charge and higher range of input voltage.
>>
>> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> ---
>> [...]
>>   	psy_cfg.drv_data = charger;
>> -	psy_cfg.attr_grp = mp2629_charger_sysfs_groups;
>> +	if (charger->chip_info->has_impedance)
>> +		psy_cfg.attr_grp = mp2629_charger_sysfs_groups;
>> +
>> +	if (charger->chip_info->has_fast_charge)
>> +		psy_cfg.attr_grp = mp2733_charger_sysfs_groups;
>> +
>>   	charger->battery = devm_power_supply_register(dev,
>>   					 &mp2629_battery_desc, &psy_cfg);
>>   	if (IS_ERR(charger->battery)) {
> 
> Instead of having has_impedance and has_fast_charge feature
> flag that are mutual exclusive, store the device type and
> use if/else or switch statement to chose the correct attr_grp.

these flags are not really mutual exclusive, limitation only for 
application between mp2629 and mp2629. In future another chipset on
same series shall have both or none, so I would consider to control 
flags with functionality rather than chipset !

Please feedback if still I have to consider your proposal.

> Other than that:
> 
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> -- Sebastian

Thanks,
Saravanan