Module Name: src
Committed By: thorpej
Date: Mon Dec 23 21:24:59 UTC 2019
Modified Files:
src/sys/dev/i2c: tps65217pmic.c
Log Message:
- No need to use I2C_F_POLL here.
- Refactor register read / write functions, splitting out i2c bus
acquire / release everywhere. This fixes what was almost certainly
a deadlock in the FDT regulator section of the code (acquire bus ->
read register, which again acquires bus -> locking against myself).
To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.15 src/sys/dev/i2c/tps65217pmic.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/i2c/tps65217pmic.c
diff -u src/sys/dev/i2c/tps65217pmic.c:1.14 src/sys/dev/i2c/tps65217pmic.c:1.15
--- src/sys/dev/i2c/tps65217pmic.c:1.14 Sun Nov 3 22:55:34 2019
+++ src/sys/dev/i2c/tps65217pmic.c Mon Dec 23 21:24:59 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: tps65217pmic.c,v 1.14 2019/11/03 22:55:34 jmcneill Exp $ */
+/* $NetBSD: tps65217pmic.c,v 1.15 2019/12/23 21:24:59 thorpej Exp $ */
/*-
* Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
#include "opt_fdt.h"
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: tps65217pmic.c,v 1.14 2019/11/03 22:55:34 jmcneill Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tps65217pmic.c,v 1.15 2019/12/23 21:24:59 thorpej Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -146,12 +146,14 @@ struct tps_reg_param {
bool is_xadj; /* voltage is adjusted externally */
uint16_t current_voltage; /* in mV */
-
};
static int tps65217pmic_match(device_t, cfdata_t, void *);
static void tps65217pmic_attach(device_t, device_t, void *);
+static int tps65217pmic_i2c_lock(struct tps65217pmic_softc *);
+static void tps65217pmic_i2c_unlock(struct tps65217pmic_softc *);
+
static uint8_t tps65217pmic_reg_read(struct tps65217pmic_softc *, uint8_t);
static void tps65217pmic_reg_write(struct tps65217pmic_softc *, uint8_t,
uint8_t);
@@ -398,11 +400,19 @@ tps65217pmic_power_monitor_init(struct t
intrmask = TPS65217PMIC_INT_USBM | TPS65217PMIC_INT_ACM |
TPS65217PMIC_INT_PBM;
+ if (tps65217pmic_i2c_lock(sc) != 0) {
+ aprint_error_dev(sc->sc_dev,
+ "failed to initialize power monitor\n");
+ return;
+ }
+
status = tps65217pmic_reg_read(sc, TPS65217PMIC_STATUS);
ppath = tps65217pmic_reg_read(sc, TPS65217PMIC_PPATH);
/* acknowledge and disregard whatever interrupt was generated earlier */
intr = tps65217pmic_reg_read(sc, TPS65217PMIC_INT);
+ tps65217pmic_i2c_unlock(sc);
+
sc->sc_usbstatus = status & TPS65217PMIC_STATUS_USBPWR;
sc->sc_acstatus = status & TPS65217PMIC_STATUS_ACPWR;
sc->sc_usbenabled = ppath & TPS65217PMIC_PPATH_USB_EN;
@@ -430,7 +440,14 @@ tps65217pmic_power_monitor_task(void *au
mutex_enter(&sc->sc_lock);
+ if (tps65217pmic_i2c_lock(sc) != 0) {
+ device_printf(sc->sc_dev,
+ "WARNING: unable to perform power monitor task.\n");
+ return;
+ }
status = tps65217pmic_reg_read(sc, TPS65217PMIC_STATUS);
+ tps65217pmic_i2c_unlock(sc);
+
usbstatus = status & TPS65217PMIC_STATUS_USBPWR;
acstatus = status & TPS65217PMIC_STATUS_ACPWR;
@@ -510,11 +527,19 @@ tps65217pmic_wled_init(struct tps65217pm
return;
}
+ if (tps65217pmic_i2c_lock(sc) != 0) {
+ device_printf(sc->sc_dev,
+ "WARNING: unable to configure LED\n");
+ return;
+ }
+
tps65217pmic_reg_write(sc, TPS65217PMIC_WLEDCTRL1, val);
tps65217pmic_reg_write(sc, TPS65217PMIC_WLEDCTRL2,
(brightness - 1) & TPS65217PMIC_WLEDCTRL2_DUTY);
val |= TPS65217PMIC_WLEDCTRL1_ISINK_EN;
tps65217pmic_reg_write(sc, TPS65217PMIC_WLEDCTRL1, val);
+
+ tps65217pmic_i2c_unlock(sc);
}
static void
@@ -523,10 +548,18 @@ tps65217pmic_reg_refresh(struct tps65217
int i;
struct tps_reg_param *c_reg;
+ if (tps65217pmic_i2c_lock(sc) != 0) {
+ device_printf(sc->sc_dev,
+ "WARNING: unable to refresh regulators\n");
+ return;
+ }
+
for (i = 0; i < NTPS_REG; i++) {
c_reg = &tps_regulators[i];
tps65217pmic_regulator_read_config(sc, c_reg);
}
+
+ tps65217pmic_i2c_unlock(sc);
}
/* Get version and revision of the chip. */
@@ -535,8 +568,16 @@ tps65217pmic_version(struct tps65217pmic
{
uint8_t chipid;
+ if (tps65217pmic_i2c_lock(sc) != 0) {
+ device_printf(sc->sc_dev,
+ "WARNING: unable to get chip ID\n");
+ return;
+ }
+
chipid = tps65217pmic_reg_read(sc, TPS65217PMIC_CHIPID);
+ tps65217pmic_i2c_unlock(sc);
+
sc->sc_version = chipid & TPS65217PMIC_CHIPID_VER_MASK;
sc->sc_revision = chipid & TPS65217PMIC_CHIPID_REV_MASK;
}
@@ -694,32 +735,45 @@ tps65217pmic_print_ppath(struct tps65217
aprint_normal("\n");
}
+static int
+tps65217pmic_i2c_lock(struct tps65217pmic_softc *sc)
+{
+ int error;
+
+ error = iic_acquire_bus(sc->sc_tag, 0);
+ if (error) {
+ device_printf(sc->sc_dev,
+ "unable to acquire i2c bus, error %d\n", error);
+ }
+ return error;
+}
+
+static void
+tps65217pmic_i2c_unlock(struct tps65217pmic_softc *sc)
+{
+ iic_release_bus(sc->sc_tag, 0);
+}
+
static uint8_t
tps65217pmic_reg_read(struct tps65217pmic_softc *sc, uint8_t reg)
{
uint8_t wbuf[2];
uint8_t rv;
- if (iic_acquire_bus(sc->sc_tag, I2C_F_POLL) != 0) {
- aprint_error_dev(sc->sc_dev, "cannot acquire bus for read\n");
- return 0;
- }
-
wbuf[0] = reg;
if (iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, wbuf,
- 1, &rv, 1, I2C_F_POLL)) {
+ 1, &rv, 1, 0)) {
aprint_error_dev(sc->sc_dev, "cannot execute operation\n");
- iic_release_bus(sc->sc_tag, I2C_F_POLL);
+ iic_release_bus(sc->sc_tag, 0);
return 0;
}
- iic_release_bus(sc->sc_tag, I2C_F_POLL);
return rv;
}
static void
-tps65217pmic_reg_write_unlocked(struct tps65217pmic_softc *sc,
+tps65217pmic_reg_write(struct tps65217pmic_softc *sc,
uint8_t reg, uint8_t data)
{
uint8_t wbuf[2];
@@ -728,40 +782,26 @@ tps65217pmic_reg_write_unlocked(struct t
wbuf[1] = data;
if (iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP, sc->sc_addr, NULL, 0,
- wbuf, 2, I2C_F_POLL)) {
+ wbuf, 2, 0)) {
aprint_error_dev(sc->sc_dev, "cannot execute I2C write\n");
}
}
-static void __unused
-tps65217pmic_reg_write(struct tps65217pmic_softc *sc, uint8_t reg, uint8_t data)
-{
-
- if (iic_acquire_bus(sc->sc_tag, I2C_F_POLL) != 0) {
- aprint_error_dev(sc->sc_dev, "cannot acquire bus for write\n");
- return;
- }
-
- tps65217pmic_reg_write_unlocked(sc, reg, data);
-
- iic_release_bus(sc->sc_tag, I2C_F_POLL);
-}
-
static void
tps65217pmic_reg_write_l2(struct tps65217pmic_softc *sc,
uint8_t reg, uint8_t data)
{
uint8_t regpw = reg ^ TPS65217PMIC_PASSWORD_XOR;
- if (iic_acquire_bus(sc->sc_tag, I2C_F_POLL) != 0) {
- aprint_error_dev(sc->sc_dev, "cannot acquire bus for write\n");
+
+ if (tps65217pmic_i2c_lock(sc))
return;
- }
- tps65217pmic_reg_write_unlocked(sc, TPS65217PMIC_PASSWORD, regpw);
- tps65217pmic_reg_write_unlocked(sc, reg, data);
- tps65217pmic_reg_write_unlocked(sc, TPS65217PMIC_PASSWORD, regpw);
- tps65217pmic_reg_write_unlocked(sc, reg, data);
- iic_release_bus(sc->sc_tag, I2C_F_POLL);
+ tps65217pmic_reg_write(sc, TPS65217PMIC_PASSWORD, regpw);
+ tps65217pmic_reg_write(sc, reg, data);
+ tps65217pmic_reg_write(sc, TPS65217PMIC_PASSWORD, regpw);
+ tps65217pmic_reg_write(sc, reg, data);
+
+ tps65217pmic_i2c_unlock(sc);
}
static void
@@ -946,7 +986,7 @@ tps65217reg_enable(device_t dev, bool en
uint8_t val;
int error;
- error = iic_acquire_bus(pmic_sc->sc_tag, I2C_F_POLL);
+ error = tps65217pmic_i2c_lock(pmic_sc);
if (error != 0)
return error;
@@ -959,7 +999,7 @@ tps65217reg_enable(device_t dev, bool en
regulator->is_enabled = enable;
- iic_release_bus(pmic_sc->sc_tag, I2C_F_POLL);
+ tps65217pmic_i2c_unlock(pmic_sc);
return 0;
}
@@ -972,13 +1012,13 @@ tps65217reg_set_voltage(device_t dev, u_
struct tps_reg_param *regulator = sc->sc_param;
int error;
- error = iic_acquire_bus(pmic_sc->sc_tag, I2C_F_POLL);
+ error = tps65217pmic_i2c_lock(pmic_sc);
if (error != 0)
return error;
error = tps65217pmic_set_volt(pmic_sc->sc_dev, regulator->name, min_uvol / 1000);
- iic_release_bus(pmic_sc->sc_tag, I2C_F_POLL);
+ tps65217pmic_i2c_unlock(pmic_sc);
return error;
}
