[PATCH v2 00/32] ASoC: rt5651: jack-detect fixes and improvements

[PATCH v2 00/32] ASoC: rt5651: jack-detect fixes and improvements

[Hans de Goede]

Hi All,

Here is v2 of my rt5651 codec and Intel machine-driver series which mainly
consists of various jack-detect related fixes and improvements.

As discussed in the review of v1 I've made the following changes:

1) Get rid of platform_data and use device-properties instead

2) Split up a couple of the micbias / OVCD / jack-detect patches into
smaller patches, as they were doing too much in a single patch

3) Improved commit messages

4) Concentrate the knowledge that SSP0 is 16 bits on BYTCR in the fixup
function and extract the sample-format from the hw_params in other places

Regards,

Hans

[PATCH v2 01/32] ASoC: rt5651: Remove unused rt5651_platform_data

[Hans de Goede]

There are no in tree users of platform-data for the rt5651 codec driver,
so lets remove support for it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/sound/rt5651.h    |  8 --------
 sound/soc/codecs/rt5651.c | 47 +++++++++--------------------------------------
 sound/soc/codecs/rt5651.h |  2 +-
 3 files changed, 10 insertions(+), 47 deletions(-)

diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
index 18b79a761f10..1612462bf5ad 100644
--- a/include/sound/rt5651.h
+++ b/include/sound/rt5651.h
@@ -18,12 +18,4 @@ enum rt5651_jd_src {
 	RT5651_JD2,
 };
 
-struct rt5651_platform_data {
-	/* IN2 can optionally be differential */
-	bool in2_diff;
-
-	bool dmic_en;
-	enum rt5651_jd_src jd_src;
-};
-
 #endif
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index de9034418ce3..2aa070554743 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -33,8 +33,6 @@
 #include "rt5651.h"
 
 #define RT5651_JD_MAP(quirk)	((quirk) & GENMASK(7, 0))
-#define RT5651_IN2_DIFF		BIT(16)
-#define RT5651_DMIC_EN		BIT(17)
 
 #define RT5651_DEVICE_ID_VALUE 0x6281
 
@@ -1569,7 +1567,7 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 		snd_soc_component_write(component, RT5651_PWR_DIG2, 0x0000);
 		snd_soc_component_write(component, RT5651_PWR_VOL, 0x0000);
 		snd_soc_component_write(component, RT5651_PWR_MIXER, 0x0000);
-		if (rt5651->pdata.jd_src) {
+		if (rt5651->jd_src) {
 			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0204);
 			snd_soc_component_write(component, RT5651_PWR_ANLG1, 0x0002);
 		} else {
@@ -1604,7 +1602,7 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
-	if (rt5651->pdata.jd_src) {
+	if (rt5651->jd_src) {
 		snd_soc_dapm_force_enable_pin(dapm, "JD Power");
 		snd_soc_dapm_force_enable_pin(dapm, "LDO");
 		snd_soc_dapm_sync(dapm);
@@ -1765,26 +1763,6 @@ static const struct dmi_system_id rt5651_quirk_table[] = {
 	{}
 };
 
-static int rt5651_parse_dt(struct rt5651_priv *rt5651, struct device_node *np)
-{
-	if (of_property_read_bool(np, "realtek,in2-differential"))
-		rt5651_quirk |= RT5651_IN2_DIFF;
-	if (of_property_read_bool(np, "realtek,dmic-en"))
-		rt5651_quirk |= RT5651_DMIC_EN;
-
-	return 0;
-}
-
-static void rt5651_set_pdata(struct rt5651_priv *rt5651)
-{
-	if (rt5651_quirk & RT5651_IN2_DIFF)
-		rt5651->pdata.in2_diff = true;
-	if (rt5651_quirk & RT5651_DMIC_EN)
-		rt5651->pdata.dmic_en = true;
-	if (RT5651_JD_MAP(rt5651_quirk))
-		rt5651->pdata.jd_src = RT5651_JD_MAP(rt5651_quirk);
-}
-
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
@@ -1841,7 +1819,7 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 	if (!rt5651->component)
 		return;
 
-	switch (rt5651->pdata.jd_src) {
+	switch (rt5651->jd_src) {
 	case RT5651_JD1_1:
 		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x1000;
 		break;
@@ -1875,7 +1853,6 @@ EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
 static int rt5651_i2c_probe(struct i2c_client *i2c,
 		    const struct i2c_device_id *id)
 {
-	struct rt5651_platform_data *pdata = dev_get_platdata(&i2c->dev);
 	struct rt5651_priv *rt5651;
 	int ret;
 
@@ -1886,14 +1863,8 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, rt5651);
 
-	if (pdata)
-		rt5651->pdata = *pdata;
-	else if (i2c->dev.of_node)
-		rt5651_parse_dt(rt5651, i2c->dev.of_node);
-	else
-		dmi_check_system(rt5651_quirk_table);
-
-	rt5651_set_pdata(rt5651);
+	dmi_check_system(rt5651_quirk_table);
+	rt5651->jd_src = RT5651_JD_MAP(rt5651_quirk);
 
 	rt5651->regmap = devm_regmap_init_i2c(i2c, &rt5651_regmap);
 	if (IS_ERR(rt5651->regmap)) {
@@ -1917,23 +1888,23 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 	if (ret != 0)
 		dev_warn(&i2c->dev, "Failed to apply regmap patch: %d\n", ret);
 
-	if (rt5651->pdata.in2_diff)
+	if (device_property_read_bool(&i2c->dev, "realtek,in2-differential"))
 		regmap_update_bits(rt5651->regmap, RT5651_IN1_IN2,
 					RT5651_IN_DF2, RT5651_IN_DF2);
 
-	if (rt5651->pdata.dmic_en)
+	if (device_property_read_bool(&i2c->dev, "realtek,dmic-en"))
 		regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
 				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
 
 	rt5651->hp_mute = 1;
 
-	if (rt5651->pdata.jd_src) {
+	if (rt5651->jd_src) {
 
 		/* IRQ output on GPIO1 */
 		regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
 				   RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
 
-		switch (rt5651->pdata.jd_src) {
+		switch (rt5651->jd_src) {
 		case RT5651_JD1_1:
 			regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
 					   RT5651_JD_TRG_SEL_MASK,
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 1ef38429e6a0..148e139e6a26 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2060,10 +2060,10 @@ struct rt5651_pll_code {
 
 struct rt5651_priv {
 	struct snd_soc_component *component;
-	struct rt5651_platform_data pdata;
 	struct regmap *regmap;
 	struct snd_soc_jack *hp_jack;
 	struct delayed_work jack_detect_work;
+	enum rt5651_jd_src jd_src;
 
 	int sysclk;
 	int sysclk_src;
-- 
2.14.3

[PATCH v2 02/32] ASoC: rt5651: Move all jack-detect initialization to rt5651_set_jack_detect

[Hans de Goede]

Move all jack-detect initialization to rt5651_set_jack_detect. The main
reason to do this is so that platform code can setup jack-detect properties
after the device has been probed, which unfortunately is necessary on some
platforms.

This has 2 additional advantages:
1) Grouping all jack-detect init together makes it easier to follow what
is happening and results in a small reduction in the number of loc.
2) Before we would register the irq handler before rt5651->hp_jack was
assigned, leading to a potential NULL deref if the jack_detect work runs
before the machine driver has called set_jack.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 119 ++++++++++++++++++++++------------------------
 sound/soc/codecs/rt5651.h |   1 +
 2 files changed, 58 insertions(+), 62 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 2aa070554743..de3f071f8695 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1586,7 +1586,6 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 static int rt5651_probe(struct snd_soc_component *component)
 {
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
-	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 
 	rt5651->component = component;
 
@@ -1602,15 +1601,6 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
-	if (rt5651->jd_src) {
-		snd_soc_dapm_force_enable_pin(dapm, "JD Power");
-		snd_soc_dapm_force_enable_pin(dapm, "LDO");
-		snd_soc_dapm_sync(dapm);
-
-		regmap_update_bits(rt5651->regmap, RT5651_MICBIAS,
-				   0x38, 0x38);
-	}
-
 	return 0;
 }
 
@@ -1841,10 +1831,65 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 int rt5651_set_jack_detect(struct snd_soc_component *component,
 			   struct snd_soc_jack *hp_jack)
 {
+	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	if (!rt5651->irq)
+		return -EINVAL;
+
+	/* IRQ output on GPIO1 */
+	snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
+		RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
+
+	/* Select jack detect source */
+	switch (rt5651->jd_src) {
+	case RT5651_JD1_1:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_1);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD1_1_IRQ_EN, RT5651_JD1_1_IRQ_EN);
+		break;
+	case RT5651_JD1_2:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_2);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD1_2_IRQ_EN, RT5651_JD1_2_IRQ_EN);
+		break;
+	case RT5651_JD2:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD2);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD2_IRQ_EN, RT5651_JD2_IRQ_EN);
+		break;
+	case RT5651_JD_NULL:
+		return 0;
+	default:
+		dev_err(component->dev, "Currently only JD1_1 / JD1_2 / JD2 are supported\n");
+		return -EINVAL;
+	}
+
+	snd_soc_dapm_force_enable_pin(dapm, "JD Power");
+	snd_soc_dapm_force_enable_pin(dapm, "LDO");
+	snd_soc_dapm_sync(dapm);
+
+	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
 
 	rt5651->hp_jack = hp_jack;
-	rt5651_irq(0, rt5651);
+
+	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
+					rt5651_irq,
+					IRQF_TRIGGER_RISING |
+					IRQF_TRIGGER_FALLING |
+					IRQF_ONESHOT, "rt5651", rt5651);
+	if (ret) {
+		dev_err(component->dev, "Failed to reguest IRQ: %d\n", ret);
+		return ret;
+	}
+
+	/* sync initial jack state */
+	queue_delayed_work(system_power_efficient_wq,
+			   &rt5651->jack_detect_work, 0);
 
 	return 0;
 }
@@ -1896,61 +1941,11 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 		regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
 				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
 
+	rt5651->irq = i2c->irq;
 	rt5651->hp_mute = 1;
 
-	if (rt5651->jd_src) {
-
-		/* IRQ output on GPIO1 */
-		regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
-				   RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
-
-		switch (rt5651->jd_src) {
-		case RT5651_JD1_1:
-			regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
-					   RT5651_JD_TRG_SEL_MASK,
-					   RT5651_JD_TRG_SEL_JD1_1);
-			regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
-					   RT5651_JD1_1_IRQ_EN,
-					   RT5651_JD1_1_IRQ_EN);
-			break;
-		case RT5651_JD1_2:
-			regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
-					   RT5651_JD_TRG_SEL_MASK,
-					   RT5651_JD_TRG_SEL_JD1_2);
-			regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
-					   RT5651_JD1_2_IRQ_EN,
-					   RT5651_JD1_2_IRQ_EN);
-			break;
-		case RT5651_JD2:
-			regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
-					   RT5651_JD_TRG_SEL_MASK,
-					   RT5651_JD_TRG_SEL_JD2);
-			regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
-					   RT5651_JD2_IRQ_EN,
-					   RT5651_JD2_IRQ_EN);
-			break;
-		case RT5651_JD_NULL:
-			break;
-		default:
-			dev_warn(&i2c->dev, "Currently only JD1_1 / JD1_2 / JD2 are supported\n");
-			break;
-		}
-	}
-
 	INIT_DELAYED_WORK(&rt5651->jack_detect_work, rt5651_jack_detect_work);
 
-	if (i2c->irq) {
-		ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
-						rt5651_irq,
-						IRQF_TRIGGER_RISING |
-						IRQF_TRIGGER_FALLING |
-						IRQF_ONESHOT, "rt5651", rt5651);
-		if (ret) {
-			dev_err(&i2c->dev, "Failed to reguest IRQ: %d\n", ret);
-			return ret;
-		}
-	}
-
 	ret = devm_snd_soc_register_component(&i2c->dev,
 				&soc_component_dev_rt5651,
 				rt5651_dai, ARRAY_SIZE(rt5651_dai));
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 148e139e6a26..8f128d057ff0 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2065,6 +2065,7 @@ struct rt5651_priv {
 	struct delayed_work jack_detect_work;
 	enum rt5651_jd_src jd_src;
 
+	int irq;
 	int sysclk;
 	int sysclk_src;
 	int lrck[RT5651_AIFS];
-- 
2.14.3

[PATCH v2 03/32] ASoC: rt5651: Move 2 functions higher up in rt5651.c

[Hans de Goede]

Move 2 functions higher up in rt5651.c, this is a preparation patch
to avoid needing forward declarations when moving over from a codec
private function to the standard snd_soc_component_set_jack().

This commit purely moves these 2 functions up, not a single line is
changed.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 154 +++++++++++++++++++++++-----------------------
 1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index de3f071f8695..11741cdf722e 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1583,6 +1583,83 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 	return 0;
 }
 
+static irqreturn_t rt5651_irq(int irq, void *data)
+{
+	struct rt5651_priv *rt5651 = data;
+
+	queue_delayed_work(system_power_efficient_wq,
+			   &rt5651->jack_detect_work, msecs_to_jiffies(250));
+
+	return IRQ_HANDLED;
+}
+
+int rt5651_set_jack_detect(struct snd_soc_component *component,
+			   struct snd_soc_jack *hp_jack)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
+	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	if (!rt5651->irq)
+		return -EINVAL;
+
+	/* IRQ output on GPIO1 */
+	snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
+		RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
+
+	/* Select jack detect source */
+	switch (rt5651->jd_src) {
+	case RT5651_JD1_1:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_1);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD1_1_IRQ_EN, RT5651_JD1_1_IRQ_EN);
+		break;
+	case RT5651_JD1_2:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_2);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD1_2_IRQ_EN, RT5651_JD1_2_IRQ_EN);
+		break;
+	case RT5651_JD2:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD2);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD2_IRQ_EN, RT5651_JD2_IRQ_EN);
+		break;
+	case RT5651_JD_NULL:
+		return 0;
+	default:
+		dev_err(component->dev, "Currently only JD1_1 / JD1_2 / JD2 are supported\n");
+		return -EINVAL;
+	}
+
+	snd_soc_dapm_force_enable_pin(dapm, "JD Power");
+	snd_soc_dapm_force_enable_pin(dapm, "LDO");
+	snd_soc_dapm_sync(dapm);
+
+	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
+
+	rt5651->hp_jack = hp_jack;
+
+	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
+					rt5651_irq,
+					IRQF_TRIGGER_RISING |
+					IRQF_TRIGGER_FALLING |
+					IRQF_ONESHOT, "rt5651", rt5651);
+	if (ret) {
+		dev_err(component->dev, "Failed to reguest IRQ: %d\n", ret);
+		return ret;
+	}
+
+	/* sync initial jack state */
+	queue_delayed_work(system_power_efficient_wq,
+			   &rt5651->jack_detect_work, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
+
 static int rt5651_probe(struct snd_soc_component *component)
 {
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
@@ -1753,16 +1830,6 @@ static const struct dmi_system_id rt5651_quirk_table[] = {
 	{}
 };
 
-static irqreturn_t rt5651_irq(int irq, void *data)
-{
-	struct rt5651_priv *rt5651 = data;
-
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, msecs_to_jiffies(250));
-
-	return IRQ_HANDLED;
-}
-
 static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
@@ -1828,73 +1895,6 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
 }
 
-int rt5651_set_jack_detect(struct snd_soc_component *component,
-			   struct snd_soc_jack *hp_jack)
-{
-	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
-	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
-	int ret;
-
-	if (!rt5651->irq)
-		return -EINVAL;
-
-	/* IRQ output on GPIO1 */
-	snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
-		RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
-
-	/* Select jack detect source */
-	switch (rt5651->jd_src) {
-	case RT5651_JD1_1:
-		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
-			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_1);
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
-			RT5651_JD1_1_IRQ_EN, RT5651_JD1_1_IRQ_EN);
-		break;
-	case RT5651_JD1_2:
-		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
-			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_2);
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
-			RT5651_JD1_2_IRQ_EN, RT5651_JD1_2_IRQ_EN);
-		break;
-	case RT5651_JD2:
-		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
-			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD2);
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
-			RT5651_JD2_IRQ_EN, RT5651_JD2_IRQ_EN);
-		break;
-	case RT5651_JD_NULL:
-		return 0;
-	default:
-		dev_err(component->dev, "Currently only JD1_1 / JD1_2 / JD2 are supported\n");
-		return -EINVAL;
-	}
-
-	snd_soc_dapm_force_enable_pin(dapm, "JD Power");
-	snd_soc_dapm_force_enable_pin(dapm, "LDO");
-	snd_soc_dapm_sync(dapm);
-
-	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
-
-	rt5651->hp_jack = hp_jack;
-
-	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
-					rt5651_irq,
-					IRQF_TRIGGER_RISING |
-					IRQF_TRIGGER_FALLING |
-					IRQF_ONESHOT, "rt5651", rt5651);
-	if (ret) {
-		dev_err(component->dev, "Failed to reguest IRQ: %d\n", ret);
-		return ret;
-	}
-
-	/* sync initial jack state */
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, 0);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
-
 static int rt5651_i2c_probe(struct i2c_client *i2c,
 		    const struct i2c_device_id *id)
 {
-- 
2.14.3

[PATCH v2 04/32] ASoC: rt5651: Use standard component set_jack callback

[Hans de Goede]

Use the standard component set_jack callback instead of defining a codec
private API for this.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c             | 6 +++---
 sound/soc/codecs/rt5651.h             | 2 --
 sound/soc/intel/boards/bytcr_rt5651.c | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 11741cdf722e..3e507f53fc13 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1593,8 +1593,8 @@ static irqreturn_t rt5651_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-int rt5651_set_jack_detect(struct snd_soc_component *component,
-			   struct snd_soc_jack *hp_jack)
+static int rt5651_set_jack(struct snd_soc_component *component,
+			   struct snd_soc_jack *hp_jack, void *data)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
@@ -1658,7 +1658,6 @@ int rt5651_set_jack_detect(struct snd_soc_component *component,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
 
 static int rt5651_probe(struct snd_soc_component *component)
 {
@@ -1762,6 +1761,7 @@ static const struct snd_soc_component_driver soc_component_dev_rt5651 = {
 	.suspend		= rt5651_suspend,
 	.resume			= rt5651_resume,
 	.set_bias_level		= rt5651_set_bias_level,
+	.set_jack		= rt5651_set_jack,
 	.controls		= rt5651_snd_controls,
 	.num_controls		= ARRAY_SIZE(rt5651_snd_controls),
 	.dapm_widgets		= rt5651_dapm_widgets,
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 8f128d057ff0..f3158488fc89 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2080,6 +2080,4 @@ struct rt5651_priv {
 	bool hp_mute;
 };
 
-int rt5651_set_jack_detect(struct snd_soc_component *component,
-			   struct snd_soc_jack *hp_jack);
 #endif /* __RT5651_H__ */
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 1f8407b17d2b..8008a027cc93 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -368,7 +368,7 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 		return ret;
 	}
 
-	rt5651_set_jack_detect(codec, &priv->jack);
+	snd_soc_component_set_jack(codec, &priv->jack, NULL);
 
 	return ret;
 }
-- 
2.14.3

[PATCH v2 05/32] ASoC: rt5651: Add rt5651_apply_properties() helper function

[Hans de Goede]

Move the applying of the differential input and dmic properties to a new
rt5651_apply_properties() helper function. This new function can be called
by platform code which attaches properties after probe() has run to apply
these new properties.

Note this also moves the time when we apply these properties for DT
platforms from i2c-probe to snd-component-probe time, this should not
result in any functional difference.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 22 ++++++++++++++--------
 sound/soc/codecs/rt5651.h |  2 ++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 3e507f53fc13..79a839c713ec 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1659,6 +1659,18 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	return 0;
 }
 
+void rt5651_apply_properties(struct snd_soc_component *component)
+{
+	if (device_property_read_bool(component->dev, "realtek,in2-differential"))
+		snd_soc_component_update_bits(component, RT5651_IN1_IN2,
+				RT5651_IN_DF2, RT5651_IN_DF2);
+
+	if (device_property_read_bool(component->dev, "realtek,dmic-en"))
+		snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
+				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
+}
+EXPORT_SYMBOL_GPL(rt5651_apply_properties);
+
 static int rt5651_probe(struct snd_soc_component *component)
 {
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
@@ -1677,6 +1689,8 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
+	rt5651_apply_properties(component);
+
 	return 0;
 }
 
@@ -1933,14 +1947,6 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 	if (ret != 0)
 		dev_warn(&i2c->dev, "Failed to apply regmap patch: %d\n", ret);
 
-	if (device_property_read_bool(&i2c->dev, "realtek,in2-differential"))
-		regmap_update_bits(rt5651->regmap, RT5651_IN1_IN2,
-					RT5651_IN_DF2, RT5651_IN_DF2);
-
-	if (device_property_read_bool(&i2c->dev, "realtek,dmic-en"))
-		regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
-				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
-
 	rt5651->irq = i2c->irq;
 	rt5651->hp_mute = 1;
 
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index f3158488fc89..7d9d5fa09d6f 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2080,4 +2080,6 @@ struct rt5651_priv {
 	bool hp_mute;
 };
 
+void rt5651_apply_properties(struct snd_soc_component *component);
+
 #endif /* __RT5651_H__ */
-- 
2.14.3

[PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede]

Configure the jack-detect source through a device-property which can be
set by code outside of the codec driver. Rather then putting platform
specific DMI quirks inside the generic codec driver.

And move the jack-detect-source quirk for the KIANO SlimNote 14.2, which
was present inside the codec driver to the machine driver, where we can
bundle it together with the other quirks already present for this laptop.

Cc: devicetree@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/sound/rt5651.txt |  6 ++++
 include/sound/rt5651.h                             |  4 +++
 sound/soc/codecs/rt5651.c                          | 31 ++++----------------
 sound/soc/intel/boards/bytcr_rt5651.c              | 34 ++++++++++++++++++----
 4 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/rt5651.txt b/Documentation/devicetree/bindings/sound/rt5651.txt
index 3875233095f5..1632c0cf6123 100644
--- a/Documentation/devicetree/bindings/sound/rt5651.txt
+++ b/Documentation/devicetree/bindings/sound/rt5651.txt
@@ -16,6 +16,12 @@ Optional properties:
 - realtek,dmic-en
   Boolean. true if dmic is used.
 
+- realtek,jack-detect-source
+  u32. Valid values:
+  1: Use JD1_1 pin for jack-dectect
+  2: Use JD1_2 pin for jack-dectect
+  3: Use JD2 pin for jack-dectect
+
 Pins on the device (for linking into audio routes) for RT5651:
 
   * DMIC L1
diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
index 1612462bf5ad..725b36c329d0 100644
--- a/include/sound/rt5651.h
+++ b/include/sound/rt5651.h
@@ -11,6 +11,10 @@
 #ifndef __LINUX_SND_RT5651_H
 #define __LINUX_SND_RT5651_H
 
+/*
+ * Note these MUST match the values from the DT binding:
+ * Documentation/devicetree/bindings/sound/rt5651.txt
+ */
 enum rt5651_jd_src {
 	RT5651_JD_NULL,
 	RT5651_JD1_1,
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 79a839c713ec..7d2bb6fbde4e 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -19,7 +19,6 @@
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/acpi.h>
-#include <linux/dmi.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -32,8 +31,6 @@
 #include "rl6231.h"
 #include "rt5651.h"
 
-#define RT5651_JD_MAP(quirk)	((quirk) & GENMASK(7, 0))
-
 #define RT5651_DEVICE_ID_VALUE 0x6281
 
 #define RT5651_PR_RANGE_BASE (0xff + 1)
@@ -41,8 +38,6 @@
 
 #define RT5651_PR_BASE (RT5651_PR_RANGE_BASE + (0 * RT5651_PR_SPACING))
 
-static unsigned long rt5651_quirk;
-
 static const struct regmap_range_cfg rt5651_ranges[] = {
 	{ .name = "PR", .range_min = RT5651_PR_BASE,
 	  .range_max = RT5651_PR_BASE + 0xb4,
@@ -1599,6 +1594,11 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
 	int ret;
+	u32 val;
+
+	if (device_property_read_u32(component->dev,
+				     "realtek,jack-detect-source", &val) == 0)
+		rt5651->jd_src = val;
 
 	if (!rt5651->irq)
 		return -EINVAL;
@@ -1826,24 +1826,6 @@ static const struct i2c_device_id rt5651_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5651_i2c_id);
 
-static int rt5651_quirk_cb(const struct dmi_system_id *id)
-{
-	rt5651_quirk = (unsigned long) id->driver_data;
-	return 1;
-}
-
-static const struct dmi_system_id rt5651_quirk_table[] = {
-	{
-		.callback = rt5651_quirk_cb,
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "KIANO"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
-		},
-		.driver_data = (unsigned long *) RT5651_JD1_1,
-	},
-	{}
-};
-
 static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
@@ -1922,9 +1904,6 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, rt5651);
 
-	dmi_check_system(rt5651_quirk_table);
-	rt5651->jd_src = RT5651_JD_MAP(rt5651_quirk);
-
 	rt5651->regmap = devm_regmap_init_i2c(i2c, &rt5651_regmap);
 	if (IS_ERR(rt5651->regmap)) {
 		ret = PTR_ERR(rt5651->regmap);
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 8008a027cc93..0af855d2df49 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -20,6 +20,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/device.h>
@@ -42,10 +43,21 @@ enum {
 	BYT_RT5651_IN3_MAP,
 };
 
-#define BYT_RT5651_MAP(quirk)	((quirk) & GENMASK(7, 0))
-#define BYT_RT5651_DMIC_EN	BIT(16)
-#define BYT_RT5651_MCLK_EN	BIT(17)
-#define BYT_RT5651_MCLK_25MHZ	BIT(18)
+enum {
+	BYT_RT5651_JD_NULL	= (RT5651_JD_NULL << 4),
+	BYT_RT5651_JD1_1	= (RT5651_JD1_1 << 4),
+	BYT_RT5651_JD1_2	= (RT5651_JD1_2 << 4),
+	BYT_RT5651_JD2		= (RT5651_JD2 << 4),
+};
+
+#define BYT_RT5651_MAP(quirk)		((quirk) & GENMASK(3, 0))
+#define BYT_RT5651_JDSRC(quirk)		(((quirk) & GENMASK(7, 4)) >> 4)
+#define BYT_RT5651_DMIC_EN		BIT(16)
+#define BYT_RT5651_MCLK_EN		BIT(17)
+#define BYT_RT5651_MCLK_25MHZ		BIT(18)
+
+/* jack-detect-source + terminating empty entry */
+#define MAX_NO_PROPS 2
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -66,6 +78,9 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
 		dev_info(dev, "quirk IN3_MAP enabled");
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk))
+		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
+			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		dev_info(dev, "quirk DMIC enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
@@ -288,6 +303,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
 	{}
@@ -298,8 +314,9 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 	struct snd_soc_card *card = runtime->card;
 	struct snd_soc_component *codec = runtime->codec_dai->component;
 	struct byt_rt5651_private *priv = snd_soc_card_get_drvdata(card);
+	struct property_entry props[MAX_NO_PROPS] = {};
 	const struct snd_soc_dapm_route *custom_map;
-	int num_routes;
+	int num_routes, cnt = 0;
 	int ret;
 
 	card->dapm.idle_bias_off = true;
@@ -360,6 +377,13 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 			dev_err(card->dev, "unable to set MCLK rate\n");
 	}
 
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
+				BYT_RT5651_JDSRC(byt_rt5651_quirk));
+
+	ret = device_add_properties(codec->dev, props);
+	if (ret)
+		return ret;
+
 	ret = snd_soc_card_jack_new(runtime->card, "Headset",
 				    SND_JACK_HEADSET, &priv->jack,
 				    bytcr_jack_pins, ARRAY_SIZE(bytcr_jack_pins));
