Discussion:
[PATCH 1/2] ASoC: rt5640: Add function for enabling DMIC from ACPI probed machine
Jarkko Nikula
2014-10-01 12:08:14 UTC
Permalink
There is no code enabling DMIC clock in systems that don't provide platform
data for rt5640 after commit 71d97a794301 ("ASoC: rt5640: Use the platform
data for DMIC settings").

I think it's worth to keep this static DMIC clock and alternative data pin
setting during probe time. For making possible to use DMIC from ACPI probed
machine (prior ACPI 5.1 with _DSD) this patch moves DMIC configuration to
new exported rt5640_dmic_enable() that machine drivers can call.

Please note, this patch moves DMIC configuration from i2c probe to codec
probe in case platform data for rt5640 is set.

Signed-off-by: Jarkko Nikula <***@linux.intel.com>
Cc: Oder Chiou <***@realtek.com>
---
for-next. I don't think there is regression caused by 71d97a794301 for the
byt-rt5640 machines. Theoretically yes for internal development platform
(which is don't care) but also byt-rt5640 got DMI quirks support for machine
specific routes only recently (i.e. machine without quirk was never working
completely).
---
sound/soc/codecs/rt5640.c | 49 +++++++++++++++++++++++++++++------------------
sound/soc/codecs/rt5640.h | 3 +++
2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index f1ec6e6bd08a..c3f2decd643c 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -1906,6 +1906,32 @@ static int rt5640_set_bias_level(struct snd_soc_codec *codec,
return 0;
}

+int rt5640_dmic_enable(struct snd_soc_codec *codec,
+ bool dmic1_data_pin, bool dmic2_data_pin)
+{
+ struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
+
+ regmap_update_bits(rt5640->regmap, RT5640_GPIO_CTRL1,
+ RT5640_GP2_PIN_MASK, RT5640_GP2_PIN_DMIC1_SCL);
+
+ if (dmic1_data_pin) {
+ regmap_update_bits(rt5640->regmap, RT5640_DMIC,
+ RT5640_DMIC_1_DP_MASK, RT5640_DMIC_1_DP_GPIO3);
+ regmap_update_bits(rt5640->regmap, RT5640_GPIO_CTRL1,
+ RT5640_GP3_PIN_MASK, RT5640_GP3_PIN_DMIC1_SDA);
+ }
+
+ if (dmic2_data_pin) {
+ regmap_update_bits(rt5640->regmap, RT5640_DMIC,
+ RT5640_DMIC_2_DP_MASK, RT5640_DMIC_2_DP_GPIO4);
+ regmap_update_bits(rt5640->regmap, RT5640_GPIO_CTRL1,
+ RT5640_GP4_PIN_MASK, RT5640_GP4_PIN_DMIC2_SDA);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(rt5640_dmic_enable);
+
static int rt5640_probe(struct snd_soc_codec *codec)
{
struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
@@ -1945,6 +1971,10 @@ static int rt5640_probe(struct snd_soc_codec *codec)
return -ENODEV;
}

+ if (rt5640->pdata.dmic_en)
+ rt5640_dmic_enable(codec, rt5640->pdata.dmic1_data_pin,
+ rt5640->pdata.dmic2_data_pin);
+
return 0;
}

@@ -2195,25 +2225,6 @@ static int rt5640_i2c_probe(struct i2c_client *i2c,
regmap_update_bits(rt5640->regmap, RT5640_IN3_IN4,
RT5640_IN_DF2, RT5640_IN_DF2);

- if (rt5640->pdata.dmic_en) {
- regmap_update_bits(rt5640->regmap, RT5640_GPIO_CTRL1,
- RT5640_GP2_PIN_MASK, RT5640_GP2_PIN_DMIC1_SCL);
-
- if (rt5640->pdata.dmic1_data_pin) {
- regmap_update_bits(rt5640->regmap, RT5640_DMIC,
- RT5640_DMIC_1_DP_MASK, RT5640_DMIC_1_DP_GPIO3);
- regmap_update_bits(rt5640->regmap, RT5640_GPIO_CTRL1,
- RT5640_GP3_PIN_MASK, RT5640_GP3_PIN_DMIC1_SDA);
- }
-
- if (rt5640->pdata.dmic2_data_pin) {
- regmap_update_bits(rt5640->regmap, RT5640_DMIC,
- RT5640_DMIC_2_DP_MASK, RT5640_DMIC_2_DP_GPIO4);
- regmap_update_bits(rt5640->regmap, RT5640_GPIO_CTRL1,
- RT5640_GP4_PIN_MASK, RT5640_GP4_PIN_DMIC2_SDA);
- }
- }
-
rt5640->hp_mute = 1;

return snd_soc_register_codec(&i2c->dev, &soc_codec_dev_rt5640,
diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index 58ebe96b86da..3deb8babeabb 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -2097,4 +2097,7 @@ struct rt5640_priv {
bool hp_mute;
};

+int rt5640_dmic_enable(struct snd_soc_codec *codec,
+ bool dmic1_data_pin, bool dmic2_data_pin);
+
#endif
--
2.1.0
Jarkko Nikula
2014-10-01 12:08:15 UTC
Permalink
It turned out DMIC interface wasn't enabled/disabled runtime for active
DMIC route in the rt5640 codec driver anymore after commit
71d97a794301 ("ASoC: rt5640: Use the platform data for DMIC settings").

Since DMIC interface must be enabled explicitly either by passing platform
data to rt5640 codec driver or by calling new rt5640_dmic_enable() this
patch adds a DMI quirk flag that is used to conditionally enable DMIC
interface during sound card init time.

Signed-off-by: Jarkko Nikula <***@linux.intel.com>
---
Goes on top of ("ASoC: Intel: byt-rt5640: Add quirk for Asus T100")
---
sound/soc/intel/byt-rt5640.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/byt-rt5640.c b/sound/soc/intel/byt-rt5640.c
index c323a101214e..8392c160d9e2 100644
--- a/sound/soc/intel/byt-rt5640.c
+++ b/sound/soc/intel/byt-rt5640.c
@@ -59,7 +59,11 @@ enum {
BYT_RT5640_IN1_MAP,
};

-static unsigned long byt_rt5640_custom_map = BYT_RT5640_DMIC1_MAP;
+#define BYT_RT5640_MAP(quirk) ((quirk) & 0xff)
+#define BYT_RT5640_DMIC_EN BIT(16)
+
+static unsigned long byt_rt5640_quirk = BYT_RT5640_DMIC1_MAP |
+ BYT_RT5640_DMIC_EN;

static const struct snd_kcontrol_new byt_rt5640_controls[] = {
SOC_DAPM_PIN_SWITCH("Headphone"),
@@ -94,7 +98,7 @@ static int byt_rt5640_hw_params(struct snd_pcm_substream *substream,

static int byt_rt5640_quirk_cb(const struct dmi_system_id *id)
{
- byt_rt5640_custom_map = (unsigned long)id->driver_data;
+ byt_rt5640_quirk = (unsigned long)id->driver_data;
return 1;
}

@@ -129,7 +133,7 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
}

dmi_check_system(byt_rt5640_quirk_table);
- switch (byt_rt5640_custom_map) {
+ switch (BYT_RT5640_MAP(byt_rt5640_quirk)) {
case BYT_RT5640_IN1_MAP:
custom_map = byt_rt5640_intmic_in1_map;
num_routes = ARRAY_SIZE(byt_rt5640_intmic_in1_map);
@@ -143,6 +147,12 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
if (ret)
return ret;

+ if (byt_rt5640_quirk & BYT_RT5640_DMIC_EN) {
+ ret = rt5640_dmic_enable(codec, 0, 0);
+ if (ret)
+ return ret;
+ }
+
snd_soc_dapm_ignore_suspend(dapm, "HPOL");
snd_soc_dapm_ignore_suspend(dapm, "HPOR");
--
2.1.0
Mark Brown
2014-10-01 16:04:44 UTC
Permalink
Post by Jarkko Nikula
There is no code enabling DMIC clock in systems that don't provide platform
data for rt5640 after commit 71d97a794301 ("ASoC: rt5640: Use the platform
data for DMIC settings").
Applied, thanks.

Loading...