| Message ID | 20180718205542.12364-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | eea1662525bd4a158a67ac836b2a1fd9cf77cc81 |
| Headers | show |
On Wed, Jul 18, 2018 at 10:55:37PM +0200, Hans de Goede wrote: > + /* IN1/IN2/IN3 Control */ > SOC_SINGLE_TLV("IN1 Boost", RT5651_IN1_IN2, > RT5651_BST_SFT1, 8, 0, bst_tlv), > SOC_SINGLE_TLV("IN2 Boost", RT5651_IN1_IN2, > RT5651_BST_SFT2, 8, 0, bst_tlv), > + SOC_SINGLE_TLV("IN3 Boost", RT5651_IN3, > + RT5651_BST_SFT1, 8, 0, bst_tlv), These should all have Volume added to the name however that's a preexisting bug so not a blocker on this.
> -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Thursday, July 19, 2018 11:03 PM > To: Hans de Goede > Cc: Liam Girdwood; Bard Liao; Oder Chiou; Pierre-Louis Bossart; Carlo Caione; > alsa-devel@alsa-project.org > Subject: Re: [PATCH 1/6] ASoC: rt5651: Add IN3 Boost volume control > > On Wed, Jul 18, 2018 at 10:55:37PM +0200, Hans de Goede wrote: > > > + /* IN1/IN2/IN3 Control */ > > SOC_SINGLE_TLV("IN1 Boost", RT5651_IN1_IN2, > > RT5651_BST_SFT1, 8, 0, bst_tlv), > > SOC_SINGLE_TLV("IN2 Boost", RT5651_IN1_IN2, > > RT5651_BST_SFT2, 8, 0, bst_tlv), > > + SOC_SINGLE_TLV("IN3 Boost", RT5651_IN3, > > + RT5651_BST_SFT1, 8, 0, bst_tlv), > > These should all have Volume added to the name however that's a > preexisting bug so not a blocker on this. I will add "Volume" to the name for all of them. > > ------Please consider the environment before printing this e-mail.
On 07/19/2018 08:31 PM, Bard Liao wrote: >> -----Original Message----- >> From: Mark Brown [mailto:broonie@kernel.org] >> Sent: Thursday, July 19, 2018 11:03 PM >> To: Hans de Goede >> Cc: Liam Girdwood; Bard Liao; Oder Chiou; Pierre-Louis Bossart; Carlo Caione; >> alsa-devel@alsa-project.org >> Subject: Re: [PATCH 1/6] ASoC: rt5651: Add IN3 Boost volume control >> >> On Wed, Jul 18, 2018 at 10:55:37PM +0200, Hans de Goede wrote: >> >>> + /* IN1/IN2/IN3 Control */ >>> SOC_SINGLE_TLV("IN1 Boost", RT5651_IN1_IN2, >>> RT5651_BST_SFT1, 8, 0, bst_tlv), >>> SOC_SINGLE_TLV("IN2 Boost", RT5651_IN1_IN2, >>> RT5651_BST_SFT2, 8, 0, bst_tlv), >>> + SOC_SINGLE_TLV("IN3 Boost", RT5651_IN3, >>> + RT5651_BST_SFT1, 8, 0, bst_tlv), >> These should all have Volume added to the name however that's a >> preexisting bug so not a blocker on this. > I will add "Volume" to the name for all of them. isn't this going to break userspace configs if we change the name of controls?
> -----Original Message----- > From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com] > Sent: Friday, July 20, 2018 9:40 AM > To: Bard Liao; Mark Brown; Hans de Goede > Cc: Liam Girdwood; Oder Chiou; alsa-devel@alsa-project.org; Carlo Caione > Subject: Re: [alsa-devel] [PATCH 1/6] ASoC: rt5651: Add IN3 Boost volume > control > > On 07/19/2018 08:31 PM, Bard Liao wrote: > >> -----Original Message----- > >> From: Mark Brown [mailto:broonie@kernel.org] > >> Sent: Thursday, July 19, 2018 11:03 PM > >> To: Hans de Goede > >> Cc: Liam Girdwood; Bard Liao; Oder Chiou; Pierre-Louis Bossart; Carlo > Caione; > >> alsa-devel@alsa-project.org > >> Subject: Re: [PATCH 1/6] ASoC: rt5651: Add IN3 Boost volume control > >> > >> On Wed, Jul 18, 2018 at 10:55:37PM +0200, Hans de Goede wrote: > >> > >>> + /* IN1/IN2/IN3 Control */ > >>> SOC_SINGLE_TLV("IN1 Boost", RT5651_IN1_IN2, > >>> RT5651_BST_SFT1, 8, 0, bst_tlv), > >>> SOC_SINGLE_TLV("IN2 Boost", RT5651_IN1_IN2, > >>> RT5651_BST_SFT2, 8, 0, bst_tlv), > >>> + SOC_SINGLE_TLV("IN3 Boost", RT5651_IN3, > >>> + RT5651_BST_SFT1, 8, 0, bst_tlv), > >> These should all have Volume added to the name however that's a > >> preexisting bug so not a blocker on this. > > I will add "Volume" to the name for all of them. > isn't this going to break userspace configs if we change the name of > controls? Yes, it should be modified, too. > > ------Please consider the environment before printing this e-mail.
Hi, On 07/20/2018 04:53 AM, Bard Liao wrote: >> -----Original Message----- >> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com] >> Sent: Friday, July 20, 2018 9:40 AM >> To: Bard Liao; Mark Brown; Hans de Goede >> Cc: Liam Girdwood; Oder Chiou; alsa-devel@alsa-project.org; Carlo Caione >> Subject: Re: [alsa-devel] [PATCH 1/6] ASoC: rt5651: Add IN3 Boost volume >> control >> >> On 07/19/2018 08:31 PM, Bard Liao wrote: >>>> -----Original Message----- >>>> From: Mark Brown [mailto:broonie@kernel.org] >>>> Sent: Thursday, July 19, 2018 11:03 PM >>>> To: Hans de Goede >>>> Cc: Liam Girdwood; Bard Liao; Oder Chiou; Pierre-Louis Bossart; Carlo >> Caione; >>>> alsa-devel@alsa-project.org >>>> Subject: Re: [PATCH 1/6] ASoC: rt5651: Add IN3 Boost volume control >>>> >>>> On Wed, Jul 18, 2018 at 10:55:37PM +0200, Hans de Goede wrote: >>>> >>>>> + /* IN1/IN2/IN3 Control */ >>>>> SOC_SINGLE_TLV("IN1 Boost", RT5651_IN1_IN2, >>>>> RT5651_BST_SFT1, 8, 0, bst_tlv), >>>>> SOC_SINGLE_TLV("IN2 Boost", RT5651_IN1_IN2, >>>>> RT5651_BST_SFT2, 8, 0, bst_tlv), >>>>> + SOC_SINGLE_TLV("IN3 Boost", RT5651_IN3, >>>>> + RT5651_BST_SFT1, 8, 0, bst_tlv), >>>> These should all have Volume added to the name however that's a >>>> preexisting bug so not a blocker on this. >>> I will add "Volume" to the name for all of them. >> isn't this going to break userspace configs if we change the name of >> controls? > > Yes, it should be modified, too. Or we just live with the "Volume" part missing for these 3 and don't break users existing setup for no good reason. Regards, Hans
> -----Original Message----- > From: Hans de Goede [mailto:hdegoede@redhat.com] > Sent: Friday, July 27, 2018 8:02 PM > To: Bard Liao; Pierre-Louis Bossart; Mark Brown > Cc: Liam Girdwood; Oder Chiou; alsa-devel@alsa-project.org; Carlo Caione > Subject: Re: [alsa-devel] [PATCH 1/6] ASoC: rt5651: Add IN3 Boost volume > control > > >> isn't this going to break userspace configs if we change the name of > >> controls? > > > > Yes, it should be modified, too. > > Or we just live with the "Volume" part missing for these 3 and don't > break users existing setup for no good reason. Agree. > > Regards, > > Hans > > ------Please consider the environment before printing this e-mail.
On Fri, Jul 27, 2018 at 02:01:44PM +0200, Hans de Goede wrote: > > Yes, it should be modified, too. > Or we just live with the "Volume" part missing for these 3 and don't > break users existing setup for no good reason. You're a few days behind the discussion on the patch series doing this...
diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c index 0462049e739c..985852fd9723 100644 --- a/sound/soc/codecs/rt5651.c +++ b/sound/soc/codecs/rt5651.c @@ -331,11 +331,13 @@ static const struct snd_kcontrol_new rt5651_snd_controls[] = { SOC_DOUBLE_TLV("Mono DAC Playback Volume", RT5651_DAC2_DIG_VOL, RT5651_L_VOL_SFT, RT5651_R_VOL_SFT, 175, 0, dac_vol_tlv), - /* IN1/IN2 Control */ + /* IN1/IN2/IN3 Control */ SOC_SINGLE_TLV("IN1 Boost", RT5651_IN1_IN2, RT5651_BST_SFT1, 8, 0, bst_tlv), SOC_SINGLE_TLV("IN2 Boost", RT5651_IN1_IN2, RT5651_BST_SFT2, 8, 0, bst_tlv), + SOC_SINGLE_TLV("IN3 Boost", RT5651_IN3, + RT5651_BST_SFT1, 8, 0, bst_tlv), /* INL/INR Volume Control */ SOC_DOUBLE_TLV("IN Capture Volume", RT5651_INL1_INR1_VOL, RT5651_INL_VOL_SFT, RT5651_INR_VOL_SFT,
Add a mixer control for the IN3 Boost volume, IN3 is used for the headset mic on most devices, so this is necessary to control the headset mic volume. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- sound/soc/codecs/rt5651.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)