-- 
2.14.3

[PATCH v2 07/32] ASoC: rt5651: Remove is_sys_clk_from_pll()

[Hans de Goede]

is_sys_clk_from_pll() is used as a snd_soc_dapm_route.connected callback,
checking RT5651_GBL_CLK to determine if the sys-clk is PLL1 and thus the
PWR_PLL bit in reg PWR_ANLG2 must be set.

RT5651_GBL_CLK is changed by rt5651_set_dai_sysclk(), which gets called by
the pre_pmu / post_pmd functions of the "Platform Clock" dapm-supply.

This creates an ordering issue, during a dapm transition first all
connected() callbacks are called to build a list of supplies to enable
and then the complete list is walked to enable the supplies. Since the
connected() check happens before enabling any supplies,
is_sys_clk_from_pll() ends up deciding if the PWR_PLL bit should be set
based on the state the "Platform Clock" supply had *before* the transition.
This sometimes results in PWR_PLL being off, even though *after* the
transition PLL1 is configured as sys-clk.

This commit removes is_sys_clk_from_pll() instead simply setting / clearing
PWR_PLL in rt5651_set_dai_sysclk() based on the selected sys-clk, which
fixes this and as a bonus results in a nice cleanup.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note that many of the other rt56?? drivers have a similar
is_sys_clk_from_pll() functon and may need a similar fix.
---
 sound/soc/codecs/rt5651.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 7d2bb6fbde4e..d4a2a1ef87b2 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -393,20 +393,6 @@ static int set_dmic_clk(struct snd_soc_dapm_widget *w,
 	return idx;
 }
 
-static int is_sysclk_from_pll(struct snd_soc_dapm_widget *source,
-			 struct snd_soc_dapm_widget *sink)
-{
-	struct snd_soc_component *component = snd_soc_dapm_to_component(source->dapm);
-	unsigned int val;
-
-	val = snd_soc_component_read32(component, RT5651_GLB_CLK);
-	val &= RT5651_SCLK_SRC_MASK;
-	if (val == RT5651_SCLK_SRC_PLL1)
-		return 1;
-	else
-		return 0;
-}
-
 /* Digital Mixer */
 static const struct snd_kcontrol_new rt5651_sto1_adc_l_mix[] = {
 	SOC_DAPM_SINGLE("ADC1 Switch", RT5651_STO1_ADC_MIXER,
@@ -878,8 +864,6 @@ static const struct snd_soc_dapm_widget rt5651_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY_S("ADC ASRC", 1, RT5651_PLL_MODE_2,
 			      11, 0, NULL, 0),
 
-	SND_SOC_DAPM_SUPPLY("PLL1", RT5651_PWR_ANLG2,
-			RT5651_PWR_PLL_BIT, 0, NULL, 0),
 	/* Input Side */
 	SND_SOC_DAPM_SUPPLY("JD Power", RT5651_PWR_ANLG2,
 		RT5651_PWM_JD_M_BIT, 0, NULL, 0),
@@ -1162,7 +1146,6 @@ static const struct snd_soc_dapm_route rt5651_dapm_routes[] = {
 	{"Stereo1 ADC MIXL", "ADC1 Switch", "Stereo1 ADC L1 Mux"},
 	{"Stereo1 ADC MIXL", "ADC2 Switch", "Stereo1 ADC L2 Mux"},
 	{"Stereo1 ADC MIXL", NULL, "Stereo1 Filter"},
-	{"Stereo1 Filter", NULL, "PLL1", is_sysclk_from_pll},
 	{"Stereo1 Filter", NULL, "ADC ASRC"},
 
 	{"Stereo1 ADC MIXR", "ADC1 Switch", "Stereo1 ADC R1 Mux"},
@@ -1172,7 +1155,6 @@ static const struct snd_soc_dapm_route rt5651_dapm_routes[] = {
 	{"Stereo2 ADC MIXL", "ADC1 Switch", "Stereo2 ADC L1 Mux"},
 	{"Stereo2 ADC MIXL", "ADC2 Switch", "Stereo2 ADC L2 Mux"},
 	{"Stereo2 ADC MIXL", NULL, "Stereo2 Filter"},
-	{"Stereo2 Filter", NULL, "PLL1", is_sysclk_from_pll},
 	{"Stereo2 Filter", NULL, "ADC ASRC"},
 
 	{"Stereo2 ADC MIXR", "ADC1 Switch", "Stereo2 ADC R1 Mux"},
@@ -1239,10 +1221,8 @@ static const struct snd_soc_dapm_route rt5651_dapm_routes[] = {
 	{"PDM R Mux", "DD MIX", "DAC MIXR"},
 
 	{"DAC L1", NULL, "Stereo DAC MIXL"},
-	{"DAC L1", NULL, "PLL1", is_sysclk_from_pll},
 	{"DAC L1", NULL, "DAC L1 Power"},
 	{"DAC R1", NULL, "Stereo DAC MIXR"},
-	{"DAC R1", NULL, "PLL1", is_sysclk_from_pll},
 	{"DAC R1", NULL, "DAC R1 Power"},
 
 	{"DD MIXL", "DAC L1 Switch", "DAC MIXL"},
@@ -1438,6 +1418,7 @@ static int rt5651_set_dai_sysclk(struct snd_soc_dai *dai,
 	struct snd_soc_component *component = dai->component;
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
 	unsigned int reg_val = 0;
+	unsigned int pll_bit = 0;
 
 	if (freq == rt5651->sysclk && clk_id == rt5651->sysclk_src)
 		return 0;
@@ -1448,6 +1429,7 @@ static int rt5651_set_dai_sysclk(struct snd_soc_dai *dai,
 		break;
 	case RT5651_SCLK_S_PLL1:
 		reg_val |= RT5651_SCLK_SRC_PLL1;
+		pll_bit |= RT5651_PWR_PLL;
 		break;
 	case RT5651_SCLK_S_RCCLK:
 		reg_val |= RT5651_SCLK_SRC_RCCLK;
@@ -1456,6 +1438,8 @@ static int rt5651_set_dai_sysclk(struct snd_soc_dai *dai,
 		dev_err(component->dev, "Invalid clock id (%d)\n", clk_id);
 		return -EINVAL;
 	}
+	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
+		RT5651_PWR_PLL, pll_bit);
 	snd_soc_component_update_bits(component, RT5651_GLB_CLK,
 		RT5651_SCLK_SRC_MASK, reg_val);
 	rt5651->sysclk = freq;
-- 
2.14.3

[PATCH v2 08/32] ASoC: rt5651: Fix bias_level confusion

[Hans de Goede]

The rt5651_set_bias_level() function was turning everything off at
SND_SOC_BIAS_STANDBY, rather then at SND_SOC_BIAS_OFF, requiring the bias-
level to be raised to SND_SOC_BIAS_PREPARE before turning anything on.

This is not how the bias-levels are supposed to work, this commit fixes
this by turning everything off at the SND_SOC_BIAS_OFF level and enabling
the pwr-bits needed for minimum functionality at SND_SOC_BIAS_STANDBY.

This fixes the minimum set of pwr-bits not getting enabled when
force-enabling some dapm-supplies (e.g. for jack type detection),
which raises the bias-level to standby.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index d4a2a1ef87b2..90e800416a8e 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1521,6 +1521,13 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 	switch (level) {
 	case SND_SOC_BIAS_PREPARE:
 		if (SND_SOC_BIAS_STANDBY == snd_soc_component_get_bias_level(component)) {
+			if (snd_soc_component_read32(component, RT5651_PLL_MODE_1) & 0x9200)
+				snd_soc_component_update_bits(component, RT5651_D_MISC,
+						    0xc00, 0xc00);
+		}
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		if (SND_SOC_BIAS_OFF == snd_soc_component_get_bias_level(component)) {
 			snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
 				RT5651_PWR_VREF1 | RT5651_PWR_MB |
 				RT5651_PWR_BG | RT5651_PWR_VREF2,
@@ -1534,13 +1541,10 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 				RT5651_PWR_LDO_DVO_MASK,
 				RT5651_PWR_LDO_DVO_1_2V);
 			snd_soc_component_update_bits(component, RT5651_D_MISC, 0x1, 0x1);
-			if (snd_soc_component_read32(component, RT5651_PLL_MODE_1) & 0x9200)
-				snd_soc_component_update_bits(component, RT5651_D_MISC,
-						    0xc00, 0xc00);
 		}
 		break;
 
-	case SND_SOC_BIAS_STANDBY:
+	case SND_SOC_BIAS_OFF:
 		snd_soc_component_write(component, RT5651_D_MISC, 0x0010);
 		snd_soc_component_write(component, RT5651_PWR_DIG1, 0x0000);
 		snd_soc_component_write(component, RT5651_PWR_DIG2, 0x0000);
-- 
2.14.3

[PATCH v2 09/32] ASoC: rt5651: Do not modify the LDO voltage control bits from set_bias_level()

[Hans de Goede]

The PWR_ANLG1 reg not only contains various power on/off bits, it also
contains 2 bits which select if the LDO generates 1.0, 1.1 or 1.2V. Note
there is a separate on/off bit for the LDO.

rt5651_set_bias_level(BIAS_OFF) used to unconditionally clear the entire
register, when jack-detection support was introduced a special case for
jack-detect was added which hard-codes a register value to keep the LDO
voltage at 1.2 volt.

This commit removes the jack-detect special case, instead simply always
leaving the LDO voltage control bits as is on BIAS_OFF.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 90e800416a8e..22f233e00ef9 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1550,11 +1550,12 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 		snd_soc_component_write(component, RT5651_PWR_DIG2, 0x0000);
 		snd_soc_component_write(component, RT5651_PWR_VOL, 0x0000);
 		snd_soc_component_write(component, RT5651_PWR_MIXER, 0x0000);
+		/* Do not touch the LDO voltage select bits on bias-off */
+		snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
+			~RT5651_PWR_LDO_DVO_MASK, 0);
 		if (rt5651->jd_src) {
 			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0204);
-			snd_soc_component_write(component, RT5651_PWR_ANLG1, 0x0002);
 		} else {
-			snd_soc_component_write(component, RT5651_PWR_ANLG1, 0x0000);
 			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0000);
 		}
 		break;
-- 
2.14.3

[PATCH v2 10/32] ASoC: rt5651: Do not modify jd and PLL power bits from set_bias_level()

[Hans de Goede]

rt5651_set_bias_level(BIAS_OFF) used to unconditionally clear the entire
register, including the jack-detect and PLL power bits. When jack-detection
support was introduced a special case for jack-detect was added which
hard-codes a register value to keep both on.

This commit removes the jack-detect special case, instead simply leaving
these bits as is on BIAS_OFF.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 22f233e00ef9..4d5d934794da 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1516,8 +1516,6 @@ static int rt5651_set_dai_pll(struct snd_soc_dai *dai, int pll_id, int source,
 static int rt5651_set_bias_level(struct snd_soc_component *component,
 			enum snd_soc_bias_level level)
 {
-	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
-
 	switch (level) {
 	case SND_SOC_BIAS_PREPARE:
 		if (SND_SOC_BIAS_STANDBY == snd_soc_component_get_bias_level(component)) {
@@ -1553,11 +1551,9 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 		/* Do not touch the LDO voltage select bits on bias-off */
 		snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
 			~RT5651_PWR_LDO_DVO_MASK, 0);
-		if (rt5651->jd_src) {
-			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0204);
-		} else {
-			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0000);
-		}
+		/* Leave PLL1 and jack-detect power as is, all others off */
+		snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
+				    ~(RT5651_PWR_PLL | RT5651_PWR_JD_M), 0);
 		break;
 
 	default:
-- 
2.14.3

[PATCH v2 11/32] ASoC: rt5651: Remove programming of PWR regs before force_bias_level() call

[Hans de Goede]

Remove the setup of the PWR_ANLG1 reg which was done directly before
calling snd_soc_component_force_bias_level(SND_SOC_BIAS_OFF), as the
latter will override any settings done to PWR_ANLG1 immediately anyways.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 4d5d934794da..1f3cd79e90d6 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1662,16 +1662,6 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	rt5651->component = component;
 
-	snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
-		RT5651_PWR_VREF1 | RT5651_PWR_MB |
-		RT5651_PWR_BG | RT5651_PWR_VREF2,
-		RT5651_PWR_VREF1 | RT5651_PWR_MB |
-		RT5651_PWR_BG | RT5651_PWR_VREF2);
-	usleep_range(10000, 15000);
-	snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
-		RT5651_PWR_FV1 | RT5651_PWR_FV2,
-		RT5651_PWR_FV1 | RT5651_PWR_FV2);
-
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
 	rt5651_apply_properties(component);
-- 
2.14.3

[PATCH v2 12/32] ASoC: rt5651: Only configure LDO voltage once at boot

[Hans de Goede]

Now that rt5651_set_bias_level(BIAS_OFF) no longer modifies the LDO
voltage selection bits, there is no need to set them each time we move
to standby. Instead configure them once at component-probe() time.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 1f3cd79e90d6..249f7b7936e4 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1535,9 +1535,6 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 			snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
 				RT5651_PWR_FV1 | RT5651_PWR_FV2,
 				RT5651_PWR_FV1 | RT5651_PWR_FV2);
-			snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
-				RT5651_PWR_LDO_DVO_MASK,
-				RT5651_PWR_LDO_DVO_1_2V);
 			snd_soc_component_update_bits(component, RT5651_D_MISC, 0x1, 0x1);
 		}
 		break;
@@ -1662,6 +1659,9 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	rt5651->component = component;
 
+	snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
+		RT5651_PWR_LDO_DVO_MASK, RT5651_PWR_LDO_DVO_1_2V);
+
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
 	rt5651_apply_properties(component);
-- 
2.14.3

[PATCH v2 13/32] ASoC: rt5651: Remove "JD Power" dapm supply

[Hans de Goede]

Remove the "JD power" dapm supply which gets force-enabled once when
using jack-detect and never gets disabled again.

Since the PWR_JD_M bit simply needs to be always on when using jack-detect
there is no need to have it tracked by dapm, instead we can simply set it
to 1 once when initializing the jack-detection.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 249f7b7936e4..bfacf8ea6069 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -864,10 +864,6 @@ static const struct snd_soc_dapm_widget rt5651_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY_S("ADC ASRC", 1, RT5651_PLL_MODE_2,
 			      11, 0, NULL, 0),
 
-	/* Input Side */
-	SND_SOC_DAPM_SUPPLY("JD Power", RT5651_PWR_ANLG2,
-		RT5651_PWM_JD_M_BIT, 0, NULL, 0),
-
 	/* micbias */
 	SND_SOC_DAPM_SUPPLY("LDO", RT5651_PWR_ANLG1,
 			RT5651_PWR_LDO_BIT, 0, NULL, 0),
@@ -1616,7 +1612,10 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 		return -EINVAL;
 	}
 
-	snd_soc_dapm_force_enable_pin(dapm, "JD Power");
+	/* Enable jack detect power */
+	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
+		RT5651_PWR_JD_M, RT5651_PWR_JD_M);
+
 	snd_soc_dapm_force_enable_pin(dapm, "LDO");
 	snd_soc_dapm_sync(dapm);
 
-- 
2.14.3

[PATCH v2 14/32] ASoC: rt5651: Enable LDO and micbias1 supplies for jack-type detection

[Hans de Goede]

To determine if a plugged in jack is a headset (speakers + mic) or
headphones (mic contact shorted to ground) we use the micbias1 OVer Current
Detect (OVCD) functionality.

For this to work we need to have a micbias current to actually cause an
overcurrent condition when headphones are plugged in, so jack-type
detection requires both the LDO and micbias1 supplies to be on.

Before this commit there were 2 issues with the handling of this:
1) The LDO supply was force-enabled twice and never disabled again even
though it only needs to be forced on when doing jack-type detection
2) micbias1 was not force-enabled, and thus may be off when doing jack-type
detection

This commit fixes both by force-enabling the LDO and micbias1 supplies
before checking for an overcurrent condition and disabling them afterwards.

Note that both supplies will still get turned on normally (and OVCD will
protect against overcurrent) when the micbias1 is enabled normally because
the user has activated a sound stream recording from the mic.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index bfacf8ea6069..2e23e126ff54 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1556,6 +1556,28 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 	return 0;
 }
 
