Skip to content

Commit ef5899e

Browse files
tristan-googlefabiobaltieri
authored andcommitted
sensors: max17262: Error-check init I2C operations
The init function of the MAX17262 sensor doesn't check the return value of its I2C read and write functions. In case a read operation fails, the output variable is not updated but the driver proceeds anyways. This can cause unintended operation due to the potentially un-initialized memory, such as getting stuck in the polling loop on line max17262.c:245. Update the init function to abort and pass along the error code when I2C transactions fail. Signed-off-by: Tristan Honscheid <[email protected]>
1 parent 9aa0d0f commit ef5899e

File tree

1 file changed

+72
-19
lines changed

1 file changed

+72
-19
lines changed

drivers/sensor/max17262/max17262.c

Lines changed: 72 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ static int max17262_reg_read(const struct device *dev, uint8_t reg_addr,
3434

3535
rc = i2c_burst_read_dt(&cfg->i2c, reg_addr, i2c_data, 2);
3636
if (rc < 0) {
37-
LOG_ERR("Unable to read register");
37+
LOG_ERR("Unable to read register 0x%02x", reg_addr);
3838
return rc;
3939
}
4040
*valp = ((int16_t)i2c_data[1] << 8) | i2c_data[0];
@@ -225,14 +225,18 @@ static int max17262_gauge_init(const struct device *dev)
225225
{
226226
const struct max17262_config *const config = dev->config;
227227
int16_t tmp, hibcfg;
228+
int rc;
228229

229230
if (!device_is_ready(config->i2c.bus)) {
230231
LOG_ERR("Bus device is not ready");
231232
return -ENODEV;
232233
}
233234

234235
/* Read Status register */
235-
max17262_reg_read(dev, STATUS, &tmp);
236+
rc = max17262_reg_read(dev, STATUS, &tmp);
237+
if (rc) {
238+
return rc;
239+
}
236240

237241
if (!(tmp & STATUS_POR)) {
238242
/*
@@ -248,65 +252,114 @@ static int max17262_gauge_init(const struct device *dev)
248252
LOG_DBG("POR detected, setting custom device configuration...");
249253

250254
/** STEP 1 */
251-
max17262_reg_read(dev, FSTAT, &tmp);
255+
rc = max17262_reg_read(dev, FSTAT, &tmp);
256+
if (rc) {
257+
return rc;
258+
}
252259

253260
/* Do not continue until FSTAT.DNR bit is cleared */
254261
while (tmp & FSTAT_DNR) {
255262
k_sleep(K_MSEC(10));
256-
max17262_reg_read(dev, FSTAT, &tmp);
263+
rc = max17262_reg_read(dev, FSTAT, &tmp);
264+
if (rc) {
265+
return rc;
266+
}
257267
}
258268

259269
/** STEP 2 */
260270
/* Store original HibCFG value */
261-
max17262_reg_read(dev, HIBCFG, &hibcfg);
271+
rc = max17262_reg_read(dev, HIBCFG, &hibcfg);
272+
if (rc) {
273+
return rc;
274+
}
262275

263276
/* Exit Hibernate Mode step 1 */
264-
max17262_reg_write(dev, SOFT_WAKEUP, 0x0090);
277+
rc = max17262_reg_write(dev, SOFT_WAKEUP, 0x0090);
278+
if (rc) {
279+
return rc;
280+
}
281+
265282
/* Exit Hibernate Mode step 2 */
266-
max17262_reg_write(dev, HIBCFG, 0x0000);
283+
rc = max17262_reg_write(dev, HIBCFG, 0x0000);
284+
if (rc) {
285+
return rc;
286+
}
287+
267288
/* Exit Hibernate Mode step 3 */
268-
max17262_reg_write(dev, SOFT_WAKEUP, 0x0000);
289+
rc = max17262_reg_write(dev, SOFT_WAKEUP, 0x0000);
290+
if (rc) {
291+
return rc;
292+
}
269293

270294
/** STEP 2.1 --> OPTION 1 EZ Config (No INI file is needed) */
271295
/* Write DesignCap */
272-
max17262_reg_write(dev, DESIGN_CAP, config->design_cap);
296+
rc = max17262_reg_write(dev, DESIGN_CAP, config->design_cap);
297+
if (rc) {
298+
return rc;
299+
}
273300

274301
/* Write IChgTerm */
275-
max17262_reg_write(dev, ICHG_TERM, config->desired_charging_current);
302+
rc = max17262_reg_write(dev, ICHG_TERM, config->desired_charging_current);
303+
if (rc) {
304+
return rc;
305+
}
276306

277307
/* Write VEmpty */
278-
max17262_reg_write(dev, VEMPTY, ((config->empty_voltage / 10) << 7) |
279-
((config->recovery_voltage / 40) & 0x7F));
308+
rc = max17262_reg_write(dev, VEMPTY,
309+
((config->empty_voltage / 10) << 7) |
310+
((config->recovery_voltage / 40) & 0x7F));
311+
if (rc) {
312+
return rc;
313+
}
280314

281315
/* Write ModelCFG */
282316
if (config->charge_voltage > 4275) {
283-
max17262_reg_write(dev, MODELCFG, 0x8400);
317+
rc = max17262_reg_write(dev, MODELCFG, 0x8400);
284318
} else {
285-
max17262_reg_write(dev, MODELCFG, 0x8000);
319+
rc = max17262_reg_write(dev, MODELCFG, 0x8000);
320+
}
321+
322+
if (rc) {
323+
return rc;
286324
}
287325

288326
/*
289327
* Read ModelCFG.Refresh (highest bit),
290328
* proceed to Step 3 when ModelCFG.Refresh == 0
291329
*/
292-
max17262_reg_read(dev, MODELCFG, &tmp);
330+
rc = max17262_reg_read(dev, MODELCFG, &tmp);
331+
if (rc) {
332+
return rc;
333+
}
293334

294335
/* Do not continue until ModelCFG.Refresh == 0 */
295336
while (tmp & MODELCFG_REFRESH) {
296337
k_sleep(K_MSEC(10));
297-
max17262_reg_read(dev, MODELCFG, &tmp);
338+
rc = max17262_reg_read(dev, MODELCFG, &tmp);
339+
if (rc) {
340+
return rc;
341+
}
298342
}
299343

300344
/* Restore Original HibCFG value */
301-
max17262_reg_write(dev, HIBCFG, hibcfg);
345+
rc = max17262_reg_write(dev, HIBCFG, hibcfg);
346+
if (rc) {
347+
return rc;
348+
}
302349

303350
/** STEP 3 */
304351
/* Read Status register */
305-
max17262_reg_read(dev, STATUS, &tmp);
352+
rc = max17262_reg_read(dev, STATUS, &tmp);
353+
if (rc) {
354+
return rc;
355+
}
306356

307357
/* Clear PowerOnReset bit */
308358
tmp &= ~STATUS_POR;
309-
max17262_reg_write(dev, STATUS, tmp);
359+
rc = max17262_reg_write(dev, STATUS, tmp);
360+
if (rc) {
361+
return rc;
362+
}
310363

311364
return 0;
312365
}

0 commit comments

Comments
 (0)