Date: Sun, 26 Feb 2012 20:30:22 +0000 (UTC) From: Olivier Houchard <cognet@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r232196 - projects/armv6/sys/arm/ti/twl Message-ID: <201202262030.q1QKUM6m081928@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: cognet Date: Sun Feb 26 20:30:21 2012 New Revision: 232196 URL: http://svn.freebsd.org/changeset/base/232196 Log: Completely butcherize the locking, it's completely wrong, but so is calling malloc() or the iicbus stuff while holding locks, this requires careful review, and eventually rewriting. Modified: projects/armv6/sys/arm/ti/twl/twl.c projects/armv6/sys/arm/ti/twl/twl_vreg.c Modified: projects/armv6/sys/arm/ti/twl/twl.c ============================================================================== --- projects/armv6/sys/arm/ti/twl/twl.c Sun Feb 26 20:05:35 2012 (r232195) +++ projects/armv6/sys/arm/ti/twl/twl.c Sun Feb 26 20:30:21 2012 (r232196) @@ -156,13 +156,13 @@ twl_read(device_t dev, uint8_t nsub, uin msg[1].flags = IIC_M_RD; msg[1].len = cnt; msg[1].buf = buf; + TWL_UNLOCK(sc); - rc = iicbus_transfer(sc->sc_dev, msg, 2); + rc = iicbus_transfer(dev, msg, 2); - TWL_UNLOCK(sc); if (rc != 0) { - device_printf(sc->sc_dev, "iicbus read failed (adr:0x%02x, reg:0x%02x)\n", + device_printf(dev, "iicbus read failed (adr:0x%02x, reg:0x%02x)\n", addr, reg); return (EIO); } @@ -217,10 +217,10 @@ twl_write(device_t dev, uint8_t nsub, ui msg.flags = IIC_M_WR; msg.len = cnt + 1; msg.buf = tmp_buf; + TWL_UNLOCK(sc); - rc = iicbus_transfer(sc->sc_dev, &msg, 1); + rc = iicbus_transfer(dev, &msg, 1); - TWL_UNLOCK(sc); if (rc != 0) { device_printf(sc->sc_dev, "iicbus write failed (adr:0x%02x, reg:0x%02x)\n", @@ -251,8 +251,6 @@ twl_test_present(struct twl_softc *sc, u struct iic_msg msg; uint8_t tmp; - TWL_ASSERT_LOCKED(sc); - /* Set the address to read from */ msg.slave = addr; msg.flags = IIC_M_RD; @@ -283,8 +281,6 @@ twl_scan(void *dev) sc = device_get_softc((device_t)dev); - TWL_LOCK(sc); - memset(sc->sc_subaddr_map, TWL_INVALID_CHIP_ID, TWL_MAX_SUBADDRS); /* Try each of the addresses (0x48, 0x49, 0x4a & 0x4b) to determine which @@ -297,8 +293,6 @@ twl_scan(void *dev) } } - TWL_UNLOCK(sc); - /* Finished with the interrupt hook */ config_intrhook_disestablish(&sc->sc_scan_hook); } Modified: projects/armv6/sys/arm/ti/twl/twl_vreg.c ============================================================================== --- projects/armv6/sys/arm/ti/twl/twl_vreg.c Sun Feb 26 20:05:35 2012 (r232195) +++ projects/armv6/sys/arm/ti/twl/twl_vreg.c Sun Feb 26 20:30:21 2012 (r232196) @@ -81,6 +81,11 @@ __FBSDID("$FreeBSD$"); */ /* + * XXX cognet: the locking is plain wrong, but we can't just keep locks while + * calling functions that can sleep, such as malloc() or iicbus_transfer + */ + +/* * Power Groups bits for the 4030 and 6030 devices */ #define TWL4030_P3_GRP 0x80 /* Peripherals, power group */ @@ -427,8 +432,6 @@ twl_vreg_is_regulator_enabled(struct twl uint8_t grp; uint8_t state; - TWL_VREG_ASSERT_LOCKED(sc); - /* The status reading is different for the different devices */ if (twl_is_4030(sc->sc_dev)) { @@ -481,8 +484,6 @@ twl_vreg_disable_regulator(struct twl_vr int err = 0; uint8_t grp; - TWL_VREG_ASSERT_LOCKED(sc); - if (twl_is_4030(sc->sc_dev)) { /* Read the regulator CFG_GRP register */ @@ -531,8 +532,6 @@ twl_vreg_enable_regulator(struct twl_vre int err; uint8_t grp; - TWL_VREG_ASSERT_LOCKED(sc); - /* Read the regulator CFG_GRP register */ err = twl_vreg_read_1(sc, regulator, TWL_VREG_GRP, &grp); if (err) @@ -586,8 +585,6 @@ twl_vreg_write_regulator_voltage(struct int err; int vsel; - TWL_VREG_ASSERT_LOCKED(sc); - /* If millivolts is zero then we simply disable the output */ if (millivolts == 0) return (twl_vreg_disable_regulator(sc, regulator)); @@ -641,8 +638,6 @@ twl_vreg_read_regulator_voltage(struct t int ret; uint8_t vsel; - TWL_VREG_ASSERT_LOCKED(sc); - /* Check if the regulator is currently enabled */ if ((ret = twl_vreg_is_regulator_enabled(sc, regulator)) < 0) return (ret); @@ -707,7 +702,6 @@ twl_vreg_get_voltage(device_t dev, const /* Get the device context, take the lock and find the matching regulator */ sc = device_get_softc(dev); - TWL_VREG_LOCK(sc); /* Find the regulator with the matching name */ LIST_FOREACH(regulator, &sc->sc_vreg_list, entries) { @@ -721,8 +715,6 @@ twl_vreg_get_voltage(device_t dev, const if (found) err = twl_vreg_read_regulator_voltage(sc, regulator, millivolts); - TWL_VREG_UNLOCK(sc); - return (err); } @@ -751,8 +743,6 @@ twl_vreg_set_voltage(device_t dev, const /* Get the device context, take the lock and find the matching regulator */ sc = device_get_softc(dev); - TWL_VREG_LOCK(sc); - /* Find the regulator with the matching name */ LIST_FOREACH(regulator, &sc->sc_vreg_list, entries) { if (strcmp(regulator->name, name) == 0) { @@ -765,8 +755,6 @@ twl_vreg_set_voltage(device_t dev, const if (found) err = twl_vreg_write_regulator_voltage(sc, regulator, millivolts); - TWL_VREG_UNLOCK(sc); - return (err); } @@ -835,14 +823,28 @@ twl_vreg_add_regulator(struct twl_vreg_s { struct sysctl_ctx_list *ctx = device_get_sysctl_ctx(sc->sc_dev); struct sysctl_oid *tree = device_get_sysctl_tree(sc->sc_dev); - struct twl_regulator_entry *new; + struct twl_regulator_entry *new, *tmp; TWL_VREG_ASSERT_LOCKED(sc); + TWL_VREG_UNLOCK(sc); new = malloc(sizeof(struct twl_regulator_entry), M_DEVBUF, M_NOWAIT | M_ZERO); + TWL_VREG_LOCK(sc); if (new == NULL) return (NULL); + /* + * We had to drop the lock while calling malloc(), maybe + * the regulator got added in the meanwhile. + */ + + LIST_FOREACH(tmp, &sc->sc_vreg_list, entries) { + if (!strncmp(new->name, name, strlen(new->name)) && + new->sub_dev == nsub && new->reg_off == regbase) { + free(new, M_DEVBUF); + return (NULL); + } + } /* Copy over the name and register details */ strncpy(new->name, name, TWL_VREG_MAX_NAMELEN); new->name[TWL_VREG_MAX_NAMELEN - 1] = '\0'; @@ -855,10 +857,17 @@ twl_vreg_add_regulator(struct twl_vreg_s new->supp_voltages = voltages; new->num_supp_voltages = num_voltages; + /* + * We're in the list now, so we should be protected against double + * inclusion. + */ + + TWL_VREG_UNLOCK(sc); /* Add a sysctl entry for the voltage */ new->oid = SYSCTL_ADD_PROC(ctx, SYSCTL_CHILDREN(tree), OID_AUTO, name, CTLTYPE_INT | CTLFLAG_RD, sc, 0, twl_vreg_sysctl_voltage, "I", "voltage regulator"); + TWL_VREG_LOCK(sc); /* Finally add the regulator to list of supported regulators */ LIST_INSERT_HEAD(&sc->sc_vreg_list, new, entries); @@ -907,7 +916,9 @@ twl_vreg_add_regulators(struct twl_vreg_ /* TODO: set voltage from FDT if required */ /* Read the current voltage and print the info */ + TWL_VREG_UNLOCK(sc); twl_vreg_read_regulator_voltage(sc, entry, &millivolts); + TWL_VREG_LOCK(sc); if (debug) device_printf(sc->sc_dev, "%s : %d mV\n", walker->name, millivolts);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201202262030.q1QKUM6m081928>