+static void rt5651_enable_micbias1_for_ovcd(struct snd_soc_component *component)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
+
+	snd_soc_dapm_mutex_lock(dapm);
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "LDO");
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "micbias1");
+	snd_soc_dapm_sync_unlocked(dapm);
+	snd_soc_dapm_mutex_unlock(dapm);
+}
+
+static void rt5651_disable_micbias1_for_ovcd(struct snd_soc_component *component)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
+
+	snd_soc_dapm_mutex_lock(dapm);
+	snd_soc_dapm_disable_pin_unlocked(dapm, "micbias1");
+	snd_soc_dapm_disable_pin_unlocked(dapm, "LDO");
+	snd_soc_dapm_sync_unlocked(dapm);
+	snd_soc_dapm_mutex_unlock(dapm);
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
@@ -1569,7 +1591,6 @@ static irqreturn_t rt5651_irq(int irq, void *data)
 static int rt5651_set_jack(struct snd_soc_component *component,
 			   struct snd_soc_jack *hp_jack, void *data)
 {
-	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
 	int ret;
 	u32 val;
@@ -1616,9 +1637,6 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
 		RT5651_PWR_JD_M, RT5651_PWR_JD_M);
 
-	snd_soc_dapm_force_enable_pin(dapm, "LDO");
-	snd_soc_dapm_sync(dapm);
-
 	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
 
 	rt5651->hp_jack = hp_jack;
@@ -1802,13 +1820,10 @@ MODULE_DEVICE_TABLE(i2c, rt5651_i2c_id);
 
 static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
 {
-	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 	int jack_type;
 
 	if (jack_insert) {
-		snd_soc_dapm_force_enable_pin(dapm, "LDO");
-		snd_soc_dapm_sync(dapm);
-
+		rt5651_enable_micbias1_for_ovcd(component);
 		snd_soc_component_update_bits(component, RT5651_MICBIAS,
 				    RT5651_MIC1_OVCD_MASK |
 				    RT5651_MIC1_OVTH_MASK |
@@ -1825,6 +1840,7 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 			jack_type = SND_JACK_HEADSET;
 		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
 				    RT5651_MB1_OC_CLR, 0);
+		rt5651_disable_micbias1_for_ovcd(component);
 	} else { /* jack out */
 		jack_type = 0;
 
-- 
2.14.3

[PATCH v2 15/32] ASoC: rt5651: Only configure OVCD once at set_jack time

[Hans de Goede]

Only configure OVCD once at set_jack time, rather then configuring
it on every jack-insertion event and switch to using bit field defines
instead of hardcoding a magic value.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 2e23e126ff54..4f9f0f99c4b0 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1637,7 +1637,15 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
 		RT5651_PWR_JD_M, RT5651_PWR_JD_M);
 
-	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
+	snd_soc_component_update_bits(component, RT5651_MICBIAS,
+				      RT5651_MIC1_OVCD_MASK |
+				      RT5651_MIC1_OVTH_MASK |
+				      RT5651_PWR_CLK12M_MASK |
+				      RT5651_PWR_MB_MASK,
+				      RT5651_MIC1_OVCD_DIS |
+				      RT5651_MIC1_OVTH_600UA |
+				      RT5651_PWR_MB_PU |
+				      RT5651_PWR_CLK12M_PU);
 
 	rt5651->hp_jack = hp_jack;
 
@@ -1825,14 +1833,8 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 	if (jack_insert) {
 		rt5651_enable_micbias1_for_ovcd(component);
 		snd_soc_component_update_bits(component, RT5651_MICBIAS,
-				    RT5651_MIC1_OVCD_MASK |
-				    RT5651_MIC1_OVTH_MASK |
-				    RT5651_PWR_CLK12M_MASK |
-				    RT5651_PWR_MB_MASK,
-				    RT5651_MIC1_OVCD_EN |
-				    RT5651_MIC1_OVTH_600UA |
-				    RT5651_PWR_MB_PU |
-				    RT5651_PWR_CLK12M_PU);
+				    RT5651_MIC1_OVCD_MASK,
+				    RT5651_MIC1_OVCD_EN);
 		msleep(100);
 		if (snd_soc_component_read32(component, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
 			jack_type = SND_JACK_HEADPHONE;
-- 
2.14.3

[PATCH v2 16/32] ASoC: rt5651: Always keep OVCD enabled

[Hans de Goede]

OVCD is not only useful for jack-type detection, but is also useful
to protect against over-current faults in general, so always keep
OVCD enabled, instead of only enabling it for jack-type detection.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 4f9f0f99c4b0..dfdc8244d308 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1642,7 +1642,7 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 				      RT5651_MIC1_OVTH_MASK |
 				      RT5651_PWR_CLK12M_MASK |
 				      RT5651_PWR_MB_MASK,
-				      RT5651_MIC1_OVCD_DIS |
+				      RT5651_MIC1_OVCD_EN |
 				      RT5651_MIC1_OVTH_600UA |
 				      RT5651_PWR_MB_PU |
 				      RT5651_PWR_CLK12M_PU);
@@ -1832,9 +1832,6 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 
 	if (jack_insert) {
 		rt5651_enable_micbias1_for_ovcd(component);
-		snd_soc_component_update_bits(component, RT5651_MICBIAS,
-				    RT5651_MIC1_OVCD_MASK,
-				    RT5651_MIC1_OVCD_EN);
 		msleep(100);
 		if (snd_soc_component_read32(component, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
 			jack_type = SND_JACK_HEADPHONE;
@@ -1845,10 +1842,6 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 		rt5651_disable_micbias1_for_ovcd(component);
 	} else { /* jack out */
 		jack_type = 0;
-
-		snd_soc_component_update_bits(component, RT5651_MICBIAS,
-				    RT5651_MIC1_OVCD_MASK,
-				    RT5651_MIC1_OVCD_DIS);
 	}
 
 	return jack_type;
-- 
2.14.3

[PATCH v2 17/32] ASoC: rt5651: Allow specifying over-current thresholds through device-properties

[Hans de Goede]

OVer-Current-Detection (OVCD) for the micbias current is used to detect if
an inserted jack is a headset or headphones (mic shorted to ground).

Some boards may need different values for the OVCD threshold because of a
resistor on the board in serial with or parallel to the jack mic contact.

This commit adds support for the sofar unset OVCD scale-factor register
values and adds support for specifying non-default values for both the
current threshold and the scale-factor through device-properties.

Cc: devicetree@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/sound/rt5651.txt | 11 ++++++
 include/sound/rt5651.h                             | 11 ++++++
 sound/soc/codecs/rt5651.c                          | 42 +++++++++++++++++++++-
 sound/soc/codecs/rt5651.h                          | 10 ++++++
 4 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/rt5651.txt b/Documentation/devicetree/bindings/sound/rt5651.txt
index 1632c0cf6123..8cb7ee8e79cf 100644
--- a/Documentation/devicetree/bindings/sound/rt5651.txt
+++ b/Documentation/devicetree/bindings/sound/rt5651.txt
@@ -22,6 +22,17 @@ Optional properties:
   2: Use JD1_2 pin for jack-dectect
   3: Use JD2 pin for jack-dectect
 
+- realtek,over-current-threshold
+  u32, micbias over-current detection threshold in µA, valid values are
+  600, 1500 and 2000µA.
+
+- realtek,over-current-scale-factor
+  u32, micbias over-current detection scale-factor, valid values are:
+  0: Scale current by 0.5
+  1: Scale current by 0.75
+  2: Scale current by 1.0
+  3: Scale current by 1.5
+
 Pins on the device (for linking into audio routes) for RT5651:
 
   * DMIC L1
diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
index 725b36c329d0..6403b862fb9a 100644
--- a/include/sound/rt5651.h
+++ b/include/sound/rt5651.h
@@ -22,4 +22,15 @@ enum rt5651_jd_src {
 	RT5651_JD2,
 };
 
+/*
+ * Note these MUST match the values from the DT binding:
+ * Documentation/devicetree/bindings/sound/rt5651.txt
+ */
+enum rt5651_ovcd_sf {
+	RT5651_OVCD_SF_0P5,
+	RT5651_OVCD_SF_0P75,
+	RT5651_OVCD_SF_1P0,
+	RT5651_OVCD_SF_1P5,
+};
+
 #endif
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index dfdc8244d308..38d2e12fe701 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1592,6 +1592,13 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 			   struct snd_soc_jack *hp_jack, void *data)
 {
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	/*
+	 * Testing on various boards has shown that good defaults for the OVCD
+	 * threshold and scale-factor are 2000µA and 0.75. For an effective
+	 * limit of 1500µA, this seems to be more reliable then 1500µA and 1.0.
+	 */
+	unsigned int ovcd_th = RT5651_MIC1_OVTH_2000UA;
+	unsigned int ovcd_sf = RT5651_MIC_OVCD_SF_0P75;
 	int ret;
 	u32 val;
 
@@ -1599,6 +1606,35 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 				     "realtek,jack-detect-source", &val) == 0)
 		rt5651->jd_src = val;
 
+	if (device_property_read_u32(component->dev,
+			"realtek,over-current-threshold", &val) == 0) {
+		switch (val) {
+		case 600:
+			ovcd_th = RT5651_MIC1_OVTH_600UA;
+			break;
+		case 1500:
+			ovcd_th = RT5651_MIC1_OVTH_1500UA;
+			break;
+		case 2000:
+			ovcd_th = RT5651_MIC1_OVTH_2000UA;
+			break;
+		default:
+			dev_err(component->dev, "Invalid over-current-threshold value: %d\n",
+				val);
+			return -EINVAL;
+		}
+	}
+
+	if (device_property_read_u32(component->dev,
+			"realtek,over-current-scale-factor", &val) == 0) {
+		if (val > RT5651_OVCD_SF_1P5) {
+			dev_err(component->dev, "Invalid over-current-scale-factor value: %d\n",
+				val);
+			return -EINVAL;
+		}
+		ovcd_sf = val << RT5651_MIC_OVCD_SF_SFT;
+	}
+
 	if (!rt5651->irq)
 		return -EINVAL;
 
@@ -1637,13 +1673,17 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
 		RT5651_PWR_JD_M, RT5651_PWR_JD_M);
 
+	/* Set OVCD threshold current and scale-factor */
+	snd_soc_component_write(component, RT5651_PR_BASE + RT5651_BIAS_CUR4,
+				0xa800 | ovcd_sf);
+
 	snd_soc_component_update_bits(component, RT5651_MICBIAS,
 				      RT5651_MIC1_OVCD_MASK |
 				      RT5651_MIC1_OVTH_MASK |
 				      RT5651_PWR_CLK12M_MASK |
 				      RT5651_PWR_MB_MASK,
 				      RT5651_MIC1_OVCD_EN |
-				      RT5651_MIC1_OVTH_600UA |
+				      ovcd_th |
 				      RT5651_PWR_MB_PU |
 				      RT5651_PWR_CLK12M_PU);
 
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 7d9d5fa09d6f..5fb25f7e2a8c 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -138,6 +138,7 @@
 /* Index of Codec Private Register definition */
 #define RT5651_BIAS_CUR1			0x12
 #define RT5651_BIAS_CUR3			0x14
+#define RT5651_BIAS_CUR4			0x15
 #define RT5651_CLSD_INT_REG1			0x1c
 #define RT5651_CHPUMP_INT_REG1			0x24
 #define RT5651_MAMP_INT_REG2			0x37
@@ -1966,6 +1967,15 @@
 #define RT5651_D_GATE_EN_SFT			0
 
 /* Codec Private Register definition */
+
+/* MIC Over current threshold scale factor (0x15) */
+#define RT5651_MIC_OVCD_SF_MASK			(0x3 << 8)
+#define RT5651_MIC_OVCD_SF_SFT			8
+#define RT5651_MIC_OVCD_SF_0P5			(0x0 << 8)
+#define RT5651_MIC_OVCD_SF_0P75			(0x1 << 8)
+#define RT5651_MIC_OVCD_SF_1P0			(0x2 << 8)
+#define RT5651_MIC_OVCD_SF_1P5			(0x3 << 8)
+
 /* 3D Speaker Control (0x63) */
 #define RT5651_3D_SPK_MASK			(0x1 << 15)
 #define RT5651_3D_SPK_SFT			15
-- 
2.14.3

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH v2 18/32] ASoC: rt5651: Enable sticky mode for OVCD

[Hans de Goede]

When the mic-gnd contacts are short-circuited by a headphones plug, the
hardware periodically retries if it can apply the bias-current leading to
the OVCD status flip-flopping 1-0-1 with it being 0 about 10% of the time.
This commit enables the sticky bit for the OVCD status to deal with this.

This commit also introduces 2 helper functions to deal with the OVCD
status bit, this may seem a bit overkill now, but these will also be
used in future patches.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 38d2e12fe701..f590906b43c6 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1578,6 +1578,22 @@ static void rt5651_disable_micbias1_for_ovcd(struct snd_soc_component *component
 	snd_soc_dapm_mutex_unlock(dapm);
 }
 
+static void rt5651_clear_micbias1_ovcd(struct snd_soc_component *component)
+{
+	snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
+		RT5651_MB1_OC_CLR, 0);
+}
+
+static bool rt5651_micbias1_ovcd(struct snd_soc_component *component)
+{
+	int val;
+
+	val = snd_soc_component_read32(component, RT5651_IRQ_CTRL2);
+	dev_dbg(component->dev, "irq ctrl2 %#04x\n", val);
+
+	return (val & RT5651_MB1_OC_CLR);
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
@@ -1687,6 +1703,18 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 				      RT5651_PWR_MB_PU |
 				      RT5651_PWR_CLK12M_PU);
 
+	/*
+	 * The over-current-detect is only reliable in detecting the absence
+	 * of over-current, when the mic-contact in the jack is short-circuited,
+	 * the hardware periodically retries if it can apply the bias-current
+	 * leading to the ovcd status flip-flopping 1-0-1 with it being 0 about
+	 * 10% of the time, as we poll the ovcd status bit we might hit that
+	 * 10%, so we enable sticky mode and when checking OVCD we clear the
+	 * status, msleep() a bit and then check to get a reliable reading.
+	 */
+	snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
+		RT5651_MB1_OC_STKY_MASK, RT5651_MB1_OC_STKY_EN);
+
 	rt5651->hp_jack = hp_jack;
 
 	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
@@ -1872,13 +1900,12 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 
 	if (jack_insert) {
 		rt5651_enable_micbias1_for_ovcd(component);
+		rt5651_clear_micbias1_ovcd(component);
 		msleep(100);
-		if (snd_soc_component_read32(component, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
+		if (rt5651_micbias1_ovcd(component))
 			jack_type = SND_JACK_HEADPHONE;
 		else
 			jack_type = SND_JACK_HEADSET;
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
-				    RT5651_MB1_OC_CLR, 0);
 		rt5651_disable_micbias1_for_ovcd(component);
 	} else { /* jack out */
 		jack_type = 0;
-- 
2.14.3

[PATCH v2 19/32] ASoC: rt5651: Enable Platform Clock during jack-type detect

[Hans de Goede]

When using RCCLK instead of MCLK / PLL1 the OVCD status often gets stuck
at its last value, which breaks jack-type detection.

This commit fixes this by force-enabling the platform clock when doing
jack-type detection.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index f590906b43c6..2d0c5ec451cd 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1563,6 +1563,8 @@ static void rt5651_enable_micbias1_for_ovcd(struct snd_soc_component *component)
 	snd_soc_dapm_mutex_lock(dapm);
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "LDO");
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "micbias1");
+	/* OVCD is unreliable when used with RCCLK as sysclk-source */
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "Platform Clock");
 	snd_soc_dapm_sync_unlocked(dapm);
 	snd_soc_dapm_mutex_unlock(dapm);
 }
@@ -1572,6 +1574,7 @@ static void rt5651_disable_micbias1_for_ovcd(struct snd_soc_component *component
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 
 	snd_soc_dapm_mutex_lock(dapm);
+	snd_soc_dapm_disable_pin_unlocked(dapm, "Platform Clock");
 	snd_soc_dapm_disable_pin_unlocked(dapm, "micbias1");
 	snd_soc_dapm_disable_pin_unlocked(dapm, "LDO");
 	snd_soc_dapm_sync_unlocked(dapm);
-- 
2.14.3

[PATCH v2 20/32] ASoC: rt5651: Add rt5651_jack_inserted() helper

[Hans de Goede]

Add rt5651_jack_inserted() helper to get the jack-detect switch status,
This is a preparation patch for rewriting the jack type-detection to
make it more reliable.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 2d0c5ec451cd..b88585d652ea 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1597,6 +1597,31 @@ static bool rt5651_micbias1_ovcd(struct snd_soc_component *component)
 	return (val & RT5651_MB1_OC_CLR);
 }
 
+static bool rt5651_jack_inserted(struct snd_soc_component *component)
+{
+	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	int val;
+
+	val = snd_soc_component_read32(component, RT5651_INT_IRQ_ST);
+	dev_dbg(component->dev, "irq status %#04x\n", val);
+
+	switch (rt5651->jd_src) {
+	case RT5651_JD1_1:
+		val &= 0x1000;
+		break;
+	case RT5651_JD1_2:
+		val &= 0x2000;
+		break;
+	case RT5651_JD2:
+		val &= 0x4000;
+		break;
+	default:
+		break;
+	}
+
+	return val == 0;
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
@@ -1921,27 +1946,13 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 {
 	struct rt5651_priv *rt5651 =
 		container_of(work, struct rt5651_priv, jack_detect_work.work);
-
-	int report, val = 0;
+	int report, jack_inserted;
 
 	if (!rt5651->component)
 		return;
 
-	switch (rt5651->jd_src) {
-	case RT5651_JD1_1:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x1000;
-		break;
-	case RT5651_JD1_2:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x2000;
-		break;
-	case RT5651_JD2:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x4000;
-		break;
-	default:
-		break;
-	}
-
-	report = rt5651_jack_detect(rt5651->component, !val);
+	jack_inserted = rt5651_jack_inserted(rt5651->component);
+	report = rt5651_jack_detect(rt5651->component, jack_inserted);
 
 	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
 }
-- 
2.14.3

[PATCH v2 21/32] ASoC: rt5651: Rewrite jack-type detection

[Hans de Goede]

We get the insertion event before the jack is fully inserted at which point
the second ring on a TRRS connector may short the 2nd ring and sleeve
contacts. Testing has shown that this short-circuit may happen as late
as 500ms after the insertion event, but it never lasts longer then 300ms.

This commit changes the detection algorithm to require 5 identical OVCD
values in a row at 100 ms intervals to fix the jack-type sometimes getting
mis-detected.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 110 +++++++++++++++++++++++++++++-----------------
 sound/soc/codecs/rt5651.h |   2 +-
 2 files changed, 70 insertions(+), 42 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index b88585d652ea..ee6f951fdbb9 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1622,12 +1622,76 @@ static bool rt5651_jack_inserted(struct snd_soc_component *component)
 	return val == 0;
 }
 
+/* Jack detect timings */
+#define JACK_SETTLE_TIME	100 /* milli seconds */
+#define JACK_DETECT_COUNT	5
+#define JACK_DETECT_MAXCOUNT	20  /* Aprox. 2 seconds worth of tries */
+
+static int rt5651_detect_headset(struct snd_soc_component *component)
+{
+	int i, headset_count = 0, headphone_count = 0;
+
+	/*
+	 * We get the insertion event before the jack is fully inserted at which
+	 * point the second ring on a TRRS connector may short the 2nd ring and
+	 * sleeve contacts, also the overcurrent detection is not entirely
+	 * reliable. So we try several times with a wait in between until we
+	 * detect the same type JACK_DETECT_COUNT times in a row.
+	 */
+	for (i = 0; i < JACK_DETECT_MAXCOUNT; i++) {
+		/* Clear any previous over-current status flag */
+		rt5651_clear_micbias1_ovcd(component);
+
+		msleep(JACK_SETTLE_TIME);
+
+		/* Check the jack is still connected before checking ovcd */
+		if (!rt5651_jack_inserted(component))
+			return 0;
+
+		if (rt5651_micbias1_ovcd(component)) {
+			/*
+			 * Over current detected, there is a short between the
+			 * 2nd ring contact and the ground, so a TRS connector
+			 * without a mic contact and thus plain headphones.
+			 */
+			dev_dbg(component->dev, "mic-gnd shorted\n");
+			headset_count = 0;
+			headphone_count++;
+			if (headphone_count == JACK_DETECT_COUNT)
+				return SND_JACK_HEADPHONE;
+		} else {
+			dev_dbg(component->dev, "mic-gnd open\n");
+			headphone_count = 0;
+			headset_count++;
+			if (headset_count == JACK_DETECT_COUNT)
+				return SND_JACK_HEADSET;
+		}
+	}
+
+	dev_err(component->dev, "Error detecting headset vs headphones, bad contact?, assuming headphones\n");
+	return SND_JACK_HEADPHONE;
+}
+
+static void rt5651_jack_detect_work(struct work_struct *work)
+{
+	struct rt5651_priv *rt5651 =
+		container_of(work, struct rt5651_priv, jack_detect_work);
+	int report = 0;
+
+	if (rt5651_jack_inserted(rt5651->component)) {
+		rt5651_enable_micbias1_for_ovcd(rt5651->component);
+		report = rt5651_detect_headset(rt5651->component);
+		rt5651_disable_micbias1_for_ovcd(rt5651->component);
+	}
+
+	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
 
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, msecs_to_jiffies(250));
+	queue_work(system_power_efficient_wq, &rt5651->jack_detect_work);
 
 	return IRQ_HANDLED;
 }
@@ -1756,8 +1820,7 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	}
 
 	/* sync initial jack state */
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, 0);
+	queue_work(system_power_efficient_wq, &rt5651->jack_detect_work);
 
 	return 0;
 }
@@ -1922,41 +1985,6 @@ static const struct i2c_device_id rt5651_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5651_i2c_id);
 
