diff mbox series

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

Message ID 20180728154415.GD9665@lenoch (mailing list archive)
State New, archived
Headers show
Series ASoC: max9867: driver update | expand

Commit Message

Ladislav Michl July 28, 2018, 3:44 p.m. UTC 
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(-)

Comments

Mark Brown July 30, 2018, 10:11 a.m. UTC | #1 
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.
Ladislav Michl July 30, 2018, 10:45 a.m. UTC | #2 
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.
Mark Brown July 30, 2018, 10:57 a.m. UTC | #3 
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.
diff mbox series

Patch

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