Hi, Thanks for including the test output. Looks like everything is returned with the correct units now :) On Fri, Dec 13, 2024 at 12:08:10PM +0530, Bhavin Sharma wrote: > Adds initial support for the STC3117 fuel gauge. > > The driver provides functionality to monitor key parameters including: > - Voltage > - Current > - State of Charge (SOC) > - Temperature > - Status > > Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@xxxxxxxxxxxxxxxxx> > Signed-off-by: Bhavin Sharma <bhavin.sharma@xxxxxxxxxxxxxxxxx> > --- You probably want to document Co-authorship? In that case you should also add Co-developed-by: Hardevsinh Palaniya <hardevsinh.palaniya@xxxxxxxxxxxxxxxxx> in addition to the Signed-off-by. See Documentation/process/submitting-patches.rst for details. > [...] > +#define INVALID_TEMPERATURE 250 > [...] > + /* INIT state, wait for batt_current & temperature value available: */ > + if (ram_data.reg.state == STC3117_INIT && count_m > 4) { > + data->avg_voltage = data->voltage; > + data->avg_current = data->batt_current; > + ram_data.reg.state = STC3117_RUNNING; > + } > + > + if (ram_data.reg.state != STC3117_RUNNING) { > + data->batt_current = 0; > + data->temp = INVALID_TEMPERATURE; Please don't return arbitrary values when there is no data available. We have error codes for this. I suppose in your case -ENODATA makes most sense. > + } else { > + if (data->voltage < APP_CUTOFF_VOLTAGE) > + data->soc = 0; > + > + if (mode & STC3117_VMODE) { > + data->avg_current = 0; > + data->batt_current = 0; > + } > + } > [...] Otherwise looks good to me. Thanks, -- Sebastian
Attachment:
signature.asc
Description: PGP signature