-static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
-{
-	int jack_type;
-
-	if (jack_insert) {
-		rt5651_enable_micbias1_for_ovcd(component);
-		rt5651_clear_micbias1_ovcd(component);
-		msleep(100);
-		if (rt5651_micbias1_ovcd(component))
-			jack_type = SND_JACK_HEADPHONE;
-		else
-			jack_type = SND_JACK_HEADSET;
-		rt5651_disable_micbias1_for_ovcd(component);
-	} else { /* jack out */
-		jack_type = 0;
-	}
-
-	return jack_type;
-}
-
-static void rt5651_jack_detect_work(struct work_struct *work)
-{
-	struct rt5651_priv *rt5651 =
-		container_of(work, struct rt5651_priv, jack_detect_work.work);
-	int report, jack_inserted;
-
-	if (!rt5651->component)
-		return;
-
-	jack_inserted = rt5651_jack_inserted(rt5651->component);
-	report = rt5651_jack_detect(rt5651->component, jack_inserted);
-
-	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
-}
-
 static int rt5651_i2c_probe(struct i2c_client *i2c,
 		    const struct i2c_device_id *id)
 {
@@ -1995,7 +2023,7 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 	rt5651->irq = i2c->irq;
 	rt5651->hp_mute = 1;
 
-	INIT_DELAYED_WORK(&rt5651->jack_detect_work, rt5651_jack_detect_work);
+	INIT_WORK(&rt5651->jack_detect_work, rt5651_jack_detect_work);
 
 	ret = devm_snd_soc_register_component(&i2c->dev,
 				&soc_component_dev_rt5651,
@@ -2008,7 +2036,7 @@ static int rt5651_i2c_remove(struct i2c_client *i2c)
 {
 	struct rt5651_priv *rt5651 = i2c_get_clientdata(i2c);
 
-	cancel_delayed_work_sync(&rt5651->jack_detect_work);
+	cancel_work_sync(&rt5651->jack_detect_work);
 
 	return 0;
 }
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 5fb25f7e2a8c..6f0e4a8e1378 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2072,7 +2072,7 @@ struct rt5651_priv {
 	struct snd_soc_component *component;
 	struct regmap *regmap;
 	struct snd_soc_jack *hp_jack;
-	struct delayed_work jack_detect_work;
+	struct work_struct jack_detect_work;
 	enum rt5651_jd_src jd_src;
 
 	int irq;
-- 
2.14.3

[PATCH v2 22/32] ASoC: Intel: bytcr_rt5651: Actually honor the DMIC_EN quirk if specified

[Hans de Goede]

Before this commit it was possible to set the DMIC_EN quirk in the machine
driver, but it would never be passed to the codec driver so it was a nop.

This commit adds code to actually pass the quirk to the codec driver.

Since the DMIC_EN quirk was ignored before, this commit removes it from
the default quirk settings, to avoid this causing an unexpected functional
change. If we really want the DMIC_EN behavior anywhere it should be
specifically enabled by follow up commits.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 0af855d2df49..1cae89e0b843 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -56,8 +56,8 @@ enum {
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
 
-/* jack-detect-source + terminating empty entry */
-#define MAX_NO_PROPS 2
+/* jack-detect-source + dmic-en + terminating empty entry */
+#define MAX_NO_PROPS 3
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -65,7 +65,6 @@ struct byt_rt5651_private {
 };
 
 static unsigned long byt_rt5651_quirk = BYT_RT5651_DMIC_MAP |
-					BYT_RT5651_DMIC_EN |
 					BYT_RT5651_MCLK_EN;
 
 static void log_quirks(struct device *dev)
@@ -380,10 +379,15 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
 				BYT_RT5651_JDSRC(byt_rt5651_quirk));
 
+	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
+		props[cnt++] = PROPERTY_ENTRY_BOOL("realtek,dmic-en");
+
 	ret = device_add_properties(codec->dev, props);
 	if (ret)
 		return ret;
 
+	rt5651_apply_properties(codec);
+
 	ret = snd_soc_card_jack_new(runtime->card, "Headset",
 				    SND_JACK_HEADSET, &priv->jack,
 				    bytcr_jack_pins, ARRAY_SIZE(bytcr_jack_pins));
-- 
2.14.3

[PATCH v2 23/32] ASoC: Intel: bytcr_rt5651: Only create jack if we have a jack-detect source

[Hans de Goede]

Only create the jack if we have a valid jack-detect source and properly
check the snd_soc_component_set_jack() return value.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 1cae89e0b843..4a9026a55ce9 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -388,17 +388,21 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 
 	rt5651_apply_properties(codec);
 
-	ret = snd_soc_card_jack_new(runtime->card, "Headset",
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
+		ret = snd_soc_card_jack_new(runtime->card, "Headset",
 				    SND_JACK_HEADSET, &priv->jack,
 				    bytcr_jack_pins, ARRAY_SIZE(bytcr_jack_pins));
-	if (ret) {
-		dev_err(runtime->dev, "Headset jack creation failed %d\n", ret);
-		return ret;
-	}
+		if (ret) {
+			dev_err(runtime->dev, "jack creation failed %d\n", ret);
+			return ret;
+		}
 
-	snd_soc_component_set_jack(codec, &priv->jack, NULL);
+		ret = snd_soc_component_set_jack(codec, &priv->jack, NULL);
+		if (ret)
+			return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static const struct snd_soc_pcm_stream byt_rt5651_dai_params = {
-- 
2.14.3

[PATCH v2 24/32] ASoC: Intel: bytcr_rt5651: Add quirk micbias OVCD configuration

[Hans de Goede]

Add support for setting the micbias OVCD limits device-properties through
quirks.

And set the limits for this to 2000uA with a scale-factor of 0.75 for the
KIANO SlimNote 14.2 device, which is the only device on which
jack-detection is currently enabled.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 4a9026a55ce9..e44851448118 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -50,14 +50,29 @@ enum {
 	BYT_RT5651_JD2		= (RT5651_JD2 << 4),
 };
 
+enum {
+	BYT_RT5651_OVCD_TH_600UA  = (6 << 8),
+	BYT_RT5651_OVCD_TH_1500UA = (15 << 8),
+	BYT_RT5651_OVCD_TH_2000UA = (20 << 8),
+};
+
+enum {
+	BYT_RT5651_OVCD_SF_0P5	= (RT5651_OVCD_SF_0P5 << 13),
+	BYT_RT5651_OVCD_SF_0P75	= (RT5651_OVCD_SF_0P75 << 13),
+	BYT_RT5651_OVCD_SF_1P0	= (RT5651_OVCD_SF_1P0 << 13),
+	BYT_RT5651_OVCD_SF_1P5	= (RT5651_OVCD_SF_1P5 << 13),
+};
+
 #define BYT_RT5651_MAP(quirk)		((quirk) & GENMASK(3, 0))
 #define BYT_RT5651_JDSRC(quirk)		(((quirk) & GENMASK(7, 4)) >> 4)
+#define BYT_RT5651_OVCD_TH(quirk)	(((quirk) & GENMASK(12, 8)) >> 8)
+#define BYT_RT5651_OVCD_SF(quirk)	(((quirk) & GENMASK(14, 13)) >> 13)
 #define BYT_RT5651_DMIC_EN		BIT(16)
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
 
-/* jack-detect-source + dmic-en + terminating empty entry */
-#define MAX_NO_PROPS 3
+/* jack-detect-source + dmic-en + ovcd-th + -sf + terminating empty entry */
+#define MAX_NO_PROPS 5
 
 struct byt_rt5651_private {
 	struct clk *mclk;
@@ -77,9 +92,14 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
 		dev_info(dev, "quirk IN3_MAP enabled");
-	if (BYT_RT5651_JDSRC(byt_rt5651_quirk))
+	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
+		dev_info(dev, "quirk realtek,over-current-threshold %ld\n",
+			 BYT_RT5651_OVCD_TH(byt_rt5651_quirk) * 100);
+		dev_info(dev, "quirk realtek,over-current-scale-factor %ld\n",
+			 BYT_RT5651_OVCD_SF(byt_rt5651_quirk));
+	}
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		dev_info(dev, "quirk DMIC enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
@@ -303,6 +323,8 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
 					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
 	{}
@@ -379,6 +401,12 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
 				BYT_RT5651_JDSRC(byt_rt5651_quirk));
 
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,over-current-threshold",
+				BYT_RT5651_OVCD_TH(byt_rt5651_quirk) * 100);
+
+	props[cnt++] = PROPERTY_ENTRY_U32("realtek,over-current-scale-factor",
+				BYT_RT5651_OVCD_SF(byt_rt5651_quirk));
+
 	if (byt_rt5651_quirk & BYT_RT5651_DMIC_EN)
 		props[cnt++] = PROPERTY_ENTRY_BOOL("realtek,dmic-en");
 
-- 
2.14.3

[PATCH v2 25/32] ASoC: Intel: bytcr_rt5651: Configure PLL1 before using it

[Hans de Goede]

When platform_clock_control() first selects PLL1 as sysclk the PLL_CTRL
registers have not been setup yet and we effectively have an invalid clock
configuration until byt_rt5651_aif1_hw_params() gets called.

Add a new byt_rt5651_prepare_and_enable_pll1() helper and use that from
both platform_clock_control() and byt_rt5651_aif1_hw_params() to fix this.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Make bclk_ratio configurable instead of hardcoding it to 50, this will be
 used when adding SSP0 support
---
 sound/soc/intel/boards/bytcr_rt5651.c | 73 +++++++++++++++++------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index e44851448118..28219945912a 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -110,6 +110,38 @@ static void log_quirks(struct device *dev)
 
 #define BYT_CODEC_DAI1	"rt5651-aif1"
 
+static int byt_rt5651_prepare_and_enable_pll1(struct snd_soc_dai *codec_dai,
+					      int rate, int bclk_ratio)
+{
+	int clk_id, clk_freq, ret;
+
+	/* Configure the PLL before selecting it */
+	if (!(byt_rt5651_quirk & BYT_RT5651_MCLK_EN)) {
+		clk_id = RT5651_PLL1_S_BCLK1,
+		clk_freq = rate * bclk_ratio;
+	} else {
+		clk_id = RT5651_PLL1_S_MCLK;
+		if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ)
+			clk_freq = 25000000;
+		else
+			clk_freq = 19200000;
+	}
+	ret = snd_soc_dai_set_pll(codec_dai, 0, clk_id, clk_freq, rate * 512);
+	if (ret < 0) {
+		dev_err(codec_dai->codec->dev, "can't set pll: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
+				     rate * 512, SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		dev_err(codec_dai->codec->dev, "can't set clock %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int platform_clock_control(struct snd_soc_dapm_widget *w,
 				  struct snd_kcontrol *k, int  event)
 {
@@ -135,9 +167,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 				return ret;
 			}
 		}
-		ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
-					     48000 * 512,
-					     SND_SOC_CLOCK_IN);
+		ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);
 	} else {
 		/*
 		 * Set codec clock source to internal clock before
@@ -251,44 +281,11 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
-	int ret;
+	int rate = params_rate(params);
 
 	snd_soc_dai_set_bclk_ratio(codec_dai, 50);
 
-	ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
-				     params_rate(params) * 512,
-				     SND_SOC_CLOCK_IN);
-	if (ret < 0) {
-		dev_err(rtd->dev, "can't set codec clock %d\n", ret);
-		return ret;
-	}
-
-	if (!(byt_rt5651_quirk & BYT_RT5651_MCLK_EN)) {
-		/* 2x25 bit slots on SSP2 */
-		ret = snd_soc_dai_set_pll(codec_dai, 0,
-					RT5651_PLL1_S_BCLK1,
-					params_rate(params) * 50,
-					params_rate(params) * 512);
-	} else {
-		if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ) {
-			ret = snd_soc_dai_set_pll(codec_dai, 0,
-						RT5651_PLL1_S_MCLK,
-						25000000,
-						params_rate(params) * 512);
-		} else {
-			ret = snd_soc_dai_set_pll(codec_dai, 0,
-						RT5651_PLL1_S_MCLK,
-						19200000,
-						params_rate(params) * 512);
-		}
-	}
-
-	if (ret < 0) {
-		dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
-		return ret;
-	}
-
-	return 0;
+	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
 }
 
 static int byt_rt5651_quirk_cb(const struct dmi_system_id *id)
-- 
2.14.3

[PATCH v2 26/32] ASoC: Intel: bytcr_rt5651: Drop snd_soc_dai_set_bclk_ratio() call

[Hans de Goede]

Drop the snd_soc_dai_set_bclk_ratio() call, the rt5651 dai does not have a
set_bclk_ratio() op, so it is a nop (and returns -EINVAL).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 28219945912a..0c024e38cf47 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -283,8 +283,6 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int rate = params_rate(params);
 
-	snd_soc_dai_set_bclk_ratio(codec_dai, 50);
-
 	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
 }
 
-- 
2.14.3

[PATCH v2 27/32] ASoC: Intel: bytcr_rt5651: Rename IN3_MAP to IN1_HS_IN3_MAP

[Hans de Goede]

All the mappings are named for where the internal mic is routed and in that
sense the newly added in3_map really is the same as in1_map, what makes it
different is that it maps the headset mic at IN3 rather then at IN2.

Rename in3_map to in1_hs_in3_map to better reflect what it actually does.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 0c024e38cf47..c3b759de9a8d 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -40,7 +40,7 @@ enum {
 	BYT_RT5651_IN1_MAP,
 	BYT_RT5651_IN2_MAP,
 	BYT_RT5651_IN1_IN2_MAP,
-	BYT_RT5651_IN3_MAP,
+	BYT_RT5651_IN1_HS_IN3_MAP,
 };
 
 enum {
@@ -90,8 +90,8 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN1_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN2_MAP)
 		dev_info(dev, "quirk IN2_MAP enabled");
-	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
-		dev_info(dev, "quirk IN3_MAP enabled");
+	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN1_HS_IN3_MAP)
+		dev_info(dev, "quirk IN1_HS_IN3_MAP enabled");
 	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
@@ -234,8 +234,8 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_dmic_map[] = {
 
 static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_map[] = {
 	{"Internal Mic", NULL, "micbias1"},
-	{"IN2P", NULL, "Headset Mic"},
 	{"IN1P", NULL, "Internal Mic"},
+	{"IN2P", NULL, "Headset Mic"},
 };
 
 static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_map[] = {
@@ -251,10 +251,10 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_in2_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
-static const struct snd_soc_dapm_route byt_rt5651_intmic_in3_map[] = {
+static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_hs_in3_map[] = {
 	{"Internal Mic", NULL, "micbias1"},
-	{"IN3P", NULL, "Headset Mic"},
 	{"IN1P", NULL, "Internal Mic"},
+	{"IN3P", NULL, "Headset Mic"},
 };
 
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
@@ -299,7 +299,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Circuitco"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Max B3 PLATFORM"),
 		},
-		.driver_data = (void *)(BYT_RT5651_IN3_MAP),
+		.driver_data = (void *)(BYT_RT5651_IN1_HS_IN3_MAP),
 	},
 	{
 		.callback = byt_rt5651_quirk_cb,
@@ -308,7 +308,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Turbot"),
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
-					BYT_RT5651_IN3_MAP),
+					BYT_RT5651_IN1_HS_IN3_MAP),
 	},
 	{
 		.callback = byt_rt5651_quirk_cb,
@@ -350,9 +350,9 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 		custom_map = byt_rt5651_intmic_in1_in2_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_in2_map);
 		break;
-	case BYT_RT5651_IN3_MAP:
-		custom_map = byt_rt5651_intmic_in3_map;
-		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in3_map);
+	case BYT_RT5651_IN1_HS_IN3_MAP:
+		custom_map = byt_rt5651_intmic_in1_hs_in3_map;
+		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_hs_in3_map);
 		break;
 	default:
 		custom_map = byt_rt5651_intmic_dmic_map;
-- 
2.14.3

[PATCH v2 28/32] ASoC: Intel: bytcr_rt5651: Add new IN2_HS_IN3 input map and a quirk using it

[Hans de Goede]

Add a new IN2_HS_IN3 input map and add a quirk for the input mapping and
jack-detect source for the Chuwi Vi8 Plus tablet, which uses this new map.

Note the Chuwi Vi8 Plus lists an extra GPIO in its codecs ACPI resources
which needs to be driven high to enable the external speaker amplifier,
this is not supported yet and will be fixed in a future patch.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index c3b759de9a8d..aac4c8f2620a 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -41,6 +41,7 @@ enum {
 	BYT_RT5651_IN2_MAP,
 	BYT_RT5651_IN1_IN2_MAP,
 	BYT_RT5651_IN1_HS_IN3_MAP,
+	BYT_RT5651_IN2_HS_IN3_MAP,
 };
 
 enum {
@@ -92,6 +93,8 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN1_HS_IN3_MAP)
 		dev_info(dev, "quirk IN1_HS_IN3_MAP enabled");
+	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN2_HS_IN3_MAP)
+		dev_info(dev, "quirk IN2_HS_IN3_MAP enabled");
 	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
@@ -257,6 +260,12 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_hs_in3_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
+static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_hs_in3_map[] = {
+	{"Internal Mic", NULL, "micbias1"},
+	{"IN2P", NULL, "Internal Mic"},
+	{"IN3P", NULL, "Headset Mic"},
+};
+
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
@@ -322,6 +331,19 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
+	{
+		/* Chuwi Vi8 Plus (CWI519) */
+		.callback = byt_rt5651_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Hampoo"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "D2D3_Vi8A1"),
+		},
+		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
+					BYT_RT5651_IN2_HS_IN3_MAP),
+	},
 	{}
 };
 
@@ -354,6 +376,10 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 		custom_map = byt_rt5651_intmic_in1_hs_in3_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_hs_in3_map);
 		break;
+	case BYT_RT5651_IN2_HS_IN3_MAP:
+		custom_map = byt_rt5651_intmic_in2_hs_in3_map;
+		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in2_hs_in3_map);
+		break;
 	default:
 		custom_map = byt_rt5651_intmic_dmic_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_dmic_map);
-- 
2.14.3

[PATCH v2 29/32] ASoC: Intel: bytcr_rt5651: Add support for Bay Trail CR / SSP0 using boards

[Hans de Goede]

Despite its name being prefixed with bytcr, before this commit the
bytcr_rt5651 machine driver could not work with Bay Trail CR boards,
as those only have SSP0 and it only supported SSP0-AIF1 setups.

This commit adds support for this, autodetecting AIF1 vs AIF2 based on
BIOS tables.

While at it also add support for SSP2-AIF2 setups, as that requires only
minimal extra code on top of the code adding SSP0-AIF1 / SSP0-AIF2 support.

Note this code is all copy-pasted from bytcr_rt5640.c. I've looked into
merging the 2 machine drivers into 1 to avoid copy-pasting, but there are
enough subtile differences to make this hard *and* with all the quirks the
machine driver already is full with if (variant-foo) then ... else ...
constructs adding more of these is going to make the code unreadable.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Determine bclk_ratio to pass to byt_rt5651_prepare_and_enable_pll1()
 based on format in hw_params rather then on quirks, so that we only
 have to care about the ssp0 quirk in the fixup function and no in other
 places
---
 sound/soc/intel/boards/bytcr_rt5651.c | 217 +++++++++++++++++++++++++++++++---
 1 file changed, 201 insertions(+), 16 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index aac4c8f2620a..4b5fb25b16ec 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -26,6 +26,7 @@
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
+#include <asm/cpu_device_id.h>
 #include <asm/platform_sst_audio.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -71,6 +72,9 @@ enum {
 #define BYT_RT5651_DMIC_EN		BIT(16)
 #define BYT_RT5651_MCLK_EN		BIT(17)
 #define BYT_RT5651_MCLK_25MHZ		BIT(18)
+#define BYT_RT5651_SSP2_AIF2		BIT(19) /* default is using AIF1  */
+#define BYT_RT5651_SSP0_AIF1		BIT(20)
+#define BYT_RT5651_SSP0_AIF2		BIT(21)
 
 /* jack-detect-source + dmic-en + ovcd-th + -sf + terminating empty entry */
 #define MAX_NO_PROPS 5
@@ -80,8 +84,7 @@ struct byt_rt5651_private {
 	struct snd_soc_jack jack;
 };
 
-static unsigned long byt_rt5651_quirk = BYT_RT5651_DMIC_MAP |
-					BYT_RT5651_MCLK_EN;
+static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN;
 
 static void log_quirks(struct device *dev)
 {
@@ -109,9 +112,16 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk MCLK_EN enabled");
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ)
 		dev_info(dev, "quirk MCLK_25MHZ enabled");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2)
+		dev_info(dev, "quirk SSP2_AIF2 enabled\n");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1)
+		dev_info(dev, "quirk SSP0_AIF1 enabled\n");
+	if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)
+		dev_info(dev, "quirk SSP0_AIF2 enabled\n");
 }
 
 #define BYT_CODEC_DAI1	"rt5651-aif1"
+#define BYT_CODEC_DAI2	"rt5651-aif2"
 
 static int byt_rt5651_prepare_and_enable_pll1(struct snd_soc_dai *codec_dai,
 					      int rate, int bclk_ratio)
@@ -155,6 +165,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	int ret;
 
 	codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI1);
+	if (!codec_dai)
+		codec_dai = snd_soc_card_get_codec_dai(card, BYT_CODEC_DAI2);
 	if (!codec_dai) {
 		dev_err(card->dev,
 			"Codec dai not found; Unable to set platform clock\n");
@@ -212,13 +224,6 @@ static const struct snd_soc_dapm_route byt_rt5651_audio_map[] = {
 	{"Speaker", NULL, "Platform Clock"},
 	{"Line In", NULL, "Platform Clock"},
 
-	{"AIF1 Playback", NULL, "ssp2 Tx"},
-	{"ssp2 Tx", NULL, "codec_out0"},
-	{"ssp2 Tx", NULL, "codec_out1"},
-	{"codec_in0", NULL, "ssp2 Rx"},
-	{"codec_in1", NULL, "ssp2 Rx"},
-	{"ssp2 Rx", NULL, "AIF1 Capture"},
-
 	{"Headset Mic", NULL, "micbias1"}, /* lowercase for rt5651 */
 	{"Headphone", NULL, "HPOL"},
 	{"Headphone", NULL, "HPOR"},
@@ -266,6 +271,42 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_hs_in3_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
+static const struct snd_soc_dapm_route byt_rt5651_ssp0_aif1_map[] = {
+	{"ssp0 Tx", NULL, "modem_out"},
+	{"modem_in", NULL, "ssp0 Rx"},
+
+	{"AIF1 Playback", NULL, "ssp0 Tx"},
+	{"ssp0 Rx", NULL, "AIF1 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp0_aif2_map[] = {
+	{"ssp0 Tx", NULL, "modem_out"},
+	{"modem_in", NULL, "ssp0 Rx"},
+
+	{"AIF2 Playback", NULL, "ssp0 Tx"},
+	{"ssp0 Rx", NULL, "AIF2 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp2_aif1_map[] = {
+	{"ssp2 Tx", NULL, "codec_out0"},
+	{"ssp2 Tx", NULL, "codec_out1"},
+	{"codec_in0", NULL, "ssp2 Rx"},
+	{"codec_in1", NULL, "ssp2 Rx"},
+
+	{"AIF1 Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Rx", NULL, "AIF1 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5651_ssp2_aif2_map[] = {
+	{"ssp2 Tx", NULL, "codec_out0"},
+	{"ssp2 Tx", NULL, "codec_out1"},
+	{"codec_in0", NULL, "ssp2 Rx"},
+	{"codec_in1", NULL, "ssp2 Rx"},
+
+	{"AIF2 Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Rx", NULL, "AIF2 Capture"},
+};
+
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
@@ -290,9 +331,16 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	snd_pcm_format_t format = params_format(params);
 	int rate = params_rate(params);
+	int bclk_ratio;
 
-	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
+	if (format == SNDRV_PCM_FORMAT_S16_LE)
+		bclk_ratio = 32;
+	else
+		bclk_ratio = 50;
+
+	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, bclk_ratio);
 }
 
 static int byt_rt5651_quirk_cb(const struct dmi_system_id *id)
@@ -388,6 +436,26 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 	if (ret)
 		return ret;
 
+	if (byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp2_aif2_map,
+					ARRAY_SIZE(byt_rt5651_ssp2_aif2_map));
+	} else if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp0_aif1_map,
+					ARRAY_SIZE(byt_rt5651_ssp0_aif1_map));
+	} else if (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp0_aif2_map,
+					ARRAY_SIZE(byt_rt5651_ssp0_aif2_map));
+	} else {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5651_ssp2_aif1_map,
+					ARRAY_SIZE(byt_rt5651_ssp2_aif1_map));
+	}
+	if (ret)
+		return ret;
+
 	ret = snd_soc_add_card_controls(card, byt_rt5651_controls,
 					ARRAY_SIZE(byt_rt5651_controls));
 	if (ret) {
@@ -469,18 +537,26 @@ static int byt_rt5651_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 			SNDRV_PCM_HW_PARAM_RATE);
 	struct snd_interval *channels = hw_param_interval(params,
 						SNDRV_PCM_HW_PARAM_CHANNELS);
