| Message ID | 496805f7ca084e9dbcf7140cbe83ed4b@realtek.com |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: rt5682s: Add LDO control for dacref | expand |
> -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Tuesday, October 31, 2023 9:30 PM > To: Jack Yu <jack.yu@realtek.com> > Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; lars@metafoo.de; > Flove(HsinFu) <flove@realtek.com>; Oder Chiou <oder_chiou@realtek.com>; > Shuming [范書銘] <shumingf@realtek.com>; Derek [方德義] > <derek.fang@realtek.com> > Subject: Re: [PATCH] ASoC: rt5682s: Add LDO control for dacref > > On Tue, Oct 31, 2023 at 02:08:10AM +0000, Jack Yu wrote: > > > +/* LDO output select */ > > +static const char * const rt5682s_ldo_output_select[] = { > > + "1.607V", "1.5V", "1.406V", "1.731V" > > +}; > > This feels like something that might be a better fit for firmware based > selection - how would someone set a value for this, and why might it vary at > runtime? I'm a bit unclear as to what the control does so this might be the > best thing but perhaps not. This control is added for specific customers, it won't be changed during runtime, but will be set in the initialization regarding to different customers, they'll set it in their own ucm and have already been verified by customers.
On Wed, Nov 01, 2023 at 02:52:20AM +0000, Jack Yu wrote: > > > +/* LDO output select */ > > > +static const char * const rt5682s_ldo_output_select[] = { > > > + "1.607V", "1.5V", "1.406V", "1.731V" > > > +}; > > This feels like something that might be a better fit for firmware based > > selection - how would someone set a value for this, and why might it vary at > > runtime? I'm a bit unclear as to what the control does so this might be the > > best thing but perhaps not. > This control is added for specific customers, it won't be changed during runtime, > but will be set in the initialization regarding to different customers, > they'll set it in their own ucm and have already been verified by customers. That sounds like it should come from the firmware then if it's supposed to be fixed for a given system (which is what the above sounds like).
-----Original Message----- From: Mark Brown <broonie@kernel.org> Sent: Wednesday, November 1, 2023 9:01 PM To: Jack Yu <jack.yu@realtek.com> Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; lars@metafoo.de; Flove(HsinFu) <flove@realtek.com>; Oder Chiou <oder_chiou@realtek.com>; Shuming [范書銘] <shumingf@realtek.com>; Derek [方德義] <derek.fang@realtek.com> Subject: Re: [PATCH] ASoC: rt5682s: Add LDO control for dacref On Wed, Nov 01, 2023 at 02:52:20AM +0000, Jack Yu wrote: > > > +/* LDO output select */ > > > +static const char * const rt5682s_ldo_output_select[] = { > > > + "1.607V", "1.5V", "1.406V", "1.731V" > > > +}; > > This feels like something that might be a better fit for firmware > > based selection - how would someone set a value for this, and why > > might it vary at runtime? I'm a bit unclear as to what the control > > does so this might be the best thing but perhaps not. > This control is added for specific customers, it won't be changed > during runtime, but will be set in the initialization regarding to > different customers, they'll set it in their own ucm and have already been verified by customers. That sounds like it should come from the firmware then if it's supposed to be fixed for a given system (which is what the above sounds like). Yes, maybe it's better to be configured by firmware, but there is no firmware to set this LDO setting in this case, so we provide this control for customer to do their own setting in UCM.
On Wed, Nov 01, 2023 at 02:02:07PM +0000, Jack Yu wrote: > > That sounds like it should come from the firmware then if it's supposed to be fixed for a given system (which is what the above sounds like). > Yes, maybe it's better to be configured by firmware, but there is no firmware to set this LDO setting in this case, > so we provide this control for customer to do their own setting in UCM. When I say "firmware" here I'm talking about DT or ACPI rather than something running on the device. If you have existing systems you can always use quirks like other drivers, though that's obviously not ideal.
diff --git a/sound/soc/codecs/rt5682s.c b/sound/soc/codecs/rt5682s.c index c261c33c4be7..c05996e8fc5a 100644 --- a/sound/soc/codecs/rt5682s.c +++ b/sound/soc/codecs/rt5682s.c @@ -1017,6 +1017,15 @@ static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -1725, 75, 0); static const DECLARE_TLV_DB_SCALE(adc_bst_tlv, 0, 1200, 0); static const DECLARE_TLV_DB_SCALE(cbj_bst_tlv, -1200, 150, 0); +/* LDO output select */ +static const char * const rt5682s_ldo_output_select[] = { + "1.607V", "1.5V", "1.406V", "1.731V" +}; + +static SOC_ENUM_SINGLE_DECL(rt5682s_ldo_output_enum, RT5682S_BIAS_CUR_CTRL_7, + RT5682S_DVO_LDO_DACREF_SFT, rt5682s_ldo_output_select); + + static const struct snd_kcontrol_new rt5682s_snd_controls[] = { /* DAC Digital Volume */ SOC_DOUBLE_TLV("DAC1 Playback Volume", RT5682S_DAC1_DIG_VOL, @@ -1035,6 +1044,9 @@ static const struct snd_kcontrol_new rt5682s_snd_controls[] = { /* ADC Boost Volume Control */ SOC_DOUBLE_TLV("STO1 ADC Boost Gain Volume", RT5682S_STO1_ADC_BOOST, RT5682S_STO1_ADC_L_BST_SFT, RT5682S_STO1_ADC_R_BST_SFT, 3, 0, adc_bst_tlv), + + /* LDO Output Select Control */ + SOC_ENUM("LDO Output Select", rt5682s_ldo_output_enum), }; /** diff --git a/sound/soc/codecs/rt5682s.h b/sound/soc/codecs/rt5682s.h index 1d79d432d0d8..0ae5a35825f4 100644 --- a/sound/soc/codecs/rt5682s.h +++ b/sound/soc/codecs/rt5682s.h @@ -1263,6 +1263,9 @@ #define RT5682S_JDH_NO_PLUG (0x1 << 4) #define RT5682S_JDH_PLUG (0x0 << 4) +/* Bias current control 7 (0x0110) */ +#define RT5682S_DVO_LDO_DACREF_SFT 4 + /* Charge Pump Internal Register1 (0x0125) */ #define RT5682S_CP_CLK_HP_MASK (0x3 << 4) #define RT5682S_CP_CLK_HP_100KHZ (0x0 << 4)
Add LDO control for dacref. Signed-off-by: Jack Yu <jack.yu@realtek.com> --- sound/soc/codecs/rt5682s.c | 12 ++++++++++++ sound/soc/codecs/rt5682s.h | 3 +++ 2 files changed, 15 insertions(+)