diff mbox series

ASoC: uniphier: evea: add switch for changing source of line-in

Message ID 20180314123900.19505-1-suzuki.katsuhiro@socionext.com
State Accepted
Commit 90e0fb05e5c1b1cf6a59c4f888f500e2b1feffc4
Headers show
Series ASoC: uniphier: evea: add switch for changing source of line-in | expand

Commit Message

Katsuhiro Suzuki March 14, 2018, 12:39 p.m. UTC 
This patch adds mixer switch for changing audio source of line-in.
We can choose one of LIN1, 2, 3, default is LIN1.

Signed-off-by: Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com>

---
 sound/soc/uniphier/evea.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

-- 
2.16.1

Comments

Katsuhiro Suzuki March 19, 2018, 4:19 a.m. UTC | #1 
Hello Mark,

> -----Original Message-----

> From: Mark Brown [mailto:broonie@kernel.org]

> Sent: Thursday, March 15, 2018 1:26 AM

> To: Suzuki, Katsuhiro <suzuki.katsuhiro@socionext.com>

> Cc: alsa-devel@alsa-project.org; Masami Hiramatsu

<masami.hiramatsu@linaro.org>;
> Jassi Brar <jaswinder.singh@linaro.org>; linux-arm-kernel@lists.infradead.org;

> linux-kernel@vger.kernel.org

> Subject: Re: [PATCH] ASoC: uniphier: evea: add switch for changing source of

line-in
> 

> On Wed, Mar 14, 2018 at 09:39:00PM +0900, Katsuhiro Suzuki wrote:

> > This patch adds mixer switch for changing audio source of line-in.

> > We can choose one of LIN1, 2, 3, default is LIN1.

> 

> I'll apply for now but this should really be a DAPM control so that we

> can power down things connected to the disconnected line inputs when

> recording.


Thanks a lot for your suggestion. I tried to change the implementation to DAPM
control as follows:

----------
static const char * const linsw1_sel1_text[] = {
	"LIN1", "LIN2", "LIN3"
};

static SOC_ENUM_SINGLE_DECL(linsw1_sel1_enum,
	ALINSW1, ALINSW1_SEL1_SHIFT,
	linsw1_sel1_text);

static const struct snd_kcontrol_new linesw1_mux =
	SOC_DAPM_ENUM("Line In 1 Source", linsw1_sel1_enum);

static const struct snd_soc_dapm_widget evea_widgets[] = {
	SND_SOC_DAPM_ADC("ADC", NULL, SND_SOC_NOPM, 0, 0),
	SND_SOC_DAPM_MUX("Line In 1 Mux", SND_SOC_NOPM, 0, 0, &linesw1_mux),
	SND_SOC_DAPM_INPUT("LIN1_LP"),
	SND_SOC_DAPM_INPUT("LIN1_RP"),
	SND_SOC_DAPM_INPUT("LIN2_LP"),
	SND_SOC_DAPM_INPUT("LIN2_RP"),
	SND_SOC_DAPM_INPUT("LIN3_LP"),
	SND_SOC_DAPM_INPUT("LIN3_RP"),

	SND_SOC_DAPM_DAC("DAC HP", NULL, SND_SOC_NOPM, 0, 0),
	SND_SOC_DAPM_DAC("DAC LO1", NULL, SND_SOC_NOPM, 0, 0),
	SND_SOC_DAPM_DAC("DAC LO2", NULL, SND_SOC_NOPM, 0, 0),
	SND_SOC_DAPM_OUTPUT("HP1_L"),
	SND_SOC_DAPM_OUTPUT("HP1_R"),
	SND_SOC_DAPM_OUTPUT("LO2_L"),
	SND_SOC_DAPM_OUTPUT("LO2_R"),
};

static const struct snd_soc_dapm_route evea_routes[] = {
	{ "Line In 1", NULL, "ADC" },
	{ "ADC", NULL, "Line In 1 Mux" },
	{ "Line In 1 Mux", NULL, "LIN1_LP" },
	{ "Line In 1 Mux", NULL, "LIN1_RP" },
	{ "Line In 1 Mux", NULL, "LIN2_LP" },
	{ "Line In 1 Mux", NULL, "LIN2_RP" },
	{ "Line In 1 Mux", NULL, "LIN3_LP" },
	{ "Line In 1 Mux", NULL, "LIN3_RP" },

	{ "DAC HP", NULL, "Headphone 1" },
	{ "DAC LO1", NULL, "Line Out 1" },
	{ "DAC LO2", NULL, "Line Out 2" },
	{ "HP1_L", NULL, "DAC HP" },
	{ "HP1_R", NULL, "DAC HP" },
	{ "LO2_L", NULL, "DAC LO2" },
	{ "LO2_R", NULL, "DAC LO2" },
};
----------

I can see the value of ALINSW1 register at 'Line In 1 Mux',0 using
  amixer get 'Line In 1 Mux',0

But I can't change the value.
  amixer set 'Line In 1 Mux',0 LIN2
  Simple mixer control 'Line In 1 Mux',0
    Capabilities: enum
    Items: 'LIN1' 'LIN2' 'LIN3'
    Item0: 'LIN1'

