[PATCH v0 0/3] ASoC: max9867: driver update

[PATCH v0 0/3] ASoC: max9867: driver update

[Ladislav Michl]

Hi there,

after this patch serie, I'm finally able to use max9867 codec. This brings
in a question how was this driver tested and whenever these tests can be
repeated with this patchset applied.

Anyway, this serie is intentionally marked as v0 as I'm unclear about few
things described in comment section of respective patches. I'd be very happy
for answers as they are missing in documentation and are not obvious from
other drivers code.

Thank you in advance,
	ladis

[PATCH v0 1/3] ASoC: max9867: Take device out of shutdown

[Ladislav Michl]

Move device enable to probe function to make line bypass
and mic sidetone work. Doing that from prepare callback
lets only DAC to produce output.
While here move suspend and resume functions to the more
common place.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 sound/soc/codecs/max9867.c | 62 +++++++++++++++-----------------------
 sound/soc/codecs/max9867.h |  2 +-
 2 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/sound/soc/codecs/max9867.c b/sound/soc/codecs/max9867.c
index 4ea3287162ad..d7c96aa0d797 100644
--- a/sound/soc/codecs/max9867.c
+++ b/sound/soc/codecs/max9867.c
@@ -248,17 +248,6 @@ static int max9867_dai_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int max9867_prepare(struct snd_pcm_substream *substream,
-			 struct snd_soc_dai *dai)
-{
-	struct snd_soc_component *component = dai->component;
-	struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component);
-
-	regmap_update_bits(max9867->regmap, MAX9867_PWRMAN,
-		MAX9867_SHTDOWN_MASK, MAX9867_SHTDOWN_MASK);
-	return 0;
-}
-
 static int max9867_mute(struct snd_soc_dai *dai, int mute)
 {
 	struct snd_soc_component *component = dai->component;
@@ -361,7 +350,6 @@ static int max9867_dai_set_fmt(struct snd_soc_dai *codec_dai,
 static const struct snd_soc_dai_ops max9867_dai_ops = {
 	.set_fmt = max9867_dai_set_fmt,
 	.set_sysclk	= max9867_set_dai_sysclk,
-	.prepare	= max9867_prepare,
 	.digital_mute	= max9867_mute,
 	.hw_params = max9867_dai_hw_params,
 };
@@ -392,27 +380,6 @@ static struct snd_soc_dai_driver max9867_dai[] = {
 	}
 };
 
-#ifdef CONFIG_PM_SLEEP
-static int max9867_suspend(struct device *dev)
-{
-	struct max9867_priv *max9867 = dev_get_drvdata(dev);
-
-	/* Drop down to power saving mode when system is suspended */
-	regmap_update_bits(max9867->regmap, MAX9867_PWRMAN,
-		MAX9867_SHTDOWN_MASK, ~MAX9867_SHTDOWN_MASK);
-	return 0;
-}
-
-static int max9867_resume(struct device *dev)
-{
-	struct max9867_priv *max9867 = dev_get_drvdata(dev);
-
-	regmap_update_bits(max9867->regmap, MAX9867_PWRMAN,
-		MAX9867_SHTDOWN_MASK, MAX9867_SHTDOWN_MASK);
-	return 0;
-}
-#endif
-
 static const struct snd_soc_component_driver max9867_component = {
 	.controls		= max9867_snd_controls,
 	.num_controls		= ARRAY_SIZE(max9867_snd_controls),
@@ -493,19 +460,40 @@ static int max9867_i2c_probe(struct i2c_client *i2c,
 	ret = regmap_read(max9867->regmap,
 			MAX9867_REVISION, &reg);
 	if (ret < 0) {
-		dev_err(&i2c->dev, "Failed to read: %d\n", ret);
+		dev_err(&i2c->dev, "Failed to read revision: %d\n", ret);
 		return ret;
 	}
 	dev_info(&i2c->dev, "device revision: %x\n", reg);
-	ret = devm_snd_soc_register_component(&i2c->dev, &max9867_component,
-			max9867_dai, ARRAY_SIZE(max9867_dai));
+	ret = regmap_update_bits(max9867->regmap, MAX9867_PWRMAN,
+				 MAX9867_SHTDOWN, MAX9867_SHTDOWN);
 	if (ret < 0) {
-		dev_err(&i2c->dev, "Failed to register component: %d\n", ret);
+		dev_err(&i2c->dev, "Failed to enable: %d\n", ret);
 		return ret;
 	}
+	ret = devm_snd_soc_register_component(&i2c->dev, &max9867_component,
+			max9867_dai, ARRAY_SIZE(max9867_dai));
+	if (ret < 0)
+		dev_err(&i2c->dev, "Failed to register component: %d\n", ret);
 	return ret;
 }
 
+static int __maybe_unused max9867_suspend(struct device *dev)
+{
+	struct max9867_priv *max9867 = dev_get_drvdata(dev);
+
+	/* Drop down to power saving mode when system is suspended */
+	return regmap_update_bits(max9867->regmap, MAX9867_PWRMAN,
+				  MAX9867_SHTDOWN, 0);
+}
+
+static int __maybe_unused max9867_resume(struct device *dev)
+{
+	struct max9867_priv *max9867 = dev_get_drvdata(dev);
+
+	return regmap_update_bits(max9867->regmap, MAX9867_PWRMAN,
+				  MAX9867_SHTDOWN, MAX9867_SHTDOWN);
+}
+
 static const struct i2c_device_id max9867_i2c_id[] = {
 	{ "max9867", 0 },
 	{ }
diff --git a/sound/soc/codecs/max9867.h b/sound/soc/codecs/max9867.h
index 55cd9976ff47..d9170850c96e 100644
--- a/sound/soc/codecs/max9867.h
+++ b/sound/soc/codecs/max9867.h
@@ -67,7 +67,7 @@
 #define MAX9867_MICCONFIG    0x15
 #define MAX9867_MODECONFIG   0x16
 #define MAX9867_PWRMAN       0x17
-#define MAX9867_SHTDOWN_MASK (1<<7)
+#define MAX9867_SHTDOWN      0x80
 #define MAX9867_REVISION     0xff
 
 #define MAX9867_CACHEREGNUM 10
-- 
2.18.0

[PATCH v0 2/3] ASoC: max9867: Calculate LRCLK divider

[Ladislav Michl]

Drop "Common NI Values Table" and calculate LRCLK divider, then
add allowed rate constraints based on master clock frequency.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Should we just warn about unexact frequencies for bail out
 with error?

 sound/soc/codecs/max9867.c | 213 ++++++++++++++++++-------------------
 sound/soc/codecs/max9867.h |  31 ++----
 2 files changed, 113 insertions(+), 131 deletions(-)

diff --git a/sound/soc/codecs/max9867.c b/sound/soc/codecs/max9867.c
index d7c96aa0d797..ec0667cc4956 100644
--- a/sound/soc/codecs/max9867.c
+++ b/sound/soc/codecs/max9867.c
@@ -126,124 +126,106 @@ static const struct snd_soc_dapm_route max9867_audio_map[] = {
 	{"LINE_IN", NULL, "Right Line"},
 };
 
-enum rates {
-	pcm_rate_8, pcm_rate_16, pcm_rate_24,
-	pcm_rate_32, pcm_rate_44,
-	pcm_rate_48, max_pcm_rate,
+static const unsigned int max9867_rates_44k1[] = {
+	11025, 22050, 44100,
 };
 
-static const struct ni_div_rates {
-	u32 mclk;
-	u16 ni[max_pcm_rate];
-} ni_div[] = {
-	{11289600, {0x116A, 0x22D4, 0x343F, 0x45A9, 0x6000, 0x687D} },
-	{12000000, {0x1062, 0x20C5, 0x3127, 0x4189, 0x5A51, 0x624E} },
-	{12288000, {0x1000, 0x2000, 0x3000, 0x4000, 0x5833, 0x6000} },
-	{13000000, {0x0F20, 0x1E3F, 0x2D5F, 0x3C7F, 0x535F, 0x5ABE} },
-	{19200000, {0x0A3D, 0x147B, 0x1EB8, 0x28F6, 0x3873, 0x3D71} },
-	{24000000, {0x1062, 0x20C5, 0x1893, 0x4189, 0x5A51, 0x624E} },
-	{26000000, {0x0F20, 0x1E3F, 0x16AF, 0x3C7F, 0x535F, 0x5ABE} },
-	{27000000, {0x0E90, 0x1D21, 0x15D8, 0x3A41, 0x5048, 0x5762} },
+static const struct snd_pcm_hw_constraint_list max9867_constraints_44k1 = {
+	.list = max9867_rates_44k1,
+	.count = ARRAY_SIZE(max9867_rates_44k1),
 };
 
-static inline int get_ni_value(int mclk, int rate)
+static const unsigned int max9867_rates_48k[] = {
+	8000, 16000, 32000, 48000,
+};
+
+static const struct snd_pcm_hw_constraint_list max9867_constraints_48k = {
+	.list = max9867_rates_48k,
+	.count = ARRAY_SIZE(max9867_rates_48k),
+};
+
+struct max9867_priv {
+	struct regmap *regmap;
+	const struct snd_pcm_hw_constraint_list *constraints;
+	unsigned int sysclk, pclk;
+	bool master, dsp_a;
+};
+
+static int max9867_startup(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
 {
-	int i, ret = 0;
+        struct max9867_priv *max9867 =
+		snd_soc_component_get_drvdata(dai->component);
 
-	/* find the closest rate index*/
-	for (i = 0; i < ARRAY_SIZE(ni_div); i++) {
-		if (ni_div[i].mclk >= mclk)
-			break;
-	}
-	if (i == ARRAY_SIZE(ni_div))
-		return -EINVAL;
+	if (max9867->constraints)
+		snd_pcm_hw_constraint_list(substream->runtime, 0,
+			SNDRV_PCM_HW_PARAM_RATE, max9867->constraints);
 
-	switch (rate) {
-	case 8000:
-		return ni_div[i].ni[pcm_rate_8];
-	case 16000:
-		return ni_div[i].ni[pcm_rate_16];
-	case 32000:
-		return ni_div[i].ni[pcm_rate_32];
-	case 44100:
-		return ni_div[i].ni[pcm_rate_44];
-	case 48000:
-		return ni_div[i].ni[pcm_rate_48];
-	default:
-		pr_err("%s wrong rate %d\n", __func__, rate);
-		ret = -EINVAL;
-	}
-	return ret;
+	return 0;
 }
 
 static int max9867_dai_hw_params(struct snd_pcm_substream *substream,
 		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
 {
+	int value;
+	unsigned long int rate, ratio;
 	struct snd_soc_component *component = dai->component;
 	struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component);
-	unsigned int ni_h, ni_l;
-	int value;
+	unsigned int ni = DIV_ROUND_CLOSEST_ULL(96ULL * 0x10000 * params_rate(params),
+						max9867->pclk);
 
-	value = get_ni_value(max9867->sysclk, params_rate(params));
-	if (value < 0)
-		return value;
-
-	ni_h = (0xFF00 & value) >> 8;
-	ni_l = 0x00FF & value;
 	/* set up the ni value */
 	regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKHIGH,
-		MAX9867_NI_HIGH_MASK, ni_h);
+		MAX9867_NI_HIGH_MASK, (0xFF00 & ni) >> 8);
 	regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKLOW,
-		MAX9867_NI_LOW_MASK, ni_l);
-	if (!max9867->master) {
-		/*
-		 * digital pll locks on to any externally supplied LRCLK signal
-		 * and also enable rapid lock mode.
-		 */
-		regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKLOW,
-			MAX9867_RAPID_LOCK, MAX9867_RAPID_LOCK);
-		regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKHIGH,
-			MAX9867_PLL, MAX9867_PLL);
-	} else {
-		unsigned long int bclk_rate, pclk_bclk_ratio;
-		int bclk_value;
-
-		bclk_rate = params_rate(params) * 2 * params_width(params);
-		pclk_bclk_ratio = max9867->pclk/bclk_rate;
-		switch (params_width(params)) {
-		case 8:
-		case 16:
-			switch (pclk_bclk_ratio) {
-			case 2:
-				bclk_value = MAX9867_IFC1B_PCLK_2;
-				break;
-			case 4:
-				bclk_value = MAX9867_IFC1B_PCLK_4;
-				break;
+		MAX9867_NI_LOW_MASK, 0x00FF & ni);
+	if (max9867->master) {
+		if (max9867->dsp_a) {
+			value = MAX9867_IFC1B_48X;
+		} else {
+			rate = params_rate(params) * 2 * params_width(params);
+			ratio = max9867->pclk / rate;
+			switch (params_width(params)) {
 			case 8:
-				bclk_value = MAX9867_IFC1B_PCLK_8;
-				break;
 			case 16:
-				bclk_value = MAX9867_IFC1B_PCLK_16;
+				switch (ratio) {
+				case 2:
+					value = MAX9867_IFC1B_PCLK_2;
+					break;
+				case 4:
+					value = MAX9867_IFC1B_PCLK_4;
+					break;
+				case 8:
+					value = MAX9867_IFC1B_PCLK_8;
+					break;
+				case 16:
+					value = MAX9867_IFC1B_PCLK_16;
+					break;
+				default:
+					return -EINVAL;
+				}
+				break;
+			case 24:
+				value = MAX9867_IFC1B_48X;
+				break;
+			case 32:
+				value = MAX9867_IFC1B_64X;
 				break;
 			default:
-				dev_err(component->dev,
-					"unsupported sampling rate\n");
 				return -EINVAL;
 			}
-			break;
-		case 24:
-			bclk_value = MAX9867_IFC1B_24BIT;
-			break;
-		case 32:
-			bclk_value = MAX9867_IFC1B_32BIT;
-			break;
-		default:
-			dev_err(component->dev, "unsupported sampling rate\n");
-			return -EINVAL;
 		}
 		regmap_update_bits(max9867->regmap, MAX9867_IFC1B,
-			MAX9867_IFC1B_BCLK_MASK, bclk_value);
+			MAX9867_IFC1B_BCLK_MASK, value);
+	} else {
+		/*
+		 * digital pll locks on to any externally supplied LRCLK signal
+		 * and also enable rapid lock mode.
+		 */
+		regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKLOW,
+			MAX9867_RAPID_LOCK, MAX9867_RAPID_LOCK);
+		regmap_update_bits(max9867->regmap, MAX9867_AUDIOCLKHIGH,
+			MAX9867_PLL, MAX9867_PLL);
 	}
 	return 0;
 }
@@ -272,21 +254,29 @@ static int max9867_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 	/* Set the prescaler based on the master clock frequency*/
 	if (freq >= 10000000 && freq <= 20000000) {
 		value |= MAX9867_PSCLK_10_20;
-		max9867->pclk =  freq;
+		max9867->pclk = freq;
 	} else if (freq >= 20000000 && freq <= 40000000) {
 		value |= MAX9867_PSCLK_20_40;
-		max9867->pclk =  freq/2;
+		max9867->pclk = freq / 2;
 	} else if (freq >= 40000000 && freq <= 60000000) {
 		value |= MAX9867_PSCLK_40_60;
-		max9867->pclk =  freq/4;
+		max9867->pclk = freq / 4;
 	} else {
 		dev_err(component->dev,
 			"Invalid clock frequency %uHz (required 10-60MHz)\n",
 			freq);
 		return -EINVAL;
 	}
-	value = value << MAX9867_PSCLK_SHIFT;
+	if (freq % 48000 == 0)
+		max9867->constraints = &max9867_constraints_48k;
+	else if (freq % 44100 == 0)
+		max9867->constraints = &max9867_constraints_44k1;
+	else
+		dev_warn(component->dev,
+			 "Unable to set exact rate with %uHz clock frequency\n",
+			 freq);
 	max9867->sysclk = freq;
+	value = value << MAX9867_PSCLK_SHIFT;
 	/* exact integer mode is not supported */
 	value &= ~MAX9867_FREQ_MASK;
 	regmap_update_bits(max9867->regmap, MAX9867_SYSCLK,
@@ -299,16 +289,17 @@ static int max9867_dai_set_fmt(struct snd_soc_dai *codec_dai,
 {
 	struct snd_soc_component *component = codec_dai->component;
 	struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component);
-	u8 iface1A = 0, iface1B = 0;
+	u8 iface1A, iface1B;
 
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBM_CFM:
-		max9867->master = 1;
-		iface1A |= MAX9867_MASTER;
+		max9867->master = true;
+		iface1A = MAX9867_MASTER;
+		iface1B = MAX9867_IFC1B_48X;
 		break;
 	case SND_SOC_DAIFMT_CBS_CFS:
-		max9867->master = 0;
-		iface1A &= ~MAX9867_MASTER;
+		max9867->master = false;
+		iface1A = iface1B = 0;
 		break;
 	default:
 		return -EINVAL;
@@ -316,9 +307,11 @@ static int max9867_dai_set_fmt(struct snd_soc_dai *codec_dai,
 
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+		max9867->dsp_a = false;
 		iface1A |= MAX9867_I2S_DLY;
 		break;
 	case SND_SOC_DAIFMT_DSP_A:
+		max9867->dsp_a = true;
 		iface1A |= MAX9867_TDM_MODE | MAX9867_SDOUT_HIZ;
 		break;
 	default:
@@ -344,20 +337,18 @@ static int max9867_dai_set_fmt(struct snd_soc_dai *codec_dai,
 
 	regmap_write(max9867->regmap, MAX9867_IFC1A, iface1A);
 	regmap_write(max9867->regmap, MAX9867_IFC1B, iface1B);
+
 	return 0;
 }
 
 static const struct snd_soc_dai_ops max9867_dai_ops = {
-	.set_fmt = max9867_dai_set_fmt,
 	.set_sysclk	= max9867_set_dai_sysclk,
+	.set_fmt	= max9867_dai_set_fmt,
 	.digital_mute	= max9867_mute,
-	.hw_params = max9867_dai_hw_params,
+	.startup	= max9867_startup,
+	.hw_params	= max9867_dai_hw_params,
 };
 
-#define MAX9867_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |\
-	SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000)
-#define MAX9867_FORMATS (SNDRV_PCM_FMTBIT_S16_LE)
-
 static struct snd_soc_dai_driver max9867_dai[] = {
 	{
 	.name = "max9867-aif1",
@@ -365,15 +356,15 @@ static struct snd_soc_dai_driver max9867_dai[] = {
 		.stream_name = "HiFi Playback",
 		.channels_min = 2,
 		.channels_max = 2,
-		.rates = MAX9867_RATES,
-		.formats = MAX9867_FORMATS,
+		.rates = SNDRV_PCM_RATE_8000_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
 	},
 	.capture = {
 		.stream_name = "HiFi Capture",
 		.channels_min = 2,
 		.channels_max = 2,
-		.rates = MAX9867_RATES,
-		.formats = MAX9867_FORMATS,
+		.rates = SNDRV_PCM_RATE_8000_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
 	},
 	.ops = &max9867_dai_ops,
 	.symmetric_rates = 1,
diff --git a/sound/soc/codecs/max9867.h b/sound/soc/codecs/max9867.h
index d9170850c96e..7f037ab701a5 100644
--- a/sound/soc/codecs/max9867.h
+++ b/sound/soc/codecs/max9867.h
@@ -26,13 +26,11 @@
 #define MAX9867_PSCLK_10_20  0x1
 #define MAX9867_PSCLK_20_40  0x2
 #define MAX9867_PSCLK_40_60  0x3
-#define MAX9867_AUDIOCLKHIGH 0x06
-#define MAX9867_NI_HIGH_WIDTH 0x7
-#define MAX9867_NI_HIGH_MASK 0x7F
-#define MAX9867_NI_LOW_MASK 0x7F
-#define MAX9867_NI_LOW_SHIFT 0x1
-#define MAX9867_PLL     (1<<7)
-#define MAX9867_AUDIOCLKLOW  0x07
+#define MAX9867_AUDIOCLKHIGH	0x06
+#define MAX9867_NI_HIGH_MASK	0x7F
+#define MAX9867_NI_LOW_MASK	0xFE
+#define MAX9867_PLL		(1<<7)
+#define MAX9867_AUDIOCLKLOW	0x07
 #define MAX9867_RAPID_LOCK   0x01
 #define MAX9867_IFC1A        0x08
 #define MAX9867_MASTER       (1<<7)
@@ -43,12 +41,12 @@
 #define MAX9867_BCI_MODE     (1<<5)
 #define MAX9867_IFC1B        0x09
 #define MAX9867_IFC1B_BCLK_MASK 7
-#define MAX9867_IFC1B_32BIT  0x01
-#define MAX9867_IFC1B_24BIT  0x02
-#define MAX9867_IFC1B_PCLK_2 4
-#define MAX9867_IFC1B_PCLK_4 5
-#define MAX9867_IFC1B_PCLK_8 6
-#define MAX9867_IFC1B_PCLK_16 7
+#define MAX9867_IFC1B_64X	0x01
+#define MAX9867_IFC1B_48X	0x02
+#define MAX9867_IFC1B_PCLK_2	0x04
+#define MAX9867_IFC1B_PCLK_4	0x05
+#define MAX9867_IFC1B_PCLK_8	0x06
+#define MAX9867_IFC1B_PCLK_16	0x07
 #define MAX9867_CODECFLTR    0x0a
 #define MAX9867_DACGAIN      0x0b
 #define MAX9867_DACLEVEL     0x0c
@@ -72,11 +70,4 @@
 
 #define MAX9867_CACHEREGNUM 10
 
-/* codec private data */
-struct max9867_priv {
-	struct regmap *regmap;
-	unsigned int sysclk;
-	unsigned int pclk;
-	unsigned int master;
-};
 #endif
-- 
2.18.0

[PATCH v0 3/3] ASoC: max9867: Fix volume controls

[Ladislav Michl]

Build proper audio paths and fix volume controls to match datasheet.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 I'm still unsure about control names, please see yet unaswered mail
 here: https://www.spinics.net/lists/alsa-devel/msg80006.html

 sound/soc/codecs/max9867.c | 180 +++++++++++++++++++------------------
 sound/soc/codecs/max9867.h |   8 +-
 2 files changed, 97 insertions(+), 91 deletions(-)

diff --git a/sound/soc/codecs/max9867.c b/sound/soc/codecs/max9867.c
index ec0667cc4956..87b0a04179ed 100644
--- a/sound/soc/codecs/max9867.c
+++ b/sound/soc/codecs/max9867.c
@@ -23,107 +23,122 @@ static const char *const max9867_spmode[] = {
 	"Stereo Single", "Mono Single",
 	"Stereo Single Fast", "Mono Single Fast"
 };
-static const char *const max9867_sidetone_text[] = {
-	"None", "Left", "Right", "LeftRight", "LeftRightDiv2",
-};
 static const char *const max9867_filter_text[] = {"IIR", "FIR"};
 
 static SOC_ENUM_SINGLE_DECL(max9867_filter, MAX9867_CODECFLTR, 7,
 	max9867_filter_text);
 static SOC_ENUM_SINGLE_DECL(max9867_spkmode, MAX9867_MODECONFIG, 0,
 	max9867_spmode);
-static SOC_ENUM_SINGLE_DECL(max9867_sidetone, MAX9867_DACGAIN, 6,
-	max9867_sidetone_text);
-static DECLARE_TLV_DB_SCALE(max9860_capture_tlv, -600, 200, 0);
-static DECLARE_TLV_DB_SCALE(max9860_mic_tlv, 2000, 100, 1);
-static DECLARE_TLV_DB_SCALE(max9860_adc_left_tlv, -1200, 100, 1);
-static DECLARE_TLV_DB_SCALE(max9860_adc_right_tlv, -1200, 100, 1);
-static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(max98088_micboost_tlv,
-	0, 1, TLV_DB_SCALE_ITEM(0, 2000, 0),
-	2, 2, TLV_DB_SCALE_ITEM(3000, 0, 0),
+static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(max9867_master_tlv,
+	 0,  2, TLV_DB_SCALE_ITEM(-8600, 200, 1),
+	 3, 17, TLV_DB_SCALE_ITEM(-7800, 400, 0),
+	18, 25, TLV_DB_SCALE_ITEM(-2000, 200, 0),
+	26, 34, TLV_DB_SCALE_ITEM( -500, 100, 0),
+	35, 40, TLV_DB_SCALE_ITEM(  350,  50, 0),
+);
+static DECLARE_TLV_DB_SCALE(max9867_mic_tlv, 0, 100, 0);
+static DECLARE_TLV_DB_SCALE(max9867_line_tlv, -600, 200, 0);
+static DECLARE_TLV_DB_SCALE(max9867_adc_tlv, -1200, 100, 0);
+static DECLARE_TLV_DB_SCALE(max9867_dac_tlv, -1500, 100, 0);
+static DECLARE_TLV_DB_SCALE(max9867_dacboost_tlv, 0, 600, 0);
+static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(max9867_micboost_tlv,
+	0, 2, TLV_DB_SCALE_ITEM(-2000, 2000, 1),
+	3, 3, TLV_DB_SCALE_ITEM(3000, 0, 0),
 );
 
 static const struct snd_kcontrol_new max9867_snd_controls[] = {
-	SOC_DOUBLE_R("Master Playback Volume", MAX9867_LEFTVOL,
-				MAX9867_RIGHTVOL, 0, 63, 1),
-	SOC_DOUBLE_R_TLV("Capture Volume", MAX9867_LEFTMICGAIN,
-			MAX9867_RIGHTMICGAIN,
-			0, 15, 1, max9860_capture_tlv),
+	SOC_DOUBLE_R_TLV("Master Playback Volume", MAX9867_LEFTVOL,
+			MAX9867_RIGHTVOL, 0, 41, 1, max9867_master_tlv),
+	SOC_DOUBLE_R_TLV("Line Volume", MAX9867_LEFTLINELVL,
+			MAX9867_RIGHTLINELVL, 0, 15, 1, max9867_line_tlv),
 	SOC_DOUBLE_R_TLV("Mic Volume", MAX9867_LEFTMICGAIN,
-			MAX9867_RIGHTMICGAIN, 0, 31, 1, max9860_mic_tlv),
-	SOC_DOUBLE_R_TLV("Mic Boost Volume", MAX9867_LEFTMICGAIN,
-			MAX9867_RIGHTMICGAIN, 5, 3, 0, max98088_micboost_tlv),
-	SOC_ENUM("Digital Sidetone Src", max9867_sidetone),
-	SOC_SINGLE("Sidetone Volume", MAX9867_DACGAIN, 0, 31, 1),
-	SOC_SINGLE("DAC Volume", MAX9867_DACLEVEL, 4, 3, 0),
-	SOC_SINGLE("DAC Attenuation", MAX9867_DACLEVEL, 0, 15, 1),
-	SOC_SINGLE_TLV("ADC Left Volume", MAX9867_ADCLEVEL,
-			4, 15, 1, max9860_adc_left_tlv),
-	SOC_SINGLE_TLV("ADC Right Volume", MAX9867_ADCLEVEL,
-			0, 15, 1, max9860_adc_right_tlv),
+			MAX9867_RIGHTMICGAIN, 0, 20, 1, max9867_mic_tlv),
+	SOC_DOUBLE_R_TLV("Mic Boost Capture Volume", MAX9867_LEFTMICGAIN,
+			MAX9867_RIGHTMICGAIN, 5, 4, 0, max9867_micboost_tlv),
+	SOC_SINGLE("Digital Sidetone Volume", MAX9867_SIDETONE, 0, 31, 1),
+	SOC_SINGLE_TLV("Digital Playback Volume", MAX9867_DACLEVEL, 0, 15, 1,
+			max9867_dac_tlv),
+	SOC_SINGLE_TLV("Digital Boost Playback Volume", MAX9867_DACLEVEL, 4, 3, 0,
+			max9867_dacboost_tlv),
+	SOC_DOUBLE_TLV("Digital Capture Volume", MAX9867_ADCLEVEL, 0, 4, 15, 1,
+			max9867_adc_tlv),
 	SOC_ENUM("Speaker Mode", max9867_spkmode),
 	SOC_SINGLE("Volume Smoothing Switch", MAX9867_MODECONFIG, 6, 1, 0),
-	SOC_SINGLE("ZCD Switch", MAX9867_MODECONFIG, 5, 1, 0),
+	SOC_SINGLE("Line ZC Switch", MAX9867_MODECONFIG, 5, 1, 0),
 	SOC_ENUM("DSP Filter", max9867_filter),
 };
 
-static const char *const max9867_mux[] = {"None", "Mic", "Line", "Mic_Line"};
+/* Input mixer */
+static const struct snd_kcontrol_new max9867_input_mixer_controls[] = {
+	SOC_DAPM_DOUBLE("Mic Switch", MAX9867_INPUTCONFIG, 6, 4, 1, 0),
+	SOC_DAPM_DOUBLE("Line Switch", MAX9867_INPUTCONFIG, 7, 5, 1, 0),
+};
+
+/* Output mixer */
+static const struct snd_kcontrol_new max9867_output_mixer_controls[] = {
+	SOC_DAPM_DOUBLE_R("Line Bypass Switch",
+			  MAX9867_LEFTLINELVL, MAX9867_RIGHTLINELVL, 6, 1, 1),
+};
 
-static SOC_ENUM_SINGLE_DECL(max9867_mux_enum,
-	MAX9867_INPUTCONFIG, MAX9867_INPUT_SHIFT,
-	max9867_mux);
+/* Sidetone mixer */
+static const struct snd_kcontrol_new max9867_sidetone_mixer_controls[] = {
+	SOC_DAPM_DOUBLE("Sidetone Switch", MAX9867_SIDETONE, 6, 7, 1, 0),
+};
 
-static const struct snd_kcontrol_new max9867_dapm_mux_controls =
-	SOC_DAPM_ENUM("Route", max9867_mux_enum);
+/* Line out switch */
+static const struct snd_kcontrol_new max9867_line_out_control =
+	SOC_DAPM_DOUBLE_R("Switch",
+			  MAX9867_LEFTVOL, MAX9867_RIGHTVOL, 6, 1, 1);
 
-static const struct snd_kcontrol_new max9867_left_dapm_control =
-	SOC_DAPM_SINGLE("Switch", MAX9867_PWRMAN, 6, 1, 0);
-static const struct snd_kcontrol_new max9867_right_dapm_control =
-	SOC_DAPM_SINGLE("Switch", MAX9867_PWRMAN, 5, 1, 0);
-static const struct snd_kcontrol_new max9867_line_dapm_control =
-	SOC_DAPM_SINGLE("Switch", MAX9867_LEFTLINELVL, 6, 1, 1);
 
 static const struct snd_soc_dapm_widget max9867_dapm_widgets[] = {
-	SND_SOC_DAPM_AIF_IN("DAI_OUT", "HiFi Playback", 0, SND_SOC_NOPM, 0, 0),
-	SND_SOC_DAPM_DAC("Left DAC", NULL, MAX9867_PWRMAN, 3, 0),
-	SND_SOC_DAPM_DAC("Right DAC", NULL, MAX9867_PWRMAN, 2, 0),
-	SND_SOC_DAPM_MIXER("Output Mixer", SND_SOC_NOPM, 0, 0, NULL, 0),
-	SND_SOC_DAPM_OUTPUT("HPOUT"),
-
-	SND_SOC_DAPM_AIF_IN("DAI_IN", "HiFi Capture", 0, SND_SOC_NOPM, 0, 0),
-	SND_SOC_DAPM_ADC("Left ADC", "HiFi Capture", MAX9867_PWRMAN, 1, 0),
-	SND_SOC_DAPM_ADC("Right ADC", "HiFi Capture", MAX9867_PWRMAN, 0, 0),
-	SND_SOC_DAPM_MUX("Input Mux", SND_SOC_NOPM, 0, 0,
-		&max9867_dapm_mux_controls),
-
-	SND_SOC_DAPM_MIXER("Input Mixer", SND_SOC_NOPM, 0, 0, NULL, 0),
-	SND_SOC_DAPM_SWITCH("Left Line", MAX9867_LEFTLINELVL, 6, 1,
-		&max9867_left_dapm_control),
-	SND_SOC_DAPM_SWITCH("Right Line", MAX9867_RIGTHLINELVL, 6, 1,
-		&max9867_right_dapm_control),
-	SND_SOC_DAPM_SWITCH("Line Mixer", SND_SOC_NOPM, 0, 0,
-		&max9867_line_dapm_control),
-	SND_SOC_DAPM_INPUT("LINE_IN"),
+	SND_SOC_DAPM_INPUT("MICL"),
+	SND_SOC_DAPM_INPUT("MICR"),
+	SND_SOC_DAPM_INPUT("LINL"),
+	SND_SOC_DAPM_INPUT("LINR"),
+	SND_SOC_DAPM_MIXER("Left Input Mixer", MAX9867_PWRMAN, 6, 0,
+			   max9867_input_mixer_controls,
+			   ARRAY_SIZE(max9867_input_mixer_controls)),
+	SND_SOC_DAPM_MIXER("Right Input Mixer", MAX9867_PWRMAN, 5, 0,
+			   max9867_input_mixer_controls,
+			   ARRAY_SIZE(max9867_input_mixer_controls)),
+	SND_SOC_DAPM_ADC("ADCL", "HiFi Capture", MAX9867_PWRMAN, 1, 0),
+	SND_SOC_DAPM_ADC("ADCR", "HiFi Capture", MAX9867_PWRMAN, 0, 0),
+
+	SND_SOC_DAPM_MIXER("Digital", SND_SOC_NOPM, 0, 0,
+			   max9867_sidetone_mixer_controls,
+			   ARRAY_SIZE(max9867_sidetone_mixer_controls)),
+	SND_SOC_DAPM_MIXER("Output Mixer", SND_SOC_NOPM, 0, 0,
+			   max9867_output_mixer_controls,
+			   ARRAY_SIZE(max9867_output_mixer_controls)),
+	SND_SOC_DAPM_DAC("DACL", "HiFi Playback", MAX9867_PWRMAN, 3, 0),
+	SND_SOC_DAPM_DAC("DACR", "HiFi Playback", MAX9867_PWRMAN, 2, 0),
+	SND_SOC_DAPM_SWITCH("Master Playback", SND_SOC_NOPM, 0, 0,
+			    &max9867_line_out_control),
+	SND_SOC_DAPM_OUTPUT("LOUT"),
+	SND_SOC_DAPM_OUTPUT("ROUT"),
 };
 
 static const struct snd_soc_dapm_route max9867_audio_map[] = {
-	{"Left DAC", NULL, "DAI_OUT"},
-	{"Right DAC", NULL, "DAI_OUT"},
-	{"Output Mixer", NULL, "Left DAC"},
-	{"Output Mixer", NULL, "Right DAC"},
-	{"HPOUT", NULL, "Output Mixer"},
-
-	{"Left ADC", NULL, "DAI_IN"},
-	{"Right ADC", NULL, "DAI_IN"},
-	{"Input Mixer", NULL, "Left ADC"},
-	{"Input Mixer", NULL, "Right ADC"},
-	{"Input Mux", "Line", "Input Mixer"},
-	{"Input Mux", "Mic", "Input Mixer"},
-	{"Input Mux", "Mic_Line", "Input Mixer"},
-	{"Right Line", "Switch", "Input Mux"},
-	{"Left Line", "Switch", "Input Mux"},
-	{"LINE_IN", NULL, "Left Line"},
-	{"LINE_IN", NULL, "Right Line"},
+	{"Left Input Mixer", "Mic Switch", "MICL"},
+	{"Right Input Mixer", "Mic Switch", "MICR"},
+	{"Left Input Mixer", "Line Switch", "LINL"},
+	{"Right Input Mixer", "Line Switch", "LINR"},
+	{"ADCL", NULL, "Left Input Mixer"},
+	{"ADCR", NULL, "Right Input Mixer"},
+
+	{"Digital", "Sidetone Switch", "ADCL"},
+	{"Digital", "Sidetone Switch", "ADCR"},
+	{"DACL", NULL, "Digital"},
+	{"DACR", NULL, "Digital"},
+
+	{"Output Mixer", "Line Bypass Switch", "LINL"},
+	{"Output Mixer", "Line Bypass Switch", "LINR"},
+	{"Output Mixer", NULL, "DACL"},
+	{"Output Mixer", NULL, "DACR"},
+	{"Master Playback", "Switch", "Output Mixer"},
+	{"LOUT", NULL, "Master Playback"},
+	{"ROUT", NULL, "Master Playback"},
 };
 
 static const unsigned int max9867_rates_44k1[] = {
@@ -235,13 +250,8 @@ static int max9867_mute(struct snd_soc_dai *dai, int mute)
 	struct snd_soc_component *component = dai->component;
 	struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component);
 
-	if (mute)
-		regmap_update_bits(max9867->regmap, MAX9867_DACLEVEL,
-			MAX9867_DAC_MUTE_MASK, MAX9867_DAC_MUTE_MASK);
-	else
-		regmap_update_bits(max9867->regmap, MAX9867_DACLEVEL,
-			MAX9867_DAC_MUTE_MASK, 0);
-	return 0;
+	return regmap_update_bits(max9867->regmap, MAX9867_DACLEVEL,
+				  1 << 6, !!mute << 6);
 }
 
 static int max9867_set_dai_sysclk(struct snd_soc_dai *codec_dai,
diff --git a/sound/soc/codecs/max9867.h b/sound/soc/codecs/max9867.h
index 7f037ab701a5..2277798291a1 100644
--- a/sound/soc/codecs/max9867.h
+++ b/sound/soc/codecs/max9867.h
@@ -48,20 +48,16 @@
 #define MAX9867_IFC1B_PCLK_8	0x06
 #define MAX9867_IFC1B_PCLK_16	0x07
 #define MAX9867_CODECFLTR    0x0a
-#define MAX9867_DACGAIN      0x0b
+#define MAX9867_SIDETONE     0x0b
 #define MAX9867_DACLEVEL     0x0c
-#define MAX9867_DAC_MUTE_SHIFT 0x6
-#define MAX9867_DAC_MUTE_WIDTH 0x1
-#define MAX9867_DAC_MUTE_MASK (0x1<<MAX9867_DAC_MUTE_SHIFT)
 #define MAX9867_ADCLEVEL     0x0d
 #define MAX9867_LEFTLINELVL  0x0e
-#define MAX9867_RIGTHLINELVL 0x0f
+#define MAX9867_RIGHTLINELVL 0x0f
 #define MAX9867_LEFTVOL      0x10
 #define MAX9867_RIGHTVOL     0x11
 #define MAX9867_LEFTMICGAIN  0x12
 #define MAX9867_RIGHTMICGAIN 0x13
 #define MAX9867_INPUTCONFIG  0x14
-#define MAX9867_INPUT_SHIFT  0x6
 #define MAX9867_MICCONFIG    0x15
 #define MAX9867_MODECONFIG   0x16
 #define MAX9867_PWRMAN       0x17
-- 
2.18.0

Re: [PATCH v0 1/3] ASoC: max9867: Take device out of shutdown

[Mark Brown]

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

On Sat, Jul 28, 2018 at 05:42:26PM +0200, Ladislav Michl wrote:
> Move device enable to probe function to make line bypass
> and mic sidetone work. Doing that from prepare callback
> lets only DAC to produce output.
> While here move suspend and resume functions to the more
> common place.

It'd be better to move this to DAPM so that the device is enabled only
when one of the paths in use - do it from set_bias_level().

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

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

Re: [PATCH v0 3/3] ASoC: max9867: Fix volume controls

[Mark Brown]

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

On Sat, Jul 28, 2018 at 05:44:15PM +0200, Ladislav Michl wrote:
> Build proper audio paths and fix volume controls to match datasheet.

This sounds like it should be split up into separate patches for at
least the routing and volume control stuff.  It also needs a bit of a
better changelog as I'm not sure what the issues with the volume
controls were or what was improper about the old audio paths.

> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  I'm still unsure about control names, please see yet unaswered mail
>  here: https://www.spinics.net/lists/alsa-devel/msg80006.html

Always CC maintainers on mails - you really shouldn't rely on anyone
seeing anything that only goes to the lists, they're quite high volume.

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

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

Re: [PATCH v0 3/3] ASoC: max9867: Fix volume controls

[Ladislav Michl]

On Mon, Jul 30, 2018 at 11:11:15AM +0100, Mark Brown wrote:
> On Sat, Jul 28, 2018 at 05:44:15PM +0200, Ladislav Michl wrote:
> > Build proper audio paths and fix volume controls to match datasheet.
> 
> This sounds like it should be split up into separate patches for at
> least the routing and volume control stuff.  It also needs a bit of a
> better changelog as I'm not sure what the issues with the volume
> controls were or what was improper about the old audio paths.

Oh well, will do. Perhaps driver author wants to step in and describe
setup driver was tested with. This is not meant as an offense, but
it does really look like code was copied over from other drivers
(sometimes without renaming) until it compiled and then submitted.
As it is I'm unable to play or record anything as power domains
are not enabled, but there still is a chance I'm doing something
wrong.

> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > ---
> >  I'm still unsure about control names, please see yet unaswered mail
> >  here: https://www.spinics.net/lists/alsa-devel/msg80006.html
> 
> Always CC maintainers on mails - you really shouldn't rely on anyone
> seeing anything that only goes to the lists, they're quite high volume.

Well, in this case I decided to sent general question to the list in a
hope anyone with enough knowledge could answer that... But point taken,
will Cc maintaniners next time.

Re: [PATCH v0 3/3] ASoC: max9867: Fix volume controls

[Mark Brown]

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

On Mon, Jul 30, 2018 at 12:45:03PM +0200, Ladislav Michl wrote:
> On Mon, Jul 30, 2018 at 11:11:15AM +0100, Mark Brown wrote:
> > On Sat, Jul 28, 2018 at 05:44:15PM +0200, Ladislav Michl wrote:
> > > Build proper audio paths and fix volume controls to match datasheet.

> > This sounds like it should be split up into separate patches for at
> > least the routing and volume control stuff.  It also needs a bit of a
> > better changelog as I'm not sure what the issues with the volume
> > controls were or what was improper about the old audio paths.

> Oh well, will do. Perhaps driver author wants to step in and describe
> setup driver was tested with. This is not meant as an offense, but
> it does really look like code was copied over from other drivers
> (sometimes without renaming) until it compiled and then submitted.
> As it is I'm unable to play or record anything as power domains
> are not enabled, but there still is a chance I'm doing something
> wrong.

It wouldn't be the first driver that was used mainly for one use case
and not fully tested, people don't always even have the hardware wired
up to cover everything and try to write things blind from the datasheet
for completeness.

> > >  I'm still unsure about control names, please see yet unaswered mail
> > >  here: https://www.spinics.net/lists/alsa-devel/msg80006.html

> > Always CC maintainers on mails - you really shouldn't rely on anyone
> > seeing anything that only goes to the lists, they're quite high volume.

> Well, in this case I decided to sent general question to the list in a
> hope anyone with enough knowledge could answer that... But point taken,
> will Cc maintaniners next time.

Yeah, it's good to send to the list in case someone else chimes in but
the copy to maintainers helps a lot still even for things that are just
discussions.

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

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