Module Name: src
Committed By: thorpej
Date: Sun Dec 22 23:40:49 UTC 2019
Modified Files:
src/sys/arch/arm/nvidia: tegra_i2c.c
Log Message:
Use a separate lock (not the i2c bus lock) to synchronize with the
interrupt handler. This in all liklihood fixes a deadlock bug that
necessitated forcing I2C_F_POLL in tegra_i2c_exec() (someone who has
the hardware should test removing that line).
Also includes the changes for:
Cleanup i2c bus acquire / release, centralizing all of the logic into
iic_acquire_bus() / iic_release_bus(). "acquire" and "release" hooks
no longer need to be provided by back-end controller drivers (only if
they need special handling, e.g. powering on the i2c controller).
This results in the removal of a bunch of rendundant code from each
back-end controller driver.
Assert that we are not in hard interrupt context in iic_acquire_bus(),
iic_exec(), and iic_release_bus().
To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/arch/arm/nvidia/tegra_i2c.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/arch/arm/nvidia/tegra_i2c.c
diff -u src/sys/arch/arm/nvidia/tegra_i2c.c:1.22 src/sys/arch/arm/nvidia/tegra_i2c.c:1.23
--- src/sys/arch/arm/nvidia/tegra_i2c.c:1.22 Tue Sep 25 22:23:22 2018
+++ src/sys/arch/arm/nvidia/tegra_i2c.c Sun Dec 22 23:40:49 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: tegra_i2c.c,v 1.22 2018/09/25 22:23:22 jmcneill Exp $ */
+/* $NetBSD: tegra_i2c.c,v 1.23 2019/12/22 23:40:49 thorpej Exp $ */
/*-
* Copyright (c) 2015 Jared D. McNeill <jmcne...@invisible.ca>
@@ -27,7 +27,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: tegra_i2c.c,v 1.22 2018/09/25 22:23:22 jmcneill Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tegra_i2c.c,v 1.23 2019/12/22 23:40:49 thorpej Exp $");
#include <sys/param.h>
#include <sys/bus.h>
@@ -63,15 +63,13 @@ struct tegra_i2c_softc {
u_int sc_cid;
struct i2c_controller sc_ic;
- kmutex_t sc_lock;
- kcondvar_t sc_cv;
+ kmutex_t sc_intr_lock;
+ kcondvar_t sc_intr_wait;
};
static void tegra_i2c_init(struct tegra_i2c_softc *);
static int tegra_i2c_intr(void *);
-static int tegra_i2c_acquire_bus(void *, int);
-static void tegra_i2c_release_bus(void *, int);
static int tegra_i2c_exec(void *, i2c_op_t, i2c_addr_t, const void *,
size_t, void *, size_t, int);
@@ -140,8 +138,8 @@ tegra_i2c_attach(device_t parent, device
addr, error);
return;
}
- mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_VM);
- cv_init(&sc->sc_cv, device_xname(self));
+ mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_VM);
+ cv_init(&sc->sc_intr_wait, device_xname(self));
aprint_naive("\n");
aprint_normal(": I2C\n");
@@ -177,11 +175,12 @@ tegra_i2c_attach(device_t parent, device
}
fdtbus_reset_deassert(sc->sc_rst);
+ mutex_enter(&sc->sc_intr_lock);
tegra_i2c_init(sc);
+ mutex_exit(&sc->sc_intr_lock);
+ iic_tag_init(&sc->sc_ic);
sc->sc_ic.ic_cookie = sc;
- sc->sc_ic.ic_acquire_bus = tegra_i2c_acquire_bus;
- sc->sc_ic.ic_release_bus = tegra_i2c_release_bus;
sc->sc_ic.ic_exec = tegra_i2c_exec;
fdtbus_register_i2c_controller(self, phandle, &tegra_i2c_funcs);
@@ -236,48 +235,34 @@ tegra_i2c_intr(void *priv)
return 0;
I2C_WRITE(sc, I2C_INTERRUPT_STATUS_REG, istatus);
- mutex_enter(&sc->sc_lock);
- cv_broadcast(&sc->sc_cv);
- mutex_exit(&sc->sc_lock);
+ mutex_enter(&sc->sc_intr_lock);
+ cv_broadcast(&sc->sc_intr_wait);
+ mutex_exit(&sc->sc_intr_lock);
return 1;
}
static int
-tegra_i2c_acquire_bus(void *priv, int flags)
-{
- struct tegra_i2c_softc * const sc = priv;
-
- mutex_enter(&sc->sc_lock);
-
- return 0;
-}
-
-static void
-tegra_i2c_release_bus(void *priv, int flags)
-{
- struct tegra_i2c_softc * const sc = priv;
-
- mutex_exit(&sc->sc_lock);
-}
-
-static int
tegra_i2c_exec(void *priv, i2c_op_t op, i2c_addr_t addr, const void *cmdbuf,
size_t cmdlen, void *buf, size_t buflen, int flags)
{
struct tegra_i2c_softc * const sc = priv;
int retry, error;
-#if notyet
- if (cold)
-#endif
- flags |= I2C_F_POLL;
-
- KASSERT(mutex_owned(&sc->sc_lock));
+ /*
+ * XXXJRT This is probably no longer necessary? Before these
+ * changes, the bus lock was also used for the interrupt handler,
+ * and there would be a deadlock when the interrupt handler tried to
+ * acquire it again. The bus lock is now owned by the mid-layer and
+ * we have our own interrupt lock.
+ */
+ flags |= I2C_F_POLL;
if (buflen == 0 && cmdlen == 0)
return EINVAL;
+ mutex_enter(&sc->sc_intr_lock);
+
if ((flags & I2C_F_POLL) == 0) {
I2C_WRITE(sc, I2C_INTERRUPT_MASK_REG,
I2C_INTERRUPT_MASK_NOACK | I2C_INTERRUPT_MASK_ARB_LOST |
@@ -296,6 +281,7 @@ tegra_i2c_exec(void *priv, i2c_op_t op,
delay(1);
}
if (retry == 0) {
+ mutex_exit(&sc->sc_intr_lock);
device_printf(sc->sc_dev, "timeout flushing FIFO\n");
return EIO;
}
@@ -325,6 +311,8 @@ done:
tegra_i2c_init(sc);
}
+ mutex_exit(&sc->sc_intr_lock);
+
return error;
}
@@ -338,8 +326,9 @@ tegra_i2c_wait(struct tegra_i2c_softc *s
while (--retry > 0) {
if ((flags & I2C_F_POLL) == 0) {
- error = cv_timedwait_sig(&sc->sc_cv, &sc->sc_lock,
- uimax(mstohz(10), 1));
+ error = cv_timedwait_sig(&sc->sc_intr_wait,
+ &sc->sc_intr_lock,
+ uimax(mstohz(10), 1));
if (error) {
return error;
}