Would you tell me what is wrong...


Regards,
--
Katsuhiro Suzuki
Katsuhiro Suzuki March 20, 2018, 2:35 a.m. UTC | #2 
Hello Mark,

> -----Original Message-----

> From: Mark Brown [mailto:broonie@kernel.org]

> Sent: Tuesday, March 20, 2018 10:12 AM

> To: Suzuki, Katsuhiro <suzuki.katsuhiro@socionext.com>

> Cc: alsa-devel@alsa-project.org; Masami Hiramatsu

<masami.hiramatsu@linaro.org>;
> Jassi Brar <jaswinder.singh@linaro.org>; linux-arm-kernel@lists.infradead.org;

> linux-kernel@vger.kernel.org

> Subject: Re: [PATCH] ASoC: uniphier: evea: add switch for changing source of

line-in
> 

> On Mon, Mar 19, 2018 at 01:19:10PM +0900, Katsuhiro Suzuki wrote:

> 

> > > I'll apply for now but this should really be a DAPM control so that we

> > > can power down things connected to the disconnected line inputs when

> > > recording.

> 

> > Thanks a lot for your suggestion. I tried to change the implementation to

DAPM
> > control as follows:

> 

> > I can see the value of ALINSW1 register at 'Line In 1 Mux',0 using

> >   amixer get 'Line In 1 Mux',0

> 

> > But I can't change the value.

> >   amixer set 'Line In 1 Mux',0 LIN2

> >   Simple mixer control 'Line In 1 Mux',0

> >     Capabilities: enum

> >     Items: 'LIN1' 'LIN2' 'LIN3'

> >     Item0: 'LIN1'

> 

> > Would you tell me what is wrong...

> 

> Ugh, I *have* run into that before but I can't remember what triggers it

> and your code doesn't have any mistakes I can spot.  Unfortunately I'm


Thank you for reviewing.


> at Linaro Connect this week and don't have a test system I can poke at

> with me to remind myself, and I'm still travelling next week

> unfortunately.

> 


I see, no problem. Have a nice trip!


> I'd add some trace to the set code path to make sure everything is being

> called as expected.  It's somemthing really small that's hard to make a

> warning for in the code IIRC.


I traced some DAPM codes. The 'get' function as follows:

int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
        struct snd_ctl_elem_value *ucontrol)
{
        //...

        mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
        if (e->reg != SND_SOC_NOPM && dapm_kcontrol_is_powered(kcontrol)) {
                int ret = soc_dapm_read(dapm, e->reg, &reg_val);

The soc_dapm_read() reads real value of register. It's simple.


But 'put' function is mysterious for me...

int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
        struct snd_ctl_elem_value *ucontrol)
{
        //...

        change = dapm_kcontrol_set_value(kcontrol, val);

        if (e->reg != SND_SOC_NOPM)
                reg_change = soc_dapm_test_bits(dapm, e->reg, mask, val);

dapm_kcontrol_set_value() has stored value into dapm_kcontrol_data. And 
soc_dapm_test_bits() has just checked value of a register and returned need 
update or not. It seems anyone does not update a register in this function.

I tried to change soc_dapm_test_bits() -> soc_dapm_update_bits(), so I can 
change value of register using amixer. But I feel this change was wrong. And I 
found dapm_seq_run_coalesced() calls soc_dapm_update_bits(). Unfortunately 
this function has not been called even if I run amixer.

Anyway, I'll continue to study about DAPM codes.


Regards,
--
Katsuhiro Suzuki
diff mbox series

Patch

diff --git a/sound/soc/uniphier/evea.c b/sound/soc/uniphier/evea.c
index 439f14f91b23..73fd6730095c 100644
--- a/sound/soc/uniphier/evea.c
+++ b/sound/soc/uniphier/evea.c
@@ -18,6 +18,8 @@ 
 
 #define AADCPOW(n)                           (0x0078 + 0x04 * (n))
 #define   AADCPOW_AADC_POWD                   BIT(0)
+#define ALINSW1                              0x0088
+#define   ALINSW1_SEL1_SHIFT                  3
 #define AHPOUTPOW                            0x0098
 #define   AHPOUTPOW_HP_ON                     BIT(4)
 #define ALINEPOW                             0x009c
@@ -278,7 +280,16 @@  static int evea_set_switch_hp(struct snd_kcontrol *kcontrol,
 	return evea_update_switch_hp(evea);
 }
 
+static const char * const linsw1_sel1_text[] = {
+	"LIN1", "LIN2", "LIN3"
+};
+
+static SOC_ENUM_SINGLE_DECL(linsw1_sel1_enum,
+	ALINSW1, ALINSW1_SEL1_SHIFT,
+	linsw1_sel1_text);
+
 static const struct snd_kcontrol_new evea_controls[] = {
+	SOC_ENUM("Line Capture Source", linsw1_sel1_enum),
 	SOC_SINGLE_BOOL_EXT("Line Capture Switch", 0,
 			    evea_get_switch_lin, evea_set_switch_lin),
 	SOC_SINGLE_BOOL_EXT("Line Playback Switch", 0,