-	int ret;
+	int ret, bits;
 
-	/* The DSP will covert the FE rate to 48k, stereo, 24bits */
+	/* The DSP will covert the FE rate to 48k, stereo */
 	rate->min = rate->max = 48000;
 	channels->min = channels->max = 2;
 
-	/* set SSP2 to 24-bit */
-	params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* set SSP0 to 16-bit */
+		params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
+		bits = 16;
+	} else {
+		/* set SSP2 to 24-bit */
+		params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+		bits = 24;
+	}
 
 	/*
 	 * Default mode for SSP configuration is TDM 4 slot, override config
-	 * with explicit setting to I2S 2ch 24-bit. The word length is set with
+	 * with explicit setting to I2S 2ch. The word length is set with
 	 * dai_set_tdm_slot() since there is no other API exposed
 	 */
 	ret = snd_soc_dai_set_fmt(rtd->cpu_dai,
@@ -494,7 +570,7 @@ static int byt_rt5651_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 		return ret;
 	}
 
-	ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, 24);
+	ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, bits);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't set I2S config, err %d\n", ret);
 		return ret;
@@ -588,12 +664,32 @@ static struct snd_soc_card byt_rt5651_card = {
 };
 
 static char byt_rt5651_codec_name[SND_ACPI_I2C_ID_LEN];
+static char byt_rt5651_codec_aif_name[12]; /*  = "rt5651-aif[1|2]" */
+static char byt_rt5651_cpu_dai_name[10]; /*  = "ssp[0|2]-port" */
+
+static bool is_valleyview(void)
+{
+	static const struct x86_cpu_id cpu_ids[] = {
+		{ X86_VENDOR_INTEL, 6, 55 }, /* Valleyview, Bay Trail */
+		{}
+	};
+
+	if (!x86_match_cpu(cpu_ids))
+		return false;
+	return true;
+}
+
+struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
+	u64 aif_value;       /* 1: AIF1, 2: AIF2 */
+	u64 mclock_value;    /* usually 25MHz (0x17d7940), ignored */
+};
 
 static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 {
 	struct byt_rt5651_private *priv;
 	struct snd_soc_acpi_mach *mach;
 	const char *i2c_name = NULL;
+	bool is_bytcr = false;
 	int ret_val = 0;
 	int dai_index = 0;
 	int i;
@@ -625,10 +721,99 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 		byt_rt5651_dais[dai_index].codec_name = byt_rt5651_codec_name;
 	}
 
+	/*
+	 * swap SSP0 if bytcr is detected
+	 * (will be overridden if DMI quirk is detected)
+	 */
+	if (is_valleyview()) {
+		struct sst_platform_info *p_info = mach->pdata;
+		const struct sst_res_info *res_info = p_info->res_info;
+
+		if (res_info->acpi_ipc_irq_index == 0)
+			is_bytcr = true;
+	}
+
+	if (is_bytcr) {
+		/*
+		 * Baytrail CR platforms may have CHAN package in BIOS, try
+		 * to find relevant routing quirk based as done on Windows
+		 * platforms. We have to read the information directly from the
+		 * BIOS, at this stage the card is not created and the links
+		 * with the codec driver/pdata are non-existent
+		 */
+
+		struct acpi_chan_package chan_package;
+
+		/* format specified: 2 64-bit integers */
+		struct acpi_buffer format = {sizeof("NN"), "NN"};
+		struct acpi_buffer state = {0, NULL};
+		struct snd_soc_acpi_package_context pkg_ctx;
+		bool pkg_found = false;
+
+		state.length = sizeof(chan_package);
+		state.pointer = &chan_package;
+
+		pkg_ctx.name = "CHAN";
+		pkg_ctx.length = 2;
+		pkg_ctx.format = &format;
+		pkg_ctx.state = &state;
+		pkg_ctx.data_valid = false;
+
+		pkg_found = snd_soc_acpi_find_package_from_hid(mach->id,
+							       &pkg_ctx);
+		if (pkg_found) {
+			if (chan_package.aif_value == 1) {
+				dev_info(&pdev->dev, "BIOS Routing: AIF1 connected\n");
+				byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF1;
+			} else  if (chan_package.aif_value == 2) {
+				dev_info(&pdev->dev, "BIOS Routing: AIF2 connected\n");
+				byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
+			} else {
+				dev_info(&pdev->dev, "BIOS Routing isn't valid, ignored\n");
+				pkg_found = false;
+			}
+		}
+
+		if (!pkg_found) {
+			/* no BIOS indications, assume SSP0-AIF2 connection */
+			byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
+		}
+
+		/* change defaults for Baytrail-CR capture */
+		byt_rt5651_quirk |= BYT_RT5651_JD1_1 |
+				    BYT_RT5651_OVCD_TH_2000UA |
+				    BYT_RT5651_OVCD_SF_0P75 |
+				    BYT_RT5651_IN2_HS_IN3_MAP;
+	} else {
+		byt_rt5651_quirk |= BYT_RT5651_DMIC_MAP;
+	}
+
 	/* check quirks before creating card */
 	dmi_check_system(byt_rt5651_quirk_table);
 	log_quirks(&pdev->dev);
 
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP2_AIF2) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* fixup codec aif name */
+		snprintf(byt_rt5651_codec_aif_name,
+			sizeof(byt_rt5651_codec_aif_name),
+			"%s", "rt5651-aif2");
+
+		byt_rt5651_dais[dai_index].codec_dai_name =
+			byt_rt5651_codec_aif_name;
+	}
+
+	if ((byt_rt5651_quirk & BYT_RT5651_SSP0_AIF1) ||
+	    (byt_rt5651_quirk & BYT_RT5651_SSP0_AIF2)) {
+		/* fixup cpu dai name name */
+		snprintf(byt_rt5651_cpu_dai_name,
+			sizeof(byt_rt5651_cpu_dai_name),
+			"%s", "ssp0-port");
+
+		byt_rt5651_dais[dai_index].cpu_dai_name =
+			byt_rt5651_cpu_dai_name;
+	}
+
 	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {
 		priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
 		if (IS_ERR(priv->mclk)) {
-- 
2.14.3

[PATCH v2 30/32] ASoC: Intel: bytcr_rt5651: Add quirk for the VIOS LTH17 laptop

[Hans de Goede]

Add a quirk setting up jack-detect and input routing for the
VIOS LTH17 laptop.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 4b5fb25b16ec..e395c9c179a1 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -392,6 +392,19 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN2_HS_IN3_MAP),
 	},
+	{
+		/* VIOS LTH17 */
+		.callback = byt_rt5651_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "VIOS"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "LTH17"),
+		},
+		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_1P0 |
+					BYT_RT5651_IN1_IN2_MAP),
+	},
 	{}
 };
 
-- 
2.14.3

[PATCH v2 31/32] ASoC: Intel: bytcr_rt5651: Change defaults to enable jack-detect, analog mics

[Hans de Goede]

Change the default quirk settings to enable jack-detect, analog mics.

The old default input mapping of DMIC for non Bay Trail CR devices seems
like a poor default as I'm not aware of any Intel SST + rt5651 using
devices with a DMIC.

All Cherry Trail devices using the bytcr_rt5651 machine driver seem to be
modelled after BYT-CR devices, And the only non CR Bay Trail devices with
a rt5651 codec I'm aware of are the Minnow boards for which we already have
board specific quirks. So it seems better to me to use the BYT-CR defaults
everywhere.

This e.g. makes the Chuwi Hi8 Pro (CWI513) work ootb without needing a
quirk.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index e395c9c179a1..c695247341f3 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -84,7 +84,12 @@ struct byt_rt5651_private {
 	struct snd_soc_jack jack;
 };
 
-static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN;
+/* Default: jack-detect on JD1_1, internal mic on in2, headsetmic on in3 */
+static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
+					BYT_RT5651_IN2_HS_IN3_MAP;
 
 static void log_quirks(struct device *dev)
 {
@@ -791,14 +796,6 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 			/* no BIOS indications, assume SSP0-AIF2 connection */
 			byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
 		}
-
-		/* change defaults for Baytrail-CR capture */
-		byt_rt5651_quirk |= BYT_RT5651_JD1_1 |
-				    BYT_RT5651_OVCD_TH_2000UA |
-				    BYT_RT5651_OVCD_SF_0P75 |
-				    BYT_RT5651_IN2_HS_IN3_MAP;
-	} else {
-		byt_rt5651_quirk |= BYT_RT5651_DMIC_MAP;
 	}
 
 	/* check quirks before creating card */
-- 
2.14.3

[PATCH v2 32/32] ASoC: Intel: bytcr_rt5651: Select RCCLK on init()

[Hans de Goede]

When the BYT_RT5651_MCLK_EN quirk is set, we disable the MCLK from
byt_rt5651_init(), we need to select the RCCLK as sysclk before doing this
to make sure that jack-detect works directly after boot.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index c695247341f3..ceb3858a0e41 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -425,6 +425,11 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 
 	card->dapm.idle_bias_off = true;
 
+	/* Start with RC clk for jack-detect (we disable MCLK below) */
+	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
+		snd_soc_component_update_bits(codec, RT5651_GLB_CLK,
+			RT5651_SCLK_SRC_MASK, RT5651_SCLK_SRC_RCCLK);
+
 	switch (BYT_RT5651_MAP(byt_rt5651_quirk)) {
 	case BYT_RT5651_IN1_MAP:
 		custom_map = byt_rt5651_intmic_in1_map;
-- 
2.14.3

Re: [PATCH v2 00/32] ASoC: rt5651: jack-detect fixes and improvements

[Hans de Goede]

Hi,

On 25-02-18 11:46, Hans de Goede wrote:
> Hi All,
> 
> Here is v2 of my rt5651 codec and Intel machine-driver series which mainly
> consists of various jack-detect related fixes and improvements.
> 
> As discussed in the review of v1 I've made the following changes:
> 
> 1) Get rid of platform_data and use device-properties instead
> 
> 2) Split up a couple of the micbias / OVCD / jack-detect patches into
> smaller patches, as they were doing too much in a single patch
> 
> 3) Improved commit messages
> 
> 4) Concentrate the knowledge that SSP0 is 16 bits on BYTCR in the fixup
> function and extract the sample-format from the hw_params in other places

p.s.

This series was testen on the following devices:

    -Chuwi Hi8 Pro (Cherry Trail)
    -Chuwi Vi8 Plus (Cherry Trail)
    -I.T.Works TW701 (Bay Trail CR)
    -Trekstor Surftab Wintron 7.0 ST70416-6 (Bay Trail CR)

Regards,

Hans

Applied "ASoC: rt5651: Add rt5651_apply_properties() helper function" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Add rt5651_apply_properties() helper function

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 5f293d4354f8aae3c8617f83a3cbd30047676b4d Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:46 +0100
Subject: [PATCH] ASoC: rt5651: Add rt5651_apply_properties() helper function

Move the applying of the differential input and dmic properties to a new
rt5651_apply_properties() helper function. This new function can be called
by platform code which attaches properties after probe() has run to apply
these new properties.

Note this also moves the time when we apply these properties for DT
platforms from i2c-probe to snd-component-probe time, this should not
result in any functional difference.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 22 ++++++++++++++--------
 sound/soc/codecs/rt5651.h |  2 ++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 030889143249..4d93248b6401 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1659,6 +1659,18 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	return 0;
 }
 
+void rt5651_apply_properties(struct snd_soc_component *component)
+{
+	if (device_property_read_bool(component->dev, "realtek,in2-differential"))
+		snd_soc_component_update_bits(component, RT5651_IN1_IN2,
+				RT5651_IN_DF2, RT5651_IN_DF2);
+
+	if (device_property_read_bool(component->dev, "realtek,dmic-en"))
+		snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
+				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
+}
+EXPORT_SYMBOL_GPL(rt5651_apply_properties);
+
 static int rt5651_probe(struct snd_soc_component *component)
 {
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
@@ -1677,6 +1689,8 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
+	rt5651_apply_properties(component);
+
 	return 0;
 }
 
@@ -1932,14 +1946,6 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 	if (ret != 0)
 		dev_warn(&i2c->dev, "Failed to apply regmap patch: %d\n", ret);
 
-	if (device_property_read_bool(&i2c->dev, "realtek,in2-differential"))
-		regmap_update_bits(rt5651->regmap, RT5651_IN1_IN2,
-					RT5651_IN_DF2, RT5651_IN_DF2);
-
-	if (device_property_read_bool(&i2c->dev, "realtek,dmic-en"))
-		regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
-				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
-
 	rt5651->irq = i2c->irq;
 	rt5651->hp_mute = 1;
 
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index f3158488fc89..7d9d5fa09d6f 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2080,4 +2080,6 @@ struct rt5651_priv {
 	bool hp_mute;
 };
 
+void rt5651_apply_properties(struct snd_soc_component *component);
+
 #endif /* __RT5651_H__ */
-- 
2.16.2

Applied "ASoC: rt5651: Move 2 functions higher up in rt5651.c" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Move 2 functions higher up in rt5651.c

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d8b8c878e86593ad8bd86676a2a4c2be58a8b889 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:44 +0100
Subject: [PATCH] ASoC: rt5651: Move 2 functions higher up in rt5651.c

Move 2 functions higher up in rt5651.c, this is a preparation patch
to avoid needing forward declarations when moving over from a codec
private function to the standard snd_soc_component_set_jack().

This commit purely moves these 2 functions up, not a single line is
changed.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 154 +++++++++++++++++++++++-----------------------
 1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 390cab134ba0..c9698229fc33 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1583,6 +1583,83 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 	return 0;
 }
 
+static irqreturn_t rt5651_irq(int irq, void *data)
+{
+	struct rt5651_priv *rt5651 = data;
+
+	queue_delayed_work(system_power_efficient_wq,
+			   &rt5651->jack_detect_work, msecs_to_jiffies(250));
+
+	return IRQ_HANDLED;
+}
+
+int rt5651_set_jack_detect(struct snd_soc_component *component,
+			   struct snd_soc_jack *hp_jack)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
+	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	if (!rt5651->irq)
+		return -EINVAL;
+
+	/* IRQ output on GPIO1 */
+	snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
+		RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
+
+	/* Select jack detect source */
+	switch (rt5651->jd_src) {
+	case RT5651_JD1_1:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_1);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD1_1_IRQ_EN, RT5651_JD1_1_IRQ_EN);
+		break;
+	case RT5651_JD1_2:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_2);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD1_2_IRQ_EN, RT5651_JD1_2_IRQ_EN);
+		break;
+	case RT5651_JD2:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD2);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD2_IRQ_EN, RT5651_JD2_IRQ_EN);
+		break;
+	case RT5651_JD_NULL:
+		return 0;
+	default:
+		dev_err(component->dev, "Currently only JD1_1 / JD1_2 / JD2 are supported\n");
+		return -EINVAL;
+	}
+
+	snd_soc_dapm_force_enable_pin(dapm, "JD Power");
+	snd_soc_dapm_force_enable_pin(dapm, "LDO");
+	snd_soc_dapm_sync(dapm);
+
+	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
+
+	rt5651->hp_jack = hp_jack;
+
+	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
+					rt5651_irq,
+					IRQF_TRIGGER_RISING |
+					IRQF_TRIGGER_FALLING |
+					IRQF_ONESHOT, "rt5651", rt5651);
+	if (ret) {
+		dev_err(component->dev, "Failed to reguest IRQ: %d\n", ret);
+		return ret;
+	}
+
+	/* sync initial jack state */
+	queue_delayed_work(system_power_efficient_wq,
+			   &rt5651->jack_detect_work, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
+
 static int rt5651_probe(struct snd_soc_component *component)
 {
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
@@ -1752,16 +1829,6 @@ static const struct dmi_system_id rt5651_quirk_table[] = {
 	{}
 };
 
-static irqreturn_t rt5651_irq(int irq, void *data)
-{
-	struct rt5651_priv *rt5651 = data;
-
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, msecs_to_jiffies(250));
-
-	return IRQ_HANDLED;
-}
-
 static int rt5651_jack_detect(struct snd_soc_component *component, int jack_insert)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
@@ -1827,73 +1894,6 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
 }
 
-int rt5651_set_jack_detect(struct snd_soc_component *component,
-			   struct snd_soc_jack *hp_jack)
-{
-	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
-	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
-	int ret;
-
-	if (!rt5651->irq)
-		return -EINVAL;
-
-	/* IRQ output on GPIO1 */
-	snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
-		RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
-
-	/* Select jack detect source */
-	switch (rt5651->jd_src) {
-	case RT5651_JD1_1:
-		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
-			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_1);
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
-			RT5651_JD1_1_IRQ_EN, RT5651_JD1_1_IRQ_EN);
-		break;
-	case RT5651_JD1_2:
-		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
-			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_2);
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
-			RT5651_JD1_2_IRQ_EN, RT5651_JD1_2_IRQ_EN);
-		break;
-	case RT5651_JD2:
-		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
-			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD2);
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
-			RT5651_JD2_IRQ_EN, RT5651_JD2_IRQ_EN);
-		break;
-	case RT5651_JD_NULL:
-		return 0;
-	default:
-		dev_err(component->dev, "Currently only JD1_1 / JD1_2 / JD2 are supported\n");
-		return -EINVAL;
-	}
-
-	snd_soc_dapm_force_enable_pin(dapm, "JD Power");
-	snd_soc_dapm_force_enable_pin(dapm, "LDO");
-	snd_soc_dapm_sync(dapm);
-
-	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
-
-	rt5651->hp_jack = hp_jack;
-
-	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
-					rt5651_irq,
-					IRQF_TRIGGER_RISING |
-					IRQF_TRIGGER_FALLING |
-					IRQF_ONESHOT, "rt5651", rt5651);
-	if (ret) {
-		dev_err(component->dev, "Failed to reguest IRQ: %d\n", ret);
-		return ret;
-	}
-
-	/* sync initial jack state */
-	queue_delayed_work(system_power_efficient_wq,
-			   &rt5651->jack_detect_work, 0);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(rt5651_set_jack_detect);
-
 static int rt5651_i2c_probe(struct i2c_client *i2c,
 		    const struct i2c_device_id *id)
 {
-- 
2.16.2

Applied "ASoC: rt5651: Move all jack-detect initialization to rt5651_set_jack_detect" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Move all jack-detect initialization to rt5651_set_jack_detect

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From f06da4fdb5fc2b78d52f62c32aec65b7819178b0 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:43 +0100
Subject: [PATCH] ASoC: rt5651: Move all jack-detect initialization to
 rt5651_set_jack_detect

Move all jack-detect initialization to rt5651_set_jack_detect. The main
reason to do this is so that platform code can setup jack-detect properties
after the device has been probed, which unfortunately is necessary on some
platforms.

This has 2 additional advantages:
1) Grouping all jack-detect init together makes it easier to follow what
is happening and results in a small reduction in the number of loc.
2) Before we would register the irq handler before rt5651->hp_jack was
assigned, leading to a potential NULL deref if the jack_detect work runs
before the machine driver has called set_jack.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 119 ++++++++++++++++++++++------------------------
 sound/soc/codecs/rt5651.h |   1 +
 2 files changed, 58 insertions(+), 62 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index eecd6e662210..390cab134ba0 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1586,7 +1586,6 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 static int rt5651_probe(struct snd_soc_component *component)
 {
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
-	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 
 	rt5651->component = component;
 
@@ -1602,15 +1601,6 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
-	if (rt5651->jd_src) {
-		snd_soc_dapm_force_enable_pin(dapm, "JD Power");
-		snd_soc_dapm_force_enable_pin(dapm, "LDO");
-		snd_soc_dapm_sync(dapm);
-
-		regmap_update_bits(rt5651->regmap, RT5651_MICBIAS,
-				   0x38, 0x38);
-	}
-
 	return 0;
 }
 
@@ -1840,10 +1830,65 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 int rt5651_set_jack_detect(struct snd_soc_component *component,
 			   struct snd_soc_jack *hp_jack)
 {
+	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	if (!rt5651->irq)
+		return -EINVAL;
+
+	/* IRQ output on GPIO1 */
+	snd_soc_component_update_bits(component, RT5651_GPIO_CTRL1,
+		RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
+
+	/* Select jack detect source */
+	switch (rt5651->jd_src) {
+	case RT5651_JD1_1:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_1);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD1_1_IRQ_EN, RT5651_JD1_1_IRQ_EN);
+		break;
+	case RT5651_JD1_2:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD1_2);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD1_2_IRQ_EN, RT5651_JD1_2_IRQ_EN);
+		break;
+	case RT5651_JD2:
+		snd_soc_component_update_bits(component, RT5651_JD_CTRL2,
+			RT5651_JD_TRG_SEL_MASK, RT5651_JD_TRG_SEL_JD2);
+		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL1,
+			RT5651_JD2_IRQ_EN, RT5651_JD2_IRQ_EN);
+		break;
+	case RT5651_JD_NULL:
+		return 0;
+	default:
+		dev_err(component->dev, "Currently only JD1_1 / JD1_2 / JD2 are supported\n");
+		return -EINVAL;
+	}
+
+	snd_soc_dapm_force_enable_pin(dapm, "JD Power");
+	snd_soc_dapm_force_enable_pin(dapm, "LDO");
+	snd_soc_dapm_sync(dapm);
+
+	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
 
 	rt5651->hp_jack = hp_jack;
