Re: [alsa-devel] [PATCH] ASoC: add RT5640 CODEC driver
On 06/10/2013 11:10 PM, bardliao@realtek.com wrote:
From: Bard Liao bardliao@realtek.com
This patch adds the ALC5640 codec driver.
diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
+static int rt5640_i2c_probe(struct i2c_client *i2c,
- rt5640->regmap = devm_regmap_init_i2c(i2c, &rt5640_regmap);
- if (rt5640->pdata.ldo1_en) {
ret = devm_gpio_request_one(&i2c->dev, rt5640->pdata.ldo1_en,GPIOF_OUT_INIT_HIGH,"RT5640 LDO1_EN");if (ret < 0) {dev_err(&i2c->dev, "Failed to request LDO1_EN %d: %d\n",rt5640->pdata.ldo1_en, ret);return ret;}msleep(400);- }
Oh I see this is the only place ldo1_en is touched. I had assumed you were going to add code to turn it off/on based on bias level. That's why I had asked you to add that feature, since you'd know any HW requirements for doing that. Still, as Mark mentioned, that can certainly be added later.
One question though: Don't you want to initially enable ldo1_en before you create the regmap? At least some regmap_init() calls end up trying to read from the device to populate the register cache, and that won't work until ldo1_en is active.
Anyway, I'll go test this again, and work on the patch to pull the platform data from DT.
On 06/10/2013 11:10 PM, bardliao@realtek.com wrote:
From: Bard Liao bardliao@realtek.com
This patch adds the ALC5640 codec driver.
diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
+static int rt5640_i2c_probe(struct i2c_client *i2c,
rt5640->regmap = devm_regmap_init_i2c(i2c, &rt5640_regmap);
if (rt5640->pdata.ldo1_en) {ret = devm_gpio_request_one(&i2c->dev, rt5640->pdata.ldo1_en,GPIOF_OUT_INIT_HIGH,"RT5640 LDO1_EN");if (ret < 0) {dev_err(&i2c->dev, "Failed to request LDO1_EN %d: %d\n",rt5640->pdata.ldo1_en, ret);return ret;}msleep(400);}Oh I see this is the only place ldo1_en is touched. I had assumed you were going to add code to turn it off/on based on bias level. That's why I had asked you to add that feature, since you'd know any HW requirements for doing that. Still, as Mark mentioned, that can certainly be added later.
I think we can't turn on/off ldo1_en based on bias level. Because it will take too long from _BIAS_OFF to _BIAS_ON. I think we can do it in suspend/resume function.
One question though: Don't you want to initially enable ldo1_en before you create the regmap? At least some regmap_init() calls end up trying to read from the device to populate the register cache, and that won't work until ldo1_en is active.
Sure, it is better. Thanks for reminding me.
On Tue, Jun 11, 2013 at 11:41:49AM -0600, Stephen Warren wrote:
One question though: Don't you want to initially enable ldo1_en before you create the regmap? At least some regmap_init() calls end up trying to read from the device to populate the register cache, and that won't work until ldo1_en is active.
They shouldn't if a set of defaults is provided, though if a patch is going to be applied then that'll write to the device.
participants (3)
-
Bard Liao -
Mark Brown -
Stephen Warren