Messages in this thread Patch in this message | | | Subject | Re: Nokia N900 - audio TPA6130A2 problems | From | Ivaylo Dimitrov <> | Date | Tue, 22 Mar 2016 10:02:23 +0200 |
| |
On 21.03.2016 21:34, Ivaylo Dimitrov wrote: > > > On 21.03.2016 16:53, Sebastian Reichel wrote: >> Hi Mark, >> >> On Mon, Mar 21, 2016 at 01:45:15PM +0000, Mark Brown wrote: >>> On Mon, Mar 21, 2016 at 03:39:15PM +0200, Ivaylo Dimitrov wrote: >>>> On 21.03.2016 13:45, Mark Brown wrote: >>> >>>>> No, if the voltage is variable we can't tell what the current >>>>> constraints are without something telling us so we just don't vary the >>>>> voltage until we're told to do this. If we immediately lower the >>>>> voltage to the minimum supported voltage that's going to break things. >>> >>>> There are constraints set by the board DTS. Isn't it reasonable the >>>> framework to set the voltage to minimum voltage from the dts if the >>>> current >>>> set one is bellow it? >>> >>> Yes, if it's out of bounds for the constraints we should bring it >>> up/down to the minimum/maximum (when copying people into a thread it's a >>> good idea to explain what the problem you are trying to solve is, >>> especially if you're throwing around bodges). >> >> We have this regulator definition in omap3-n900.dts: >> >> &vmmc2 { >> regulator-name = "V28_A"; >> regulator-min-microvolt = <2800000>; >> regulator-max-microvolt = <3000000>; >> regulator-always-on; /* due VIO leak to AIC34 VDDs */ >> }; >> >> The regulator is enabled during probe, but the voltage is not >> configured. The default reset voltage of the regulator is 2.6V. >> So basically when the regulator is enabled, it uses a voltage, >> which is out of the DT specified range. >> >> We also have a second problem: If the system has been rebooted from >> Nokia's stock kernel the regulator is left in STANDBY mode. Since >> the mode is not configured during probe, it results in different >> problems. According to my understanding it can be fixed trivially >> by adding >> >> &vmmc2 { >> regulator-initial-mode = <2>; >> }; >> > > doesn't work: > > "regulator-vmmc2: mapping for mode 2 not defined" > > twl-regulator is missing .of_map_mode function. > > Also, if we go that route, we should set the initial modes for all the > regulators, not only vmmc2 (and not only for N900), as we don't really > know what is the status of regulators at startup. I think a better > approach is if regulator framework sets all always-on regulators to > enabled, unless stated otherwise (which it already does iiuc). > > I think there is a bug in twl-regulator twl4030reg_enable() and/or > twl4030reg_is_enabled() - the latter only checks if DEV_GRP is P1, but > not for the actual state of the regulator (bits 3:0). Also, what looks > suspicious to me is that all the regulators are put in P1 device group. > Legacy board code spreads the regulators all over the groups, so maybe > this is simply a regression compared to legacy boot. >
This is what seems to work, I would like some comments from those who are more experienced with twl4030 than me before posting a formal patch. I borrowed the code from stock Nokia kernel.
diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c index 955a6fb..3740df4 100644 --- a/drivers/regulator/twl-regulator.c +++ b/drivers/regulator/twl-regulator.c @@ -21,7 +21,7 @@ #include <linux/regulator/machine.h> #include <linux/regulator/of_regulator.h> #include <linux/i2c/twl.h> - +#include <linux/delay.h>
/* * The TWL4030/TW5030/TPS659x0/TWL6030 family chips include power management, a @@ -165,7 +165,7 @@ static int twl4030reg_is_enabled(struct regulator_dev *rdev) if (state < 0) return state;
- return state & P1_GRP_4030; + return (state & 0x0f) != 0; }
static int twl6030reg_is_enabled(struct regulator_dev *rdev) @@ -188,11 +188,75 @@ static int twl6030reg_is_enabled(struct regulator_dev *rdev) return grp && (val == TWL6030_CFG_STATE_ON); }
+static int twl4030_wait_pb_ready(void) +{ + + int ret, timeout = 10; + u8 pb_state; + + do { + ret = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &pb_state, + TWL4030_PM_MASTER_PB_CFG); + if (ret < 0) + return ret; + + if (!(pb_state & 1)) + return 0; + + mdelay(1); + timeout--; + + } while (timeout); + + return -ETIMEDOUT; +} + +static int twl4030_send_pb_msg(unsigned msg) +{ + u8 pb_state; + int ret; + + /* save powerbus configuration */ + ret = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &pb_state, + TWL4030_PM_MASTER_PB_CFG); + if (ret < 0) + return ret; + + /* Enable I2C access to powerbus */ + ret = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, pb_state | BIT(1), + TWL4030_PM_MASTER_PB_CFG); + if (ret < 0) + return ret; + + ret = twl4030_wait_pb_ready(); + if (ret < 0) + return ret; + + ret = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, msg >> 8, + TWL4030_PM_MASTER_PB_WORD_MSB); + if (ret < 0) + return ret; + + ret = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, msg & 0xff, + TWL4030_PM_MASTER_PB_WORD_LSB); + if (ret < 0) + return ret; + + ret = twl4030_wait_pb_ready(); + if (ret < 0) + return ret; + + /* Restore powerbus configuration */ + return twl_i2c_write_u8(TWL_MODULE_PM_MASTER, pb_state, + TWL_MODULE_PM_MASTER); +} + static int twl4030reg_enable(struct regulator_dev *rdev) { struct twlreg_info *info = rdev_get_drvdata(rdev); int grp; int ret; + unsigned message;
grp = twlreg_grp(rdev); if (grp < 0) @@ -201,8 +265,12 @@ static int twl4030reg_enable(struct regulator_dev *rdev) grp |= P1_GRP_4030;
ret = twlreg_write(info, TWL_MODULE_PM_RECEIVER, VREG_GRP, grp); + if (ret < 0) + return ret;
- return ret; + message = MSG_SINGULAR(DEV_GRP_P1, info->id, RES_STATE_ACTIVE); + + return twl4030_send_pb_msg(message); }
static int twl6030reg_enable(struct regulator_dev *rdev) @@ -324,13 +392,7 @@ static int twl4030reg_set_mode(struct regulator_dev *rdev, unsigned mode) if (!(status & (P3_GRP_4030 | P2_GRP_4030 | P1_GRP_4030))) return -EACCES;
- status = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, - message >> 8, TWL4030_PM_MASTER_PB_WORD_MSB); - if (status < 0) - return status; - - return twl_i2c_write_u8(TWL_MODULE_PM_MASTER, - message & 0xff, TWL4030_PM_MASTER_PB_WORD_LSB); + return twl4030_send_pb_msg(message); }
static int twl6030reg_set_mode(struct regulator_dev *rdev, unsigned mode) Regards, Ivo
| |