-	rt5651_irq(0, rt5651);
+
+	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
+					rt5651_irq,
+					IRQF_TRIGGER_RISING |
+					IRQF_TRIGGER_FALLING |
+					IRQF_ONESHOT, "rt5651", rt5651);
+	if (ret) {
+		dev_err(component->dev, "Failed to reguest IRQ: %d\n", ret);
+		return ret;
+	}
+
+	/* sync initial jack state */
+	queue_delayed_work(system_power_efficient_wq,
+			   &rt5651->jack_detect_work, 0);
 
 	return 0;
 }
@@ -1895,61 +1940,11 @@ static int rt5651_i2c_probe(struct i2c_client *i2c,
 		regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
 				RT5651_GP2_PIN_MASK, RT5651_GP2_PIN_DMIC1_SCL);
 
+	rt5651->irq = i2c->irq;
 	rt5651->hp_mute = 1;
 
-	if (rt5651->jd_src) {
-
-		/* IRQ output on GPIO1 */
-		regmap_update_bits(rt5651->regmap, RT5651_GPIO_CTRL1,
-				   RT5651_GP1_PIN_MASK, RT5651_GP1_PIN_IRQ);
-
-		switch (rt5651->jd_src) {
-		case RT5651_JD1_1:
-			regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
-					   RT5651_JD_TRG_SEL_MASK,
-					   RT5651_JD_TRG_SEL_JD1_1);
-			regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
-					   RT5651_JD1_1_IRQ_EN,
-					   RT5651_JD1_1_IRQ_EN);
-			break;
-		case RT5651_JD1_2:
-			regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
-					   RT5651_JD_TRG_SEL_MASK,
-					   RT5651_JD_TRG_SEL_JD1_2);
-			regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
-					   RT5651_JD1_2_IRQ_EN,
-					   RT5651_JD1_2_IRQ_EN);
-			break;
-		case RT5651_JD2:
-			regmap_update_bits(rt5651->regmap, RT5651_JD_CTRL2,
-					   RT5651_JD_TRG_SEL_MASK,
-					   RT5651_JD_TRG_SEL_JD2);
-			regmap_update_bits(rt5651->regmap, RT5651_IRQ_CTRL1,
-					   RT5651_JD2_IRQ_EN,
-					   RT5651_JD2_IRQ_EN);
-			break;
-		case RT5651_JD_NULL:
-			break;
-		default:
-			dev_warn(&i2c->dev, "Currently only JD1_1 / JD1_2 / JD2 are supported\n");
-			break;
-		}
-	}
-
 	INIT_DELAYED_WORK(&rt5651->jack_detect_work, rt5651_jack_detect_work);
 
-	if (i2c->irq) {
-		ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
-						rt5651_irq,
-						IRQF_TRIGGER_RISING |
-						IRQF_TRIGGER_FALLING |
-						IRQF_ONESHOT, "rt5651", rt5651);
-		if (ret) {
-			dev_err(&i2c->dev, "Failed to reguest IRQ: %d\n", ret);
-			return ret;
-		}
-	}
-
 	ret = devm_snd_soc_register_component(&i2c->dev,
 				&soc_component_dev_rt5651,
 				rt5651_dai, ARRAY_SIZE(rt5651_dai));
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 148e139e6a26..8f128d057ff0 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -2065,6 +2065,7 @@ struct rt5651_priv {
 	struct delayed_work jack_detect_work;
 	enum rt5651_jd_src jd_src;
 
+	int irq;
 	int sysclk;
 	int sysclk_src;
 	int lrck[RT5651_AIFS];
-- 
2.16.2

Applied "ASoC: rt5651: Always keep OVCD enabled" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Always keep OVCD enabled

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From f1088d4b813fb4d20fc9e8e8db5ca252923f25eb Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:57 +0100
Subject: [PATCH] ASoC: rt5651: Always keep OVCD enabled

OVCD is not only useful for jack-type detection, but is also useful
to protect against over-current faults in general, so always keep
OVCD enabled, instead of only enabling it for jack-type detection.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 41d7c192ac03..55bcc74e344f 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1642,7 +1642,7 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 				      RT5651_MIC1_OVTH_MASK |
 				      RT5651_PWR_CLK12M_MASK |
 				      RT5651_PWR_MB_MASK,
-				      RT5651_MIC1_OVCD_DIS |
+				      RT5651_MIC1_OVCD_EN |
 				      RT5651_MIC1_OVTH_600UA |
 				      RT5651_PWR_MB_PU |
 				      RT5651_PWR_CLK12M_PU);
@@ -1849,9 +1849,6 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 
 	if (jack_insert) {
 		rt5651_enable_micbias1_for_ovcd(component);
-		snd_soc_component_update_bits(component, RT5651_MICBIAS,
-				    RT5651_MIC1_OVCD_MASK,
-				    RT5651_MIC1_OVCD_EN);
 		msleep(100);
 		if (snd_soc_component_read32(component, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
 			jack_type = SND_JACK_HEADPHONE;
@@ -1862,10 +1859,6 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 		rt5651_disable_micbias1_for_ovcd(component);
 	} else { /* jack out */
 		jack_type = 0;
-
-		snd_soc_component_update_bits(component, RT5651_MICBIAS,
-				    RT5651_MIC1_OVCD_MASK,
-				    RT5651_MIC1_OVCD_DIS);
 	}
 
 	return jack_type;
-- 
2.16.2

Applied "ASoC: rt5651: Only configure OVCD once at set_jack time" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Only configure OVCD once at set_jack time

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 9e1795925d0e967d2a0191b1487b5caf0693f2ae Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:56 +0100
Subject: [PATCH] ASoC: rt5651: Only configure OVCD once at set_jack time

Only configure OVCD once at set_jack time, rather then configuring
it on every jack-insertion event and switch to using bit field defines
instead of hardcoding a magic value.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index be942886700d..41d7c192ac03 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1637,7 +1637,15 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
 		RT5651_PWR_JD_M, RT5651_PWR_JD_M);
 
-	snd_soc_component_update_bits(component, RT5651_MICBIAS, 0x38, 0x38);
+	snd_soc_component_update_bits(component, RT5651_MICBIAS,
+				      RT5651_MIC1_OVCD_MASK |
+				      RT5651_MIC1_OVTH_MASK |
+				      RT5651_PWR_CLK12M_MASK |
+				      RT5651_PWR_MB_MASK,
+				      RT5651_MIC1_OVCD_DIS |
+				      RT5651_MIC1_OVTH_600UA |
+				      RT5651_PWR_MB_PU |
+				      RT5651_PWR_CLK12M_PU);
 
 	rt5651->hp_jack = hp_jack;
 
@@ -1842,14 +1850,8 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 	if (jack_insert) {
 		rt5651_enable_micbias1_for_ovcd(component);
 		snd_soc_component_update_bits(component, RT5651_MICBIAS,
-				    RT5651_MIC1_OVCD_MASK |
-				    RT5651_MIC1_OVTH_MASK |
-				    RT5651_PWR_CLK12M_MASK |
-				    RT5651_PWR_MB_MASK,
-				    RT5651_MIC1_OVCD_EN |
-				    RT5651_MIC1_OVTH_600UA |
-				    RT5651_PWR_MB_PU |
-				    RT5651_PWR_CLK12M_PU);
+				    RT5651_MIC1_OVCD_MASK,
+				    RT5651_MIC1_OVCD_EN);
 		msleep(100);
 		if (snd_soc_component_read32(component, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
 			jack_type = SND_JACK_HEADPHONE;
-- 
2.16.2

Applied "ASoC: rt5651: Remove "JD Power" dapm supply" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Remove "JD Power" dapm supply

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 57d9d7c32fc24ca4de49392fec1fdd3db1f58e92 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:54 +0100
Subject: [PATCH] ASoC: rt5651: Remove "JD Power" dapm supply

Remove the "JD power" dapm supply which gets force-enabled once when
using jack-detect and never gets disabled again.

Since the PWR_JD_M bit simply needs to be always on when using jack-detect
there is no need to have it tracked by dapm, instead we can simply set it
to 1 once when initializing the jack-detection.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 9460bdcad349..cbf9f56ecb14 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -869,10 +869,6 @@ static const struct snd_soc_dapm_widget rt5651_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY_S("ADC ASRC", 1, RT5651_PLL_MODE_2,
 			      11, 0, NULL, 0),
 
-	/* Input Side */
-	SND_SOC_DAPM_SUPPLY("JD Power", RT5651_PWR_ANLG2,
-		RT5651_PWM_JD_M_BIT, 0, NULL, 0),
-
 	/* micbias */
 	SND_SOC_DAPM_SUPPLY("LDO", RT5651_PWR_ANLG1,
 			RT5651_PWR_LDO_BIT, 0, NULL, 0),
@@ -1616,7 +1612,10 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 		return -EINVAL;
 	}
 
-	snd_soc_dapm_force_enable_pin(dapm, "JD Power");
+	/* Enable jack detect power */
+	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
+		RT5651_PWR_JD_M, RT5651_PWR_JD_M);
+
 	snd_soc_dapm_force_enable_pin(dapm, "LDO");
 	snd_soc_dapm_sync(dapm);
 
-- 
2.16.2

Applied "ASoC: rt5651: Only configure LDO voltage once at boot" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Only configure LDO voltage once at boot

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 3d7719d3cc8efe88be4c72cb7cbb0d866ed7d1b2 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:53 +0100
Subject: [PATCH] ASoC: rt5651: Only configure LDO voltage once at boot

Now that rt5651_set_bias_level(BIAS_OFF) no longer modifies the LDO
voltage selection bits, there is no need to set them each time we move
to standby. Instead configure them once at component-probe() time.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 74f83af3b6be..9460bdcad349 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1540,9 +1540,6 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 			snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
 				RT5651_PWR_FV1 | RT5651_PWR_FV2,
 				RT5651_PWR_FV1 | RT5651_PWR_FV2);
-			snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
-				RT5651_PWR_LDO_DVO_MASK,
-				RT5651_PWR_LDO_DVO_1_2V);
 			snd_soc_component_update_bits(component, RT5651_D_MISC, 0x1, 0x1);
 		}
 		break;
@@ -1662,6 +1659,9 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	rt5651->component = component;
 
+	snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
+		RT5651_PWR_LDO_DVO_MASK, RT5651_PWR_LDO_DVO_1_2V);
+
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
 	rt5651_apply_properties(component);
-- 
2.16.2

Applied "ASoC: rt5651: Remove programming of PWR regs before force_bias_level() call" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Remove programming of PWR regs before force_bias_level() call

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From a25fe11746846f5dd3286cf690b470e3d049797f Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:52 +0100
Subject: [PATCH] ASoC: rt5651: Remove programming of PWR regs before
 force_bias_level() call

Remove the setup of the PWR_ANLG1 reg which was done directly before
calling snd_soc_component_force_bias_level(SND_SOC_BIAS_OFF), as the
latter will override any settings done to PWR_ANLG1 immediately anyways.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index d4fb16686889..74f83af3b6be 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1662,16 +1662,6 @@ static int rt5651_probe(struct snd_soc_component *component)
 
 	rt5651->component = component;
 
-	snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
-		RT5651_PWR_VREF1 | RT5651_PWR_MB |
-		RT5651_PWR_BG | RT5651_PWR_VREF2,
-		RT5651_PWR_VREF1 | RT5651_PWR_MB |
-		RT5651_PWR_BG | RT5651_PWR_VREF2);
-	usleep_range(10000, 15000);
-	snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
-		RT5651_PWR_FV1 | RT5651_PWR_FV2,
-		RT5651_PWR_FV1 | RT5651_PWR_FV2);
-
 	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
 
 	rt5651_apply_properties(component);
-- 
2.16.2

Applied "ASoC: rt5651: Do not modify jd and PLL power bits from set_bias_level()" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Do not modify jd and PLL power bits from set_bias_level()

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 887fcc6f0514380f17d5016dd8cdfc4d9a4437ee Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:51 +0100
Subject: [PATCH] ASoC: rt5651: Do not modify jd and PLL power bits from
 set_bias_level()

rt5651_set_bias_level(BIAS_OFF) used to unconditionally clear the entire
register, including the jack-detect and PLL power bits. When jack-detection
support was introduced a special case for jack-detect was added which
hard-codes a register value to keep both on.

This commit removes the jack-detect special case, instead simply leaving
these bits as is on BIAS_OFF.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index c93539ee2200..d4fb16686889 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1521,8 +1521,6 @@ static int rt5651_set_dai_pll(struct snd_soc_dai *dai, int pll_id, int source,
 static int rt5651_set_bias_level(struct snd_soc_component *component,
 			enum snd_soc_bias_level level)
 {
-	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
-
 	switch (level) {
 	case SND_SOC_BIAS_PREPARE:
 		if (SND_SOC_BIAS_STANDBY == snd_soc_component_get_bias_level(component)) {
@@ -1558,11 +1556,9 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 		/* Do not touch the LDO voltage select bits on bias-off */
 		snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
 			~RT5651_PWR_LDO_DVO_MASK, 0);
-		if (rt5651->jd_src) {
-			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0204);
-		} else {
-			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0000);
-		}
+		/* Leave PLL1 and jack-detect power as is, all others off */
+		snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
+				    ~(RT5651_PWR_PLL | RT5651_PWR_JD_M), 0);
 		break;
 
 	default:
-- 
2.16.2

Applied "ASoC: rt5651: Do not modify the LDO voltage control bits from set_bias_level()" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Do not modify the LDO voltage control bits from set_bias_level()

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From bba4e685dae8643469ca6fac22f10ca81554586e Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:50 +0100
Subject: [PATCH] ASoC: rt5651: Do not modify the LDO voltage control bits from
 set_bias_level()

The PWR_ANLG1 reg not only contains various power on/off bits, it also
contains 2 bits which select if the LDO generates 1.0, 1.1 or 1.2V. Note
there is a separate on/off bit for the LDO.

rt5651_set_bias_level(BIAS_OFF) used to unconditionally clear the entire
register, when jack-detection support was introduced a special case for
jack-detect was added which hard-codes a register value to keep the LDO
voltage at 1.2 volt.

This commit removes the jack-detect special case, instead simply always
leaving the LDO voltage control bits as is on BIAS_OFF.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index c565607f95f7..c93539ee2200 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1555,11 +1555,12 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 		snd_soc_component_write(component, RT5651_PWR_DIG2, 0x0000);
 		snd_soc_component_write(component, RT5651_PWR_VOL, 0x0000);
 		snd_soc_component_write(component, RT5651_PWR_MIXER, 0x0000);
+		/* Do not touch the LDO voltage select bits on bias-off */
+		snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
+			~RT5651_PWR_LDO_DVO_MASK, 0);
 		if (rt5651->jd_src) {
 			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0204);
-			snd_soc_component_write(component, RT5651_PWR_ANLG1, 0x0002);
 		} else {
-			snd_soc_component_write(component, RT5651_PWR_ANLG1, 0x0000);
 			snd_soc_component_write(component, RT5651_PWR_ANLG2, 0x0000);
 		}
 		break;
-- 
2.16.2

Applied "ASoC: rt5651: Fix bias_level confusion" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Fix bias_level confusion

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 984c803f9a2ad6f1d2bea0b7ef2e3c18d69fbdfd Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:49 +0100
Subject: [PATCH] ASoC: rt5651: Fix bias_level confusion

The rt5651_set_bias_level() function was turning everything off at
SND_SOC_BIAS_STANDBY, rather then at SND_SOC_BIAS_OFF, requiring the bias-
level to be raised to SND_SOC_BIAS_PREPARE before turning anything on.

This is not how the bias-levels are supposed to work, this commit fixes
this by turning everything off at the SND_SOC_BIAS_OFF level and enabling
the pwr-bits needed for minimum functionality at SND_SOC_BIAS_STANDBY.

