| Message ID | 20211015133619.4698-8-rf@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: cs42l42: Collection of bugfixes | expand |
On Fri, Oct 15, 2021 at 02:36:10PM +0100, Richard Fitzgerald wrote: > The power-up sequence mandated in the datasheet is: > - VP must turn on first > - VA, VCP, VL, in any order > - VD_FILT after VL > static const char *const cs42l42_supply_names[CS42L42_NUM_SUPPLIES] = { > - "VA", > "VP", > + "VA", > "VCP", > - "VD_FILT", > "VL", > + "VD_FILT", > }; If you need the regulators to be turned on in sequence you shouldn't rely on bulk enable doing it for you - the existing regulator code will initiate all the enables in parallel and then wait for them all to complete ramping up so if for example VD_FILT were to ramp more quickly than the earlier regulators the hardware might notice it getting to whatever voltage the hardware cares about before them. The only sequencing you're getting at the minute is when the enables for the regulators are toggled and you shouldn't even rely on that. To get the sequencing guaranteed you should pull VP and VD_FILT out of the bulk enable and do individual enables for them.
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 629a0783e693..420e16563c45 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -2025,22 +2025,23 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client, return ret; } - ret = regulator_bulk_enable(ARRAY_SIZE(cs42l42->supplies), - cs42l42->supplies); - if (ret != 0) { - dev_err(&i2c_client->dev, - "Failed to enable supplies: %d\n", ret); - return ret; - } - - /* Reset the Device */ + /* Hold device in reset while it powers up */ cs42l42->reset_gpio = devm_gpiod_get(&i2c_client->dev, "reset", GPIOD_OUT_LOW); if (IS_ERR(cs42l42->reset_gpio)) { ret = PTR_ERR(cs42l42->reset_gpio); dev_err(&i2c_client->dev, "Failed to request reset gpio: %d\n", ret); - goto err_disable; + return ret; } + ret = regulator_bulk_enable(ARRAY_SIZE(cs42l42->supplies), + cs42l42->supplies); + if (ret != 0) { + dev_err(&i2c_client->dev, + "Failed to enable supplies: %d\n", ret); + return ret; + } + + /* Release reset and wait for boot */ gpiod_set_value_cansleep(cs42l42->reset_gpio, 1); usleep_range(CS42L42_BOOT_TIME_US, CS42L42_BOOT_TIME_US * 2); @@ -2116,6 +2117,7 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client, return 0; err_disable: + gpiod_set_value_cansleep(cs42l42->reset_gpio, 0); regulator_bulk_disable(ARRAY_SIZE(cs42l42->supplies), cs42l42->supplies); return ret; diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index 0704c902475f..2343213d0cdb 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -822,11 +822,11 @@ #define CS42L42_PLL_LOCK_TIMEOUT_US 1250 static const char *const cs42l42_supply_names[CS42L42_NUM_SUPPLIES] = { - "VA", "VP", + "VA", "VCP", - "VD_FILT", "VL", + "VD_FILT", }; struct cs42l42_private {
The power-up sequence mandated in the datasheet is: - VP must turn on first - VA, VCP, VL, in any order - VD_FILT after VL - RESET must be asserted while VP turns on - VD_FILT must turn off before VL - VP must turn off last This patch fixes the order the regulators are enabled and holds RESET asserted around the power-up. The datasheet power-down order is the reverse of the power-up order so this is automatically covered by listing the bulk regulators in power-up order. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> --- sound/soc/codecs/cs42l42.c | 22 ++++++++++++---------- sound/soc/codecs/cs42l42.h | 4 ++-- 2 files changed, 14 insertions(+), 12 deletions(-)