Re: [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices

[carl@uvos.xyz]

On Apr 24, 2021 08:47, Tony Lindgren <tony@atomide.com> wrote:
>
> Hi, 
>
> * Carl Philipp Klemm <philipp@uvos.xyz> [210423 12:55]: 
> > +static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata) 
> > +{ 
> > + struct nvmem_device *nvmem; 
> > + u8 battery_id = 0; 
> > + 
> > + ddata->check_nvmem = false; 
> > + 
> > + nvmem = nvmem_device_find(NULL, &cpcap_battery_match_nvmem); 
> > + if (IS_ERR_OR_NULL(nvmem)) { 
> > + ddata->check_nvmem = true; 
> > + if (!ddata->no_nvmem_warned) { 
> > + dev_info(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n"); 
> > + ddata->no_nvmem_warned = true; 
> > + } 
>
> Folks are also using the device with no battery at all to have it directly 
> connected to the power supply. This is handy for remotely power cycling 
> the device, and also for measuring power consumption with a bench power 
> supply. So by default I think we should continue assuming no battery is 
> inserted rather than assume a generic battery is inserted. 
>
> How about require configuring the undetected battery parameters via 
> /sys/class/power_supply to indicate a non-standard battery is inserted? 
>
> At least battery type, capacity, and voltage can depend on the generic 
> battery inserted. 
>
> Other than that, the NVRAM changes look nice to me. 
>
> Regards, 
>
> Tony 

The battery inserted property is still based on the presence of a thermistor, so I don't see how this patch changes the bevior with regards to this use case at all except for that info print. Previously the battery information struct was simply set to the values expected from eb41 no matter what.