This fixes the minimum set of pwr-bits not getting enabled when
force-enabling some dapm-supplies (e.g. for jack type detection),
which raises the bias-level to standby.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 8548f9406520..c565607f95f7 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1526,6 +1526,13 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 	switch (level) {
 	case SND_SOC_BIAS_PREPARE:
 		if (SND_SOC_BIAS_STANDBY == snd_soc_component_get_bias_level(component)) {
+			if (snd_soc_component_read32(component, RT5651_PLL_MODE_1) & 0x9200)
+				snd_soc_component_update_bits(component, RT5651_D_MISC,
+						    0xc00, 0xc00);
+		}
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		if (SND_SOC_BIAS_OFF == snd_soc_component_get_bias_level(component)) {
 			snd_soc_component_update_bits(component, RT5651_PWR_ANLG1,
 				RT5651_PWR_VREF1 | RT5651_PWR_MB |
 				RT5651_PWR_BG | RT5651_PWR_VREF2,
@@ -1539,13 +1546,10 @@ static int rt5651_set_bias_level(struct snd_soc_component *component,
 				RT5651_PWR_LDO_DVO_MASK,
 				RT5651_PWR_LDO_DVO_1_2V);
 			snd_soc_component_update_bits(component, RT5651_D_MISC, 0x1, 0x1);
-			if (snd_soc_component_read32(component, RT5651_PLL_MODE_1) & 0x9200)
-				snd_soc_component_update_bits(component, RT5651_D_MISC,
-						    0xc00, 0xc00);
 		}
 		break;
 
-	case SND_SOC_BIAS_STANDBY:
+	case SND_SOC_BIAS_OFF:
 		snd_soc_component_write(component, RT5651_D_MISC, 0x0010);
 		snd_soc_component_write(component, RT5651_PWR_DIG1, 0x0000);
 		snd_soc_component_write(component, RT5651_PWR_DIG2, 0x0000);
-- 
2.16.2

Applied "ASoC: rt5651: Remove is_sys_clk_from_pll()" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Remove is_sys_clk_from_pll()

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d082174c99e77beb06a51c90ac61934554e516e1 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 25 Feb 2018 11:46:48 +0100
Subject: [PATCH] ASoC: rt5651: Remove is_sys_clk_from_pll()

is_sys_clk_from_pll() is used as a snd_soc_dapm_route.connected callback,
checking RT5651_GBL_CLK to determine if the sys-clk is PLL1 and thus the
PWR_PLL bit in reg PWR_ANLG2 must be set.

RT5651_GBL_CLK is changed by rt5651_set_dai_sysclk(), which gets called by
the pre_pmu / post_pmd functions of the "Platform Clock" dapm-supply.

This creates an ordering issue, during a dapm transition first all
connected() callbacks are called to build a list of supplies to enable
and then the complete list is walked to enable the supplies. Since the
connected() check happens before enabling any supplies,
is_sys_clk_from_pll() ends up deciding if the PWR_PLL bit should be set
based on the state the "Platform Clock" supply had *before* the transition.
This sometimes results in PWR_PLL being off, even though *after* the
transition PLL1 is configured as sys-clk.

This commit removes is_sys_clk_from_pll() instead simply setting / clearing
PWR_PLL in rt5651_set_dai_sysclk() based on the selected sys-clk, which
fixes this and as a bonus results in a nice cleanup.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 4d93248b6401..8548f9406520 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -398,20 +398,6 @@ static int set_dmic_clk(struct snd_soc_dapm_widget *w,
 	return idx;
 }
 
-static int is_sysclk_from_pll(struct snd_soc_dapm_widget *source,
-			 struct snd_soc_dapm_widget *sink)
-{
-	struct snd_soc_component *component = snd_soc_dapm_to_component(source->dapm);
-	unsigned int val;
-
-	val = snd_soc_component_read32(component, RT5651_GLB_CLK);
-	val &= RT5651_SCLK_SRC_MASK;
-	if (val == RT5651_SCLK_SRC_PLL1)
-		return 1;
-	else
-		return 0;
-}
-
 /* Digital Mixer */
 static const struct snd_kcontrol_new rt5651_sto1_adc_l_mix[] = {
 	SOC_DAPM_SINGLE("ADC1 Switch", RT5651_STO1_ADC_MIXER,
@@ -883,8 +869,6 @@ static const struct snd_soc_dapm_widget rt5651_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY_S("ADC ASRC", 1, RT5651_PLL_MODE_2,
 			      11, 0, NULL, 0),
 
-	SND_SOC_DAPM_SUPPLY("PLL1", RT5651_PWR_ANLG2,
-			RT5651_PWR_PLL_BIT, 0, NULL, 0),
 	/* Input Side */
 	SND_SOC_DAPM_SUPPLY("JD Power", RT5651_PWR_ANLG2,
 		RT5651_PWM_JD_M_BIT, 0, NULL, 0),
@@ -1167,7 +1151,6 @@ static const struct snd_soc_dapm_route rt5651_dapm_routes[] = {
 	{"Stereo1 ADC MIXL", "ADC1 Switch", "Stereo1 ADC L1 Mux"},
 	{"Stereo1 ADC MIXL", "ADC2 Switch", "Stereo1 ADC L2 Mux"},
 	{"Stereo1 ADC MIXL", NULL, "Stereo1 Filter"},
-	{"Stereo1 Filter", NULL, "PLL1", is_sysclk_from_pll},
 	{"Stereo1 Filter", NULL, "ADC ASRC"},
 
 	{"Stereo1 ADC MIXR", "ADC1 Switch", "Stereo1 ADC R1 Mux"},
@@ -1177,7 +1160,6 @@ static const struct snd_soc_dapm_route rt5651_dapm_routes[] = {
 	{"Stereo2 ADC MIXL", "ADC1 Switch", "Stereo2 ADC L1 Mux"},
 	{"Stereo2 ADC MIXL", "ADC2 Switch", "Stereo2 ADC L2 Mux"},
 	{"Stereo2 ADC MIXL", NULL, "Stereo2 Filter"},
-	{"Stereo2 Filter", NULL, "PLL1", is_sysclk_from_pll},
 	{"Stereo2 Filter", NULL, "ADC ASRC"},
 
 	{"Stereo2 ADC MIXR", "ADC1 Switch", "Stereo2 ADC R1 Mux"},
@@ -1244,10 +1226,8 @@ static const struct snd_soc_dapm_route rt5651_dapm_routes[] = {
 	{"PDM R Mux", "DD MIX", "DAC MIXR"},
 
 	{"DAC L1", NULL, "Stereo DAC MIXL"},
-	{"DAC L1", NULL, "PLL1", is_sysclk_from_pll},
 	{"DAC L1", NULL, "DAC L1 Power"},
 	{"DAC R1", NULL, "Stereo DAC MIXR"},
-	{"DAC R1", NULL, "PLL1", is_sysclk_from_pll},
 	{"DAC R1", NULL, "DAC R1 Power"},
 
 	{"DD MIXL", "DAC L1 Switch", "DAC MIXL"},
@@ -1443,6 +1423,7 @@ static int rt5651_set_dai_sysclk(struct snd_soc_dai *dai,
 	struct snd_soc_component *component = dai->component;
 	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
 	unsigned int reg_val = 0;
+	unsigned int pll_bit = 0;
 
 	if (freq == rt5651->sysclk && clk_id == rt5651->sysclk_src)
 		return 0;
@@ -1453,6 +1434,7 @@ static int rt5651_set_dai_sysclk(struct snd_soc_dai *dai,
 		break;
 	case RT5651_SCLK_S_PLL1:
 		reg_val |= RT5651_SCLK_SRC_PLL1;
+		pll_bit |= RT5651_PWR_PLL;
 		break;
 	case RT5651_SCLK_S_RCCLK:
 		reg_val |= RT5651_SCLK_SRC_RCCLK;
@@ -1461,6 +1443,8 @@ static int rt5651_set_dai_sysclk(struct snd_soc_dai *dai,
 		dev_err(component->dev, "Invalid clock id (%d)\n", clk_id);
 		return -EINVAL;
 	}
+	snd_soc_component_update_bits(component, RT5651_PWR_ANLG2,
+		RT5651_PWR_PLL, pll_bit);
 	snd_soc_component_update_bits(component, RT5651_GLB_CLK,
 		RT5651_SCLK_SRC_MASK, reg_val);
 	rt5651->sysclk = freq;
-- 
2.16.2

Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1751 bytes --]

On Sun, Feb 25, 2018 at 11:46:47AM +0100, Hans de Goede wrote:
> Configure the jack-detect source through a device-property which can be
> set by code outside of the codec driver. Rather then putting platform
> specific DMI quirks inside the generic codec driver.

> And move the jack-detect-source quirk for the KIANO SlimNote 14.2, which
> was present inside the codec driver to the machine driver, where we can
> bundle it together with the other quirks already present for this laptop.

Multiple things in a patch again :/  The property itself is fine but...

> @@ -360,6 +377,13 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
>  			dev_err(card->dev, "unable to set MCLK rate\n");
>  	}
>  
> +	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
> +				BYT_RT5651_JDSRC(byt_rt5651_quirk));
> +
> +	ret = device_add_properties(codec->dev, props);
> +	if (ret)
> +		return ret;
> +
>  	ret = snd_soc_card_jack_new(runtime->card, "Headset",

I'm having a hard time geting comfortable with this; it's all a bit
fragile feeling to me - someone deciding to centralise the property
parsing (eg, if they are using a platform where platform data makes
sense or want to cross-validate with some other property on probe) could
easily break things and there's not even a comment in the CODEC driver.
It'll work but it doesn't seem to match expectations for how firmware
data is provided which is what's bugging me here.  It at least needs a
"here be dragons" style comment in the code.

Ideally we'd have a general way to massage the ACPI data at init time
and could have board files that are instantiated from DMI to do that
(like are needed even more for SFI) but that's a long way off.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede]

Hi,

On 01-03-18 20:30, Mark Brown wrote:
> On Sun, Feb 25, 2018 at 11:46:47AM +0100, Hans de Goede wrote:
>> Configure the jack-detect source through a device-property which can be
>> set by code outside of the codec driver. Rather then putting platform
>> specific DMI quirks inside the generic codec driver.
> 
>> And move the jack-detect-source quirk for the KIANO SlimNote 14.2, which
>> was present inside the codec driver to the machine driver, where we can
>> bundle it together with the other quirks already present for this laptop.
> 
> Multiple things in a patch again :/

Yes because the property replaces the quirk, I can split the changes between
the codec and machine driver into 2 patches but then the intermediate state
will be that the jack-detection no longer works.

> The property itself is fine but...
> 
>> @@ -360,6 +377,13 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
>>   			dev_err(card->dev, "unable to set MCLK rate\n");
>>   	}
>>   
>> +	props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
>> +				BYT_RT5651_JDSRC(byt_rt5651_quirk));
>> +
>> +	ret = device_add_properties(codec->dev, props);
>> +	if (ret)
>> +		return ret;
>> +
>>   	ret = snd_soc_card_jack_new(runtime->card, "Headset",
> 
> I'm having a hard time geting comfortable with this; it's all a bit
> fragile feeling to me - someone deciding to centralise the property
> parsing (eg, if they are using a platform where platform data makes
> sense or want to cross-validate with some other property on probe) could
> easily break things and there's not even a comment in the CODEC driver.

It is not _that_ fragile, but I agree that it would be good to add
a comment about not moving the property-parsing to the codec driver.

I will rebase and submit a new version with a comment added.
Do you want me to split this patch into separate codec and machine
driver patches while at it?

> It'll work but it doesn't seem to match expectations for how firmware
> data is provided which is what's bugging me here.

The problem here is that the data is not coming from the firmware,
which is a reality we have to deal with leading to some sort of
compromise.

> It at least needs a
> "here be dragons" style comment in the code.

Ack.

> Ideally we'd have a general way to massage the ACPI data at init time
> and could have board files that are instantiated from DMI to do that
> (like are needed even more for SFI) but that's a long way off.

Something like DT overlays for ACPI? With the overlays being triggered
by a DMI match? Interesting concept, but given how much discussion there
still is surrounding DT overlays I don't see this happening anytime soon.

Regards,

Hans

Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede]

Hi,

On 01-03-18 23:35, Hans de Goede wrote:
> Hi,
> 
> On 01-03-18 20:30, Mark Brown wrote:
>> On Sun, Feb 25, 2018 at 11:46:47AM +0100, Hans de Goede wrote:
>>> Configure the jack-detect source through a device-property which can be
>>> set by code outside of the codec driver. Rather then putting platform
>>> specific DMI quirks inside the generic codec driver.
>>
>>> And move the jack-detect-source quirk for the KIANO SlimNote 14.2, which
>>> was present inside the codec driver to the machine driver, where we can
>>> bundle it together with the other quirks already present for this laptop.
>>
>> Multiple things in a patch again :/
> 
> Yes because the property replaces the quirk, I can split the changes between
> the codec and machine driver into 2 patches but then the intermediate state
> will be that the jack-detection no longer works.
> 
>> The property itself is fine but...
>>
>>> @@ -360,6 +377,13 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
>>>               dev_err(card->dev, "unable to set MCLK rate\n");
>>>       }
>>> +    props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
>>> +                BYT_RT5651_JDSRC(byt_rt5651_quirk));
>>> +
>>> +    ret = device_add_properties(codec->dev, props);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>>       ret = snd_soc_card_jack_new(runtime->card, "Headset",
>>
>> I'm having a hard time geting comfortable with this; it's all a bit
>> fragile feeling to me - someone deciding to centralise the property
>> parsing (eg, if they are using a platform where platform data makes
>> sense or want to cross-validate with some other property on probe) could
>> easily break things and there's not even a comment in the CODEC driver.
> 
> It is not _that_ fragile, but I agree that it would be good to add
> a comment about not moving the property-parsing to the codec driver.

So one other solution which comes to mind here is to move the
snd_soc_card_jack_new() call into the codec driver's
rt5651_apply_properties() function (conditional on a jack src being
set in the properties).

This would make the machine driver code look like this:

	props[cnt++] = PROPERTY_ENTRY_...
	props[cnt++] = PROPERTY_ENTRY_...
	props[cnt++] = PROPERTY_ENTRY_...
	props[cnt++] = PROPERTY_ENTRY_...

	ret = device_add_properties(codec->dev, props);
	if (ret)
		return ret;

	ret = rt5651_apply_properties(codec);
	if (ret)
		return ret;

Which makes the ordering really clear without needing any
comments.

This will also make the jack-detect device-properties work with
devicetree platforms without any changes to their platform code,
which currently is not the case since the non Intel platform-code
does not call snd_soc_component_set_jack().

Thinking more about this I believe that doing the
snd_soc_card_jack_new() call inside the codec driver based on
device-properties is a better solution then doing it in the
machine driver.

Please let me know if you agree then I will rework the remaining
patches accordingly.

Regards,

Hans

Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2017 bytes --]

On Fri, Mar 02, 2018 at 10:32:25AM +0100, Hans de Goede wrote:
> On 01-03-18 23:35, Hans de Goede wrote:

> > It is not _that_ fragile, but I agree that it would be good to add
> > a comment about not moving the property-parsing to the codec driver.

> So one other solution which comes to mind here is to move the
> snd_soc_card_jack_new() call into the codec driver's
> rt5651_apply_properties() function (conditional on a jack src being
> set in the properties).

> This would make the machine driver code look like this:
> 
> 	props[cnt++] = PROPERTY_ENTRY_...
> 	props[cnt++] = PROPERTY_ENTRY_...
> 	props[cnt++] = PROPERTY_ENTRY_...
> 	props[cnt++] = PROPERTY_ENTRY_...

> 	ret = device_add_properties(codec->dev, props);
> 	if (ret)
> 		return ret;

> 	ret = rt5651_apply_properties(codec);
> 	if (ret)
> 		return ret;

> Which makes the ordering really clear without needing any
> comments.

It's still unusual to have something outside the driver trigger property
parsing, though the EXPORT_SYMBOL_GPL() would make that apparent.

> This will also make the jack-detect device-properties work with
> devicetree platforms without any changes to their platform code,
> which currently is not the case since the non Intel platform-code
> does not call snd_soc_component_set_jack().

What makes you claim that?  Any board with jack detection wired up will
call that function.  Not all boards have that configured of course but
there's absolutely nothing Intel specific about that interface.

> Thinking more about this I believe that doing the
> snd_soc_card_jack_new() call inside the codec driver based on
> device-properties is a better solution then doing it in the
> machine driver.

No, that's really not a good idea.  Any jacks in the system are part of
the system specific wiring, they're not something that's intrinsic to
the CODEC.  Even with fairly basic setups you've got options like having
a headset jack or separate microphone and headphone jacks.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede]

Hi,

On 02-03-18 13:18, Mark Brown wrote:
> On Fri, Mar 02, 2018 at 10:32:25AM +0100, Hans de Goede wrote:
>> On 01-03-18 23:35, Hans de Goede wrote:
> 
>>> It is not _that_ fragile, but I agree that it would be good to add
>>> a comment about not moving the property-parsing to the codec driver.
> 
>> So one other solution which comes to mind here is to move the
>> snd_soc_card_jack_new() call into the codec driver's
>> rt5651_apply_properties() function (conditional on a jack src being
>> set in the properties).
> 
>> This would make the machine driver code look like this:
>>
>> 	props[cnt++] = PROPERTY_ENTRY_...
>> 	props[cnt++] = PROPERTY_ENTRY_...
>> 	props[cnt++] = PROPERTY_ENTRY_...
>> 	props[cnt++] = PROPERTY_ENTRY_...
> 
>> 	ret = device_add_properties(codec->dev, props);
>> 	if (ret)
>> 		return ret;
> 
>> 	ret = rt5651_apply_properties(codec);
>> 	if (ret)
>> 		return ret;
> 
>> Which makes the ordering really clear without needing any
>> comments.
> 
> It's still unusual to have something outside the driver trigger property
> parsing, though the EXPORT_SYMBOL_GPL() would make that apparent.
> 
>> This will also make the jack-detect device-properties work with
>> devicetree platforms without any changes to their platform code,
>> which currently is not the case since the non Intel platform-code
>> does not call snd_soc_component_set_jack().
> 
> What makes you claim that?  Any board with jack detection wired up will
> call that function.  Not all boards have that configured of course but
> there's absolutely nothing Intel specific about that interface.

Right I'm not claiming the interface is Intel specific, what I'm trying
to say is that the need for some code outside of the codec driver
to create the jack means that adding jack-support to DT platforms
cannot be done through just adding DT properties (once this patch
is merged), it will still require platform code changes.

That is fine, I was just thinking it would be convenient if DT
platforms could lift on the jack-detect work being done for
the rt5651 driver now with just some DT changes and no other
changes required.

>> Thinking more about this I believe that doing the
>> snd_soc_card_jack_new() call inside the codec driver based on
>> device-properties is a better solution then doing it in the
>> machine driver.
> 
> No, that's really not a good idea.  Any jacks in the system are part of
> the system specific wiring, they're not something that's intrinsic to
> the CODEC.  Even with fairly basic setups you've got options like having
> a headset jack or separate microphone and headphone jacks.

OK, so if doing the jack creation in the machine-driver / platform
code is by design and you want to keep things that way, then I
assume that what needs changing for v3 of this patch is:

1) Split the patch in separate codec + machine-drv patches
2) Add comments to make the ordering requirements clear to
both the codec- and machine-driver.

Correct?

Regards,

Hans

Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1745 bytes --]

On Fri, Mar 02, 2018 at 01:58:53PM +0100, Hans de Goede wrote:
> On 02-03-18 13:18, Mark Brown wrote:

> > What makes you claim that?  Any board with jack detection wired up will
> > call that function.  Not all boards have that configured of course but
> > there's absolutely nothing Intel specific about that interface.

> Right I'm not claiming the interface is Intel specific, what I'm trying
> to say is that the need for some code outside of the codec driver
> to create the jack means that adding jack-support to DT platforms
> cannot be done through just adding DT properties (once this patch
> is merged), it will still require platform code changes.

On DT the situation with generic drivers is much better than it is on
ACPI so we're actually able to create jacks purely from DT with both the
simple and graph cards.  This was what drove the creation of the generic
jack operation rather than device specific function calls, Bard realized
that the device specific calls were all very similar and adding the call
allowed the generic cards to work with jacks.

> > No, that's really not a good idea.  Any jacks in the system are part of
> > the system specific wiring, they're not something that's intrinsic to
> > the CODEC.  Even with fairly basic setups you've got options like having
> > a headset jack or separate microphone and headphone jacks.

> OK, so if doing the jack creation in the machine-driver / platform
> code is by design and you want to keep things that way, then I
> assume that what needs changing for v3 of this patch is:

> 1) Split the patch in separate codec + machine-drv patches
> 2) Add comments to make the ordering requirements clear to
> both the codec- and machine-driver.

> Correct?

Yes, I think so.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Rob Herring]

On Fri, Mar 02, 2018 at 01:58:53PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 02-03-18 13:18, Mark Brown wrote:
> > On Fri, Mar 02, 2018 at 10:32:25AM +0100, Hans de Goede wrote:
> > > On 01-03-18 23:35, Hans de Goede wrote:
> > 
> > > > It is not _that_ fragile, but I agree that it would be good to add
> > > > a comment about not moving the property-parsing to the codec driver.
> > 
> > > So one other solution which comes to mind here is to move the
> > > snd_soc_card_jack_new() call into the codec driver's
> > > rt5651_apply_properties() function (conditional on a jack src being
> > > set in the properties).
> > 
> > > This would make the machine driver code look like this:
> > > 
> > > 	props[cnt++] = PROPERTY_ENTRY_...
> > > 	props[cnt++] = PROPERTY_ENTRY_...
> > > 	props[cnt++] = PROPERTY_ENTRY_...
> > > 	props[cnt++] = PROPERTY_ENTRY_...
> > 
> > > 	ret = device_add_properties(codec->dev, props);
> > > 	if (ret)
> > > 		return ret;
> > 
> > > 	ret = rt5651_apply_properties(codec);
> > > 	if (ret)
> > > 		return ret;
> > 
> > > Which makes the ordering really clear without needing any
> > > comments.
> > 
> > It's still unusual to have something outside the driver trigger property
> > parsing, though the EXPORT_SYMBOL_GPL() would make that apparent.
> > 
> > > This will also make the jack-detect device-properties work with
> > > devicetree platforms without any changes to their platform code,
> > > which currently is not the case since the non Intel platform-code
> > > does not call snd_soc_component_set_jack().
> > 
> > What makes you claim that?  Any board with jack detection wired up will
> > call that function.  Not all boards have that configured of course but
> > there's absolutely nothing Intel specific about that interface.
> 
> Right I'm not claiming the interface is Intel specific, what I'm trying
> to say is that the need for some code outside of the codec driver
> to create the jack means that adding jack-support to DT platforms
> cannot be done through just adding DT properties (once this patch
> is merged), it will still require platform code changes.
> 
> That is fine, I was just thinking it would be convenient if DT
> platforms could lift on the jack-detect work being done for
> the rt5651 driver now with just some DT changes and no other
> changes required.
> 
> > > Thinking more about this I believe that doing the
> > > snd_soc_card_jack_new() call inside the codec driver based on
> > > device-properties is a better solution then doing it in the
> > > machine driver.
> > 
> > No, that's really not a good idea.  Any jacks in the system are part of
> > the system specific wiring, they're not something that's intrinsic to
> > the CODEC.  Even with fairly basic setups you've got options like having
> > a headset jack or separate microphone and headphone jacks.
> 
> OK, so if doing the jack creation in the machine-driver / platform
> code is by design and you want to keep things that way, then I
> assume that what needs changing for v3 of this patch is:
> 
> 1) Split the patch in separate codec + machine-drv patches
> 2) Add comments to make the ordering requirements clear to
> both the codec- and machine-driver.
> 
> Correct?

And split bindings to separate patch please.

Rob

Re: [PATCH v2 17/32] ASoC: rt5651: Allow specifying over-current thresholds through device-properties

[Rob Herring]

On Sun, Feb 25, 2018 at 11:46:58AM +0100, Hans de Goede wrote:
> OVer-Current-Detection (OVCD) for the micbias current is used to detect if
> an inserted jack is a headset or headphones (mic shorted to ground).
> 
> Some boards may need different values for the OVCD threshold because of a
> resistor on the board in serial with or parallel to the jack mic contact.
> 
> This commit adds support for the sofar unset OVCD scale-factor register
> values and adds support for specifying non-default values for both the
> current threshold and the scale-factor through device-properties.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  Documentation/devicetree/bindings/sound/rt5651.txt | 11 ++++++

Please split to a separate patch.

>  include/sound/rt5651.h                             | 11 ++++++
>  sound/soc/codecs/rt5651.c                          | 42 +++++++++++++++++++++-
>  sound/soc/codecs/rt5651.h                          | 10 ++++++
>  4 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/rt5651.txt b/Documentation/devicetree/bindings/sound/rt5651.txt
> index 1632c0cf6123..8cb7ee8e79cf 100644
> --- a/Documentation/devicetree/bindings/sound/rt5651.txt
> +++ b/Documentation/devicetree/bindings/sound/rt5651.txt
> @@ -22,6 +22,17 @@ Optional properties:
>    2: Use JD1_2 pin for jack-dectect
>    3: Use JD2 pin for jack-dectect
>  
> +- realtek,over-current-threshold
> +  u32, micbias over-current detection threshold in µA, valid values are
> +  600, 1500 and 2000µA.

Needs a unit suffix as defined in property-units.txt

> +
> +- realtek,over-current-scale-factor
> +  u32, micbias over-current detection scale-factor, valid values are:
> +  0: Scale current by 0.5
> +  1: Scale current by 0.75
> +  2: Scale current by 1.0
> +  3: Scale current by 1.5
> +
>  Pins on the device (for linking into audio routes) for RT5651:
>  
>    * DMIC L1

Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede]

Hi,

On 02-03-18 18:41, Mark Brown wrote:
> On Fri, Mar 02, 2018 at 01:58:53PM +0100, Hans de Goede wrote:
>> On 02-03-18 13:18, Mark Brown wrote:
> 
>>> What makes you claim that?  Any board with jack detection wired up will
>>> call that function.  Not all boards have that configured of course but
>>> there's absolutely nothing Intel specific about that interface.
> 
>> Right I'm not claiming the interface is Intel specific, what I'm trying
>> to say is that the need for some code outside of the codec driver
>> to create the jack means that adding jack-support to DT platforms
>> cannot be done through just adding DT properties (once this patch
>> is merged), it will still require platform code changes.
> 
> On DT the situation with generic drivers is much better than it is on
> ACPI so we're actually able to create jacks purely from DT with both the
> simple and graph cards.  This was what drove the creation of the generic
> jack operation rather than device specific function calls, Bard realized
> that the device specific calls were all very similar and adding the call
> allowed the generic cards to work with jacks.
> 
>>> No, that's really not a good idea.  Any jacks in the system are part of
>>> the system specific wiring, they're not something that's intrinsic to
>>> the CODEC.  Even with fairly basic setups you've got options like having
>>> a headset jack or separate microphone and headphone jacks.
> 
>> OK, so if doing the jack creation in the machine-driver / platform
>> code is by design and you want to keep things that way, then I
>> assume that what needs changing for v3 of this patch is:
> 
>> 1) Split the patch in separate codec + machine-drv patches
>> 2) Add comments to make the ordering requirements clear to
>> both the codec- and machine-driver.
> 
>> Correct?
> 
> Yes, I think so.

Actually I've come up with a nicer way to deal with the ordering issues
around setting the device-properties from the machine-driver.

If we attach the properties in the machine-driver *before* calling
snd_soc_register_card() and check them from the codec/component driver's
probe function (rather then from the i2c_driver.probe function), then
there is no need for a codec private apply_properties() function to
get the ordering right, since the codec/component driver's probe
function is called from snd_soc_register_card().

This still needs some comments in both the codec and machine driver
to make sure that future changes don't cause issues, but it gets rid
of the ugly apply_properties() function call from the machine-driver.

I'm currently preparing a v3 with this change + other requested changes.
I need to re-run a bunch of tests before posting, so I hope to post the
v3 series tomorrow.

Regards,

Hans

