RE: [PATCH RESEND 1/1] power_supply: wilco_ec: Add permanent long life charging mode

[Wang, Crag]

> > +		Permanent Long Life: Last longer battery life, this mode
> > +			is programmed once in the factory. Switching to a
> > +			different mode is unavailable.
> 
> The documentation lacks one important aspect: What happens if I have a
> device where the factory did not program "Long Life"?
> I.e. what happens when
> 
> # cat /sys/class/power_supply/wilco-charger/charge_type
> Standard
> # echo "Long Life" > /sys/class/power_supply/wilco-charger/charge_type
> 
> Will the controller switch into permanent long life battery mode without any
> exit strategy?
>

The set_property attempt will convert "Long Life" to its index of charge mode
and send a EC command to Property ID 0x0710 for configuration. This try will be
denied by EC because "Long Life" mode must be set (enable/disable) through a
different PID that isn't publicly known. In the factory there's a proprietary tool
in use for PLL configuration.

In above example after the run the charge mode will remain unchanged i.e.: 
"Standard".

> >  What:		/sys/class/power_supply/wilco-
> charger/charge_control_start_threshold
> >  Date:		April 2019
> > diff --git a/drivers/power/supply/power_supply_sysfs.c
> > b/drivers/power/supply/power_supply_sysfs.c
> > index bc79560229b5..af3884015ad8 100644
> > --- a/drivers/power/supply/power_supply_sysfs.c
> > +++ b/drivers/power/supply/power_supply_sysfs.c
> > @@ -87,6 +87,7 @@ static const char * const
> POWER_SUPPLY_CHARGE_TYPE_TEXT[] = {
> >  	[POWER_SUPPLY_CHARGE_TYPE_STANDARD]	= "Standard",
> >  	[POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE]	= "Adaptive",
> >  	[POWER_SUPPLY_CHARGE_TYPE_CUSTOM]	= "Custom",
> > +	[POWER_SUPPLY_CHARGE_TYPE_LONGLIFE]	= "Permanent Long
> Life",
> 
> The "Permanent" part is specific to the Wilco EC, so I think it's better to avoid
> it in the generic API. I think it's better to use just "Long Life" (and keep the
> wilco specific sysfs Documentation, that Long Life configuration is
> permanent).

Agree with you, I will include this modification in the next patch.


> 
> -- Sebastian
>