Applied "ASoC: rt5651: Add rt5651_jack_inserted() helper" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Add rt5651_jack_inserted() helper

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 0fe9474598890b4bd8a7228c9b1872ed7fe3c2c5 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:56 +0100
Subject: [PATCH] ASoC: rt5651: Add rt5651_jack_inserted() helper

Add rt5651_jack_inserted() helper to get the jack-detect switch status,
This is a preparation patch for rewriting the jack type-detection to
make it more reliable.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 873af6798d7a..41fc574dbc3d 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1597,6 +1597,31 @@ static bool rt5651_micbias1_ovcd(struct snd_soc_component *component)
 	return (val & RT5651_MB1_OC_CLR);
 }
 
+static bool rt5651_jack_inserted(struct snd_soc_component *component)
+{
+	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+	int val;
+
+	val = snd_soc_component_read32(component, RT5651_INT_IRQ_ST);
+	dev_dbg(component->dev, "irq status %#04x\n", val);
+
+	switch (rt5651->jd_src) {
+	case RT5651_JD1_1:
+		val &= 0x1000;
+		break;
+	case RT5651_JD1_2:
+		val &= 0x2000;
+		break;
+	case RT5651_JD2:
+		val &= 0x4000;
+		break;
+	default:
+		break;
+	}
+
+	return val == 0;
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
@@ -1927,27 +1952,13 @@ static void rt5651_jack_detect_work(struct work_struct *work)
 {
 	struct rt5651_priv *rt5651 =
 		container_of(work, struct rt5651_priv, jack_detect_work.work);
-
-	int report, val = 0;
+	int report, jack_inserted;
 
 	if (!rt5651->component)
 		return;
 
-	switch (rt5651->jd_src) {
-	case RT5651_JD1_1:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x1000;
-		break;
-	case RT5651_JD1_2:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x2000;
-		break;
-	case RT5651_JD2:
-		val = snd_soc_component_read32(rt5651->component, RT5651_INT_IRQ_ST) & 0x4000;
-		break;
-	default:
-		break;
-	}
-
-	report = rt5651_jack_detect(rt5651->component, !val);
+	jack_inserted = rt5651_jack_inserted(rt5651->component);
+	report = rt5651_jack_detect(rt5651->component, jack_inserted);
 
 	snd_soc_jack_report(rt5651->hp_jack, report, SND_JACK_HEADSET);
 }
-- 
2.16.2

Applied "ASoC: rt5651: Enable Platform Clock during jack-type detect" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Enable Platform Clock during jack-type detect

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 4b4a373c0212e084045530e47071374a87a3761e Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:55 +0100
Subject: [PATCH] ASoC: rt5651: Enable Platform Clock during jack-type detect

When using RCCLK instead of MCLK / PLL1 the OVCD status often gets stuck
at its last value, which breaks jack-type detection.

This commit fixes this by force-enabling the platform clock when doing
jack-type detection.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 62918f7e8270..873af6798d7a 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1563,6 +1563,8 @@ static void rt5651_enable_micbias1_for_ovcd(struct snd_soc_component *component)
 	snd_soc_dapm_mutex_lock(dapm);
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "LDO");
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "micbias1");
+	/* OVCD is unreliable when used with RCCLK as sysclk-source */
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "Platform Clock");
 	snd_soc_dapm_sync_unlocked(dapm);
 	snd_soc_dapm_mutex_unlock(dapm);
 }
@@ -1572,6 +1574,7 @@ static void rt5651_disable_micbias1_for_ovcd(struct snd_soc_component *component
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 
 	snd_soc_dapm_mutex_lock(dapm);
+	snd_soc_dapm_disable_pin_unlocked(dapm, "Platform Clock");
 	snd_soc_dapm_disable_pin_unlocked(dapm, "micbias1");
 	snd_soc_dapm_disable_pin_unlocked(dapm, "LDO");
 	snd_soc_dapm_sync_unlocked(dapm);
-- 
2.16.2

Applied "ASoC: rt5651: Enable sticky mode for OVCD" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5651: Enable sticky mode for OVCD

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 1b1ad83539a701a6bdcb25dc9160386e71f6016e Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:35:54 +0100
Subject: [PATCH] ASoC: rt5651: Enable sticky mode for OVCD

When the mic-gnd contacts are short-circuited by a headphones plug, the
hardware periodically retries if it can apply the bias-current leading to
the OVCD status flip-flopping 1-0-1 with it being 0 about 10% of the time.
This commit enables the sticky bit for the OVCD status to deal with this.

This commit also introduces 2 helper functions to deal with the OVCD
status bit, this may seem a bit overkill now, but these will also be
used in future patches.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5651.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 486817809b7b..62918f7e8270 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -1578,6 +1578,22 @@ static void rt5651_disable_micbias1_for_ovcd(struct snd_soc_component *component
 	snd_soc_dapm_mutex_unlock(dapm);
 }
 
+static void rt5651_clear_micbias1_ovcd(struct snd_soc_component *component)
+{
+	snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
+		RT5651_MB1_OC_CLR, 0);
+}
+
+static bool rt5651_micbias1_ovcd(struct snd_soc_component *component)
+{
+	int val;
+
+	val = snd_soc_component_read32(component, RT5651_IRQ_CTRL2);
+	dev_dbg(component->dev, "irq ctrl2 %#04x\n", val);
+
+	return (val & RT5651_MB1_OC_CLR);
+}
+
 static irqreturn_t rt5651_irq(int irq, void *data)
 {
 	struct rt5651_priv *rt5651 = data;
@@ -1646,6 +1662,18 @@ static int rt5651_set_jack(struct snd_soc_component *component,
 				      RT5651_PWR_MB_PU |
 				      RT5651_PWR_CLK12M_PU);
 
+	/*
+	 * The over-current-detect is only reliable in detecting the absence
+	 * of over-current, when the mic-contact in the jack is short-circuited,
+	 * the hardware periodically retries if it can apply the bias-current
+	 * leading to the ovcd status flip-flopping 1-0-1 with it being 0 about
+	 * 10% of the time, as we poll the ovcd status bit we might hit that
+	 * 10%, so we enable sticky mode and when checking OVCD we clear the
+	 * status, msleep() a bit and then check to get a reliable reading.
+	 */
+	snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
+		RT5651_MB1_OC_STKY_MASK, RT5651_MB1_OC_STKY_EN);
+
 	rt5651->hp_jack = hp_jack;
 
 	ret = devm_request_threaded_irq(component->dev, rt5651->irq, NULL,
@@ -1878,13 +1906,12 @@ static int rt5651_jack_detect(struct snd_soc_component *component, int jack_inse
 
 	if (jack_insert) {
 		rt5651_enable_micbias1_for_ovcd(component);
+		rt5651_clear_micbias1_ovcd(component);
 		msleep(100);
-		if (snd_soc_component_read32(component, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
+		if (rt5651_micbias1_ovcd(component))
 			jack_type = SND_JACK_HEADPHONE;
 		else
 			jack_type = SND_JACK_HEADSET;
-		snd_soc_component_update_bits(component, RT5651_IRQ_CTRL2,
-				    RT5651_MB1_OC_CLR, 0);
 		rt5651_disable_micbias1_for_ovcd(component);
 	} else { /* jack out */
 		jack_type = 0;
-- 
2.16.2

Applied "ASoC: Intel: bytcr_rt5651: Select RCCLK on init()" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Select RCCLK on init()

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From c22969d70fc9253112e88da55116e04074cdeac4 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:10 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Select RCCLK on init()

When the BYT_RT5651_MCLK_EN quirk is set, we disable the MCLK from
byt_rt5651_init(), we need to select the RCCLK as sysclk before doing this
to make sure that jack-detect works directly after boot.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 6efd7c9e7407..53ce313024f1 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -457,6 +457,11 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 
 	card->dapm.idle_bias_off = true;
 
+	/* Start with RC clk for jack-detect (we disable MCLK below) */
+	if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN)
+		snd_soc_component_update_bits(codec, RT5651_GLB_CLK,
+			RT5651_SCLK_SRC_MASK, RT5651_SCLK_SRC_RCCLK);
+
 	switch (BYT_RT5651_MAP(byt_rt5651_quirk)) {
 	case BYT_RT5651_IN1_MAP:
 		custom_map = byt_rt5651_intmic_in1_map;
-- 
2.16.2

Applied "ASoC: Intel: bytcr_rt5651: Change defaults to enable jack-detect, analog mics" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Change defaults to enable jack-detect, analog mics

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From b4b6377e07727df8292539062d9a56e6bd83ba89 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:09 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Change defaults to enable
 jack-detect, analog mics

Change the default quirk settings to enable jack-detect, analog mics.

The old default input mapping of DMIC for non Bay Trail CR devices seems
like a poor default as I'm not aware of any Intel SST + rt5651 using
devices with a DMIC.

All Cherry Trail devices using the bytcr_rt5651 machine driver seem to be
modelled after BYT-CR devices, And the only non CR Bay Trail devices with
a rt5651 codec I'm aware of are the Minnow boards for which we already have
board specific quirks. So it seems better to me to use the BYT-CR defaults
everywhere.

This e.g. makes the Chuwi Hi8 Pro (CWI513) work ootb without needing a
quirk.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index d5a3d48294d2..6efd7c9e7407 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -85,7 +85,12 @@ struct byt_rt5651_private {
 	struct snd_soc_jack jack;
 };
 
-static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN;
+/* Default: jack-detect on JD1_1, internal mic on in2, headsetmic on in3 */
+static unsigned long byt_rt5651_quirk = BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
+					BYT_RT5651_IN2_HS_IN3_MAP;
 
 static void log_quirks(struct device *dev)
 {
@@ -809,14 +814,6 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 			/* no BIOS indications, assume SSP0-AIF2 connection */
 			byt_rt5651_quirk |= BYT_RT5651_SSP0_AIF2;
 		}
-
-		/* change defaults for Baytrail-CR capture */
-		byt_rt5651_quirk |= BYT_RT5651_JD1_1 |
-				    BYT_RT5651_OVCD_TH_2000UA |
-				    BYT_RT5651_OVCD_SF_0P75 |
-				    BYT_RT5651_IN2_HS_IN3_MAP;
-	} else {
-		byt_rt5651_quirk |= BYT_RT5651_DMIC_MAP;
 	}
 
 	/* check quirks before creating card */
-- 
2.16.2

Applied "ASoC: Intel: bytcr_rt5651: Add quirk for the VIOS LTH17 laptop" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Add quirk for the VIOS LTH17 laptop

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From f9877eb598d5bd2e6d9b4dc28068b66c464d1e5f Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:08 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Add quirk for the VIOS LTH17
 laptop

Add a quirk setting up jack-detect and input routing for the
VIOS LTH17 laptop.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 103c4b0e6505..d5a3d48294d2 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -393,6 +393,19 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN2_HS_IN3_MAP),
 	},
+	{
+		/* VIOS LTH17 */
+		.callback = byt_rt5651_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "VIOS"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "LTH17"),
+		},
+		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_1P0 |
+					BYT_RT5651_IN1_IN2_MAP),
+	},
 	{}
 };
 
-- 
2.16.2

Applied "ASoC: Intel: bytcr_rt5651: Add new IN2_HS_IN3 input map and a quirk using it" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Add new IN2_HS_IN3 input map and a quirk using it

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From f026e06317804f305d4665d266191e07e2bf6394 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:06 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Add new IN2_HS_IN3 input map and a
 quirk using it

Add a new IN2_HS_IN3 input map and add a quirk for the input mapping and
jack-detect source for the Chuwi Vi8 Plus tablet, which uses this new map.

Note the Chuwi Vi8 Plus lists an extra GPIO in its codecs ACPI resources
which needs to be driven high to enable the external speaker amplifier,
this is not supported yet and will be fixed in a future patch.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 7d5764d23c51..b798ebb18bf1 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -42,6 +42,7 @@ enum {
 	BYT_RT5651_IN2_MAP,
 	BYT_RT5651_IN1_IN2_MAP,
 	BYT_RT5651_IN1_HS_IN3_MAP,
+	BYT_RT5651_IN2_HS_IN3_MAP,
 };
 
 enum {
@@ -93,6 +94,8 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN2_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN1_HS_IN3_MAP)
 		dev_info(dev, "quirk IN1_HS_IN3_MAP enabled");
+	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN2_HS_IN3_MAP)
+		dev_info(dev, "quirk IN2_HS_IN3_MAP enabled");
 	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
@@ -258,6 +261,12 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_hs_in3_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
+static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_hs_in3_map[] = {
+	{"Internal Mic", NULL, "micbias1"},
+	{"IN2P", NULL, "Internal Mic"},
+	{"IN3P", NULL, "Headset Mic"},
+};
+
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headphone"),
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
@@ -323,6 +332,19 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 					BYT_RT5651_OVCD_SF_0P75 |
 					BYT_RT5651_IN1_IN2_MAP),
 	},
+	{
+		/* Chuwi Vi8 Plus (CWI519) */
+		.callback = byt_rt5651_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Hampoo"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "D2D3_Vi8A1"),
+		},
+		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
+					BYT_RT5651_JD1_1 |
+					BYT_RT5651_OVCD_TH_2000UA |
+					BYT_RT5651_OVCD_SF_0P75 |
+					BYT_RT5651_IN2_HS_IN3_MAP),
+	},
 	{}
 };
 
@@ -386,6 +408,10 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 		custom_map = byt_rt5651_intmic_in1_hs_in3_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_hs_in3_map);
 		break;
+	case BYT_RT5651_IN2_HS_IN3_MAP:
+		custom_map = byt_rt5651_intmic_in2_hs_in3_map;
+		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in2_hs_in3_map);
+		break;
 	default:
 		custom_map = byt_rt5651_intmic_dmic_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_dmic_map);
-- 
2.16.2

Applied "ASoC: Intel: bytcr_rt5651: Rename IN3_MAP to IN1_HS_IN3_MAP" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Rename IN3_MAP to IN1_HS_IN3_MAP

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 7f2e2299cf169d8c517fbe2bcff3ca7bc8fa5f5c Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:05 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Rename IN3_MAP to IN1_HS_IN3_MAP

All the mappings are named for where the internal mic is routed and in that
sense the newly added in3_map really is the same as in1_map, what makes it
different is that it maps the headset mic at IN3 rather then at IN2.

Rename in3_map to in1_hs_in3_map to better reflect what it actually does.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 1c771930772d..7d5764d23c51 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -41,7 +41,7 @@ enum {
 	BYT_RT5651_IN1_MAP,
 	BYT_RT5651_IN2_MAP,
 	BYT_RT5651_IN1_IN2_MAP,
-	BYT_RT5651_IN3_MAP,
+	BYT_RT5651_IN1_HS_IN3_MAP,
 };
 
 enum {
@@ -91,8 +91,8 @@ static void log_quirks(struct device *dev)
 		dev_info(dev, "quirk IN1_MAP enabled");
 	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN2_MAP)
 		dev_info(dev, "quirk IN2_MAP enabled");
-	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN3_MAP)
-		dev_info(dev, "quirk IN3_MAP enabled");
+	if (BYT_RT5651_MAP(byt_rt5651_quirk) == BYT_RT5651_IN1_HS_IN3_MAP)
+		dev_info(dev, "quirk IN1_HS_IN3_MAP enabled");
 	if (BYT_RT5651_JDSRC(byt_rt5651_quirk)) {
 		dev_info(dev, "quirk realtek,jack-detect-source %ld\n",
 			 BYT_RT5651_JDSRC(byt_rt5651_quirk));
@@ -235,8 +235,8 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_dmic_map[] = {
 
 static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_map[] = {
 	{"Internal Mic", NULL, "micbias1"},
-	{"IN2P", NULL, "Headset Mic"},
 	{"IN1P", NULL, "Internal Mic"},
+	{"IN2P", NULL, "Headset Mic"},
 };
 
 static const struct snd_soc_dapm_route byt_rt5651_intmic_in2_map[] = {
@@ -252,10 +252,10 @@ static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_in2_map[] = {
 	{"IN3P", NULL, "Headset Mic"},
 };
 
-static const struct snd_soc_dapm_route byt_rt5651_intmic_in3_map[] = {
+static const struct snd_soc_dapm_route byt_rt5651_intmic_in1_hs_in3_map[] = {
 	{"Internal Mic", NULL, "micbias1"},
-	{"IN3P", NULL, "Headset Mic"},
 	{"IN1P", NULL, "Internal Mic"},
+	{"IN3P", NULL, "Headset Mic"},
 };
 
 static const struct snd_kcontrol_new byt_rt5651_controls[] = {
@@ -300,7 +300,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Circuitco"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Max B3 PLATFORM"),
 		},
-		.driver_data = (void *)(BYT_RT5651_IN3_MAP),
+		.driver_data = (void *)(BYT_RT5651_IN1_HS_IN3_MAP),
 	},
 	{
 		.callback = byt_rt5651_quirk_cb,
@@ -309,7 +309,7 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Turbot"),
 		},
 		.driver_data = (void *)(BYT_RT5651_MCLK_EN |
-					BYT_RT5651_IN3_MAP),
+					BYT_RT5651_IN1_HS_IN3_MAP),
 	},
 	{
 		.callback = byt_rt5651_quirk_cb,
@@ -382,9 +382,9 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
 		custom_map = byt_rt5651_intmic_in1_in2_map;
 		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_in2_map);
 		break;
-	case BYT_RT5651_IN3_MAP:
-		custom_map = byt_rt5651_intmic_in3_map;
-		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in3_map);
+	case BYT_RT5651_IN1_HS_IN3_MAP:
+		custom_map = byt_rt5651_intmic_in1_hs_in3_map;
+		num_routes = ARRAY_SIZE(byt_rt5651_intmic_in1_hs_in3_map);
 		break;
 	default:
 		custom_map = byt_rt5651_intmic_dmic_map;
-- 
2.16.2

Applied "ASoC: Intel: bytcr_rt5651: Drop snd_soc_dai_set_bclk_ratio() call" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Drop snd_soc_dai_set_bclk_ratio() call

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 3fdae070e6fcd9f9ca0745fd69d40085625a45f5 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:04 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Drop snd_soc_dai_set_bclk_ratio()
 call

Drop the snd_soc_dai_set_bclk_ratio() call, the rt5651 dai does not have a
set_bclk_ratio() op, so it is a nop (and returns -EINVAL).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index af30c730f432..1c771930772d 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -284,8 +284,6 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int rate = params_rate(params);
 
-	snd_soc_dai_set_bclk_ratio(codec_dai, 50);
-
 	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
 }
 
-- 
2.16.2

Applied "ASoC: Intel: bytcr_rt5651: Configure PLL1 before using it" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: bytcr_rt5651: Configure PLL1 before using it

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From aeec6cc0821573920d559f7d6297ea5dd3fbbd17 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 4 Mar 2018 15:36:03 +0100
Subject: [PATCH] ASoC: Intel: bytcr_rt5651: Configure PLL1 before using it

When platform_clock_control() first selects PLL1 as sysclk the PLL_CTRL
registers have not been setup yet and we effectively have an invalid clock
configuration until byt_rt5651_aif1_hw_params() gets called.

Add a new byt_rt5651_prepare_and_enable_pll1() helper and use that from
both platform_clock_control() and byt_rt5651_aif1_hw_params() to fix this.

Tested-by: Carlo Caione <carlo@endlessm.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 73 +++++++++++++++++------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 91b5841af622..af30c730f432 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -111,6 +111,38 @@ static void log_quirks(struct device *dev)
 
 #define BYT_CODEC_DAI1	"rt5651-aif1"
 
+static int byt_rt5651_prepare_and_enable_pll1(struct snd_soc_dai *codec_dai,
+					      int rate, int bclk_ratio)
+{
+	int clk_id, clk_freq, ret;
+
+	/* Configure the PLL before selecting it */
+	if (!(byt_rt5651_quirk & BYT_RT5651_MCLK_EN)) {
+		clk_id = RT5651_PLL1_S_BCLK1,
+		clk_freq = rate * bclk_ratio;
+	} else {
+		clk_id = RT5651_PLL1_S_MCLK;
+		if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ)
+			clk_freq = 25000000;
+		else
+			clk_freq = 19200000;
+	}
+	ret = snd_soc_dai_set_pll(codec_dai, 0, clk_id, clk_freq, rate * 512);
+	if (ret < 0) {
+		dev_err(codec_dai->codec->dev, "can't set pll: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
+				     rate * 512, SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		dev_err(codec_dai->codec->dev, "can't set clock %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int platform_clock_control(struct snd_soc_dapm_widget *w,
 				  struct snd_kcontrol *k, int  event)
 {
@@ -136,9 +168,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 				return ret;
 			}
 		}
-		ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
-					     48000 * 512,
-					     SND_SOC_CLOCK_IN);
+		ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);
 	} else {
 		/*
 		 * Set codec clock source to internal clock before
@@ -252,44 +282,11 @@ static int byt_rt5651_aif1_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
-	int ret;
+	int rate = params_rate(params);
 
 	snd_soc_dai_set_bclk_ratio(codec_dai, 50);
 
-	ret = snd_soc_dai_set_sysclk(codec_dai, RT5651_SCLK_S_PLL1,
-				     params_rate(params) * 512,
-				     SND_SOC_CLOCK_IN);
-	if (ret < 0) {
-		dev_err(rtd->dev, "can't set codec clock %d\n", ret);
-		return ret;
-	}
-
-	if (!(byt_rt5651_quirk & BYT_RT5651_MCLK_EN)) {
-		/* 2x25 bit slots on SSP2 */
-		ret = snd_soc_dai_set_pll(codec_dai, 0,
-					RT5651_PLL1_S_BCLK1,
-					params_rate(params) * 50,
-					params_rate(params) * 512);
-	} else {
-		if (byt_rt5651_quirk & BYT_RT5651_MCLK_25MHZ) {
-			ret = snd_soc_dai_set_pll(codec_dai, 0,
-						RT5651_PLL1_S_MCLK,
-						25000000,
-						params_rate(params) * 512);
-		} else {
-			ret = snd_soc_dai_set_pll(codec_dai, 0,
-						RT5651_PLL1_S_MCLK,
-						19200000,
-						params_rate(params) * 512);
-		}
-	}
-
-	if (ret < 0) {
-		dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
-		return ret;
-	}
-
-	return 0;
+	return byt_rt5651_prepare_and_enable_pll1(codec_dai, rate, 50);
 }
 
 static int byt_rt5651_quirk_cb(const struct dmi_system_id *id)
-- 
2.16.2