* [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes
@ 2011-05-12 5:58 jinyoungp
2011-05-12 5:58 ` [PATCH 1/3] ASoC: codecs: max98088: Fixed invalid register definitions in mixer controls jinyoungp
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: jinyoungp @ 2011-05-12 5:58 UTC (permalink / raw)
To: broonie; +Cc: Peter.Hsiang, alsa-devel, Jin Park, lrg
From: Jin Park <jinyoungp@nvidia.com>
Recently, I working on max98088 and changed something in max98088 asoc driver.
The MAX98088 driver had some bug and needed modifies.
The changes was confirmed by Peter Hsiang who author of max98088 driver and
maxim engineer.
Please review my changes.
Thanks,
Jin Park.
Jin Park (3):
ASoC: codecs: max98088: Fixed invalid register definitions in mixer
controls
ASoC: codecs: max98088: Moved the EX Limiter Mode from dapm widget to
control
ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2
sound/soc/codecs/max98088.c | 62 +++++++++++++++++++++++++++++++------------
sound/soc/codecs/max98088.h | 13 +++++++++
2 files changed, 58 insertions(+), 17 deletions(-)
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] ASoC: codecs: max98088: Fixed invalid register definitions in mixer controls 2011-05-12 5:58 [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes jinyoungp @ 2011-05-12 5:58 ` jinyoungp 2011-05-12 5:58 ` [PATCH 2/3] ASoC: codecs: max98088: Moved the EX Limiter Mode from dapm widget to control jinyoungp ` (3 subsequent siblings) 4 siblings, 0 replies; 13+ messages in thread From: jinyoungp @ 2011-05-12 5:58 UTC (permalink / raw) To: broonie; +Cc: Peter.Hsiang, alsa-devel, Jin Park, lrg From: Jin Park <jinyoungp@nvidia.com> Fixed invalid register definitions in mixer controls such as left speaker mixer, left hp mixer and left rec mixer. Signed-off-by: Jin Park <jinyoungp@nvidia.com> --- sound/soc/codecs/max98088.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index 89498f9..3bdaa04 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -808,10 +808,10 @@ static const struct snd_kcontrol_new max98088_snd_controls[] = { /* Left speaker mixer switch */ static const struct snd_kcontrol_new max98088_left_speaker_mixer_controls[] = { - SOC_DAPM_SINGLE("Left DAC1 Switch", M98088_REG_2B_MIX_SPK_LEFT, 7, 1, 0), - SOC_DAPM_SINGLE("Right DAC1 Switch", M98088_REG_2B_MIX_SPK_LEFT, 0, 1, 0), - SOC_DAPM_SINGLE("Left DAC2 Switch", M98088_REG_2B_MIX_SPK_LEFT, 7, 1, 0), - SOC_DAPM_SINGLE("Right DAC2 Switch", M98088_REG_2B_MIX_SPK_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Left DAC1 Switch", M98088_REG_2B_MIX_SPK_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Right DAC1 Switch", M98088_REG_2B_MIX_SPK_LEFT, 7, 1, 0), + SOC_DAPM_SINGLE("Left DAC2 Switch", M98088_REG_2B_MIX_SPK_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Right DAC2 Switch", M98088_REG_2B_MIX_SPK_LEFT, 7, 1, 0), SOC_DAPM_SINGLE("MIC1 Switch", M98088_REG_2B_MIX_SPK_LEFT, 5, 1, 0), SOC_DAPM_SINGLE("MIC2 Switch", M98088_REG_2B_MIX_SPK_LEFT, 6, 1, 0), SOC_DAPM_SINGLE("INA1 Switch", M98088_REG_2B_MIX_SPK_LEFT, 1, 1, 0), @@ -836,10 +836,10 @@ static const struct snd_kcontrol_new max98088_right_speaker_mixer_controls[] = { /* Left headphone mixer switch */ static const struct snd_kcontrol_new max98088_left_hp_mixer_controls[] = { - SOC_DAPM_SINGLE("Left DAC1 Switch", M98088_REG_25_MIX_HP_LEFT, 7, 1, 0), - SOC_DAPM_SINGLE("Right DAC1 Switch", M98088_REG_25_MIX_HP_LEFT, 0, 1, 0), - SOC_DAPM_SINGLE("Left DAC2 Switch", M98088_REG_25_MIX_HP_LEFT, 7, 1, 0), - SOC_DAPM_SINGLE("Right DAC2 Switch", M98088_REG_25_MIX_HP_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Left DAC1 Switch", M98088_REG_25_MIX_HP_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Right DAC1 Switch", M98088_REG_25_MIX_HP_LEFT, 7, 1, 0), + SOC_DAPM_SINGLE("Left DAC2 Switch", M98088_REG_25_MIX_HP_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Right DAC2 Switch", M98088_REG_25_MIX_HP_LEFT, 7, 1, 0), SOC_DAPM_SINGLE("MIC1 Switch", M98088_REG_25_MIX_HP_LEFT, 5, 1, 0), SOC_DAPM_SINGLE("MIC2 Switch", M98088_REG_25_MIX_HP_LEFT, 6, 1, 0), SOC_DAPM_SINGLE("INA1 Switch", M98088_REG_25_MIX_HP_LEFT, 1, 1, 0), @@ -864,10 +864,10 @@ static const struct snd_kcontrol_new max98088_right_hp_mixer_controls[] = { /* Left earpiece/receiver mixer switch */ static const struct snd_kcontrol_new max98088_left_rec_mixer_controls[] = { - SOC_DAPM_SINGLE("Left DAC1 Switch", M98088_REG_28_MIX_REC_LEFT, 7, 1, 0), - SOC_DAPM_SINGLE("Right DAC1 Switch", M98088_REG_28_MIX_REC_LEFT, 0, 1, 0), - SOC_DAPM_SINGLE("Left DAC2 Switch", M98088_REG_28_MIX_REC_LEFT, 7, 1, 0), - SOC_DAPM_SINGLE("Right DAC2 Switch", M98088_REG_28_MIX_REC_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Left DAC1 Switch", M98088_REG_28_MIX_REC_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Right DAC1 Switch", M98088_REG_28_MIX_REC_LEFT, 7, 1, 0), + SOC_DAPM_SINGLE("Left DAC2 Switch", M98088_REG_28_MIX_REC_LEFT, 0, 1, 0), + SOC_DAPM_SINGLE("Right DAC2 Switch", M98088_REG_28_MIX_REC_LEFT, 7, 1, 0), SOC_DAPM_SINGLE("MIC1 Switch", M98088_REG_28_MIX_REC_LEFT, 5, 1, 0), SOC_DAPM_SINGLE("MIC2 Switch", M98088_REG_28_MIX_REC_LEFT, 6, 1, 0), SOC_DAPM_SINGLE("INA1 Switch", M98088_REG_28_MIX_REC_LEFT, 1, 1, 0), -- 1.7.1 ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] ASoC: codecs: max98088: Moved the EX Limiter Mode from dapm widget to control 2011-05-12 5:58 [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes jinyoungp 2011-05-12 5:58 ` [PATCH 1/3] ASoC: codecs: max98088: Fixed invalid register definitions in mixer controls jinyoungp @ 2011-05-12 5:58 ` jinyoungp 2011-05-12 5:58 ` [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 jinyoungp ` (2 subsequent siblings) 4 siblings, 0 replies; 13+ messages in thread From: jinyoungp @ 2011-05-12 5:58 UTC (permalink / raw) To: broonie; +Cc: Peter.Hsiang, alsa-devel, Jin Park, lrg From: Jin Park <jinyoungp@nvidia.com> Moved the EX Limiter Mode from dapm widget to control, because it was not required DAPM route. Signed-off-by: Jin Park <jinyoungp@nvidia.com> --- sound/soc/codecs/max98088.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-) diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index 3bdaa04..c520093 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -656,8 +656,6 @@ static const struct soc_enum max98088_exmode_enum = ARRAY_SIZE(max98088_exmode_texts), max98088_exmode_texts, max98088_exmode_values); -static const struct snd_kcontrol_new max98088_exmode_controls = - SOC_DAPM_VALUE_ENUM("Route", max98088_exmode_enum); static const char *max98088_ex_thresh[] = { /* volts PP */ "0.6", "1.2", "1.8", "2.4", "3.0", "3.6", "4.2", "4.8"}; @@ -783,6 +781,7 @@ static const struct snd_kcontrol_new max98088_snd_controls[] = { SOC_SINGLE("EQ1 Switch", M98088_REG_49_CFG_LEVEL, 0, 1, 0), SOC_SINGLE("EQ2 Switch", M98088_REG_49_CFG_LEVEL, 1, 1, 0), + SOC_ENUM("EX Limiter Mode", max98088_exmode_enum), SOC_ENUM("EX Limiter Threshold", max98088_ex_thresh_enum), SOC_ENUM("DAI1 Filter Mode", max98088_filter_mode_enum), @@ -1094,9 +1093,6 @@ static const struct snd_soc_dapm_widget max98088_dapm_widgets[] = { SND_SOC_DAPM_MICBIAS("MICBIAS", M98088_REG_4C_PWR_EN_IN, 3, 0), - SND_SOC_DAPM_MUX("EX Limiter Mode", SND_SOC_NOPM, 0, 0, - &max98088_exmode_controls), - SND_SOC_DAPM_OUTPUT("HPL"), SND_SOC_DAPM_OUTPUT("HPR"), SND_SOC_DAPM_OUTPUT("SPKL"), -- 1.7.1 ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 2011-05-12 5:58 [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes jinyoungp 2011-05-12 5:58 ` [PATCH 1/3] ASoC: codecs: max98088: Fixed invalid register definitions in mixer controls jinyoungp 2011-05-12 5:58 ` [PATCH 2/3] ASoC: codecs: max98088: Moved the EX Limiter Mode from dapm widget to control jinyoungp @ 2011-05-12 5:58 ` jinyoungp 2011-05-12 20:43 ` Seungwhan Youn 2011-05-13 7:11 ` Jassi Brar 2011-05-12 8:42 ` [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes Liam Girdwood 2011-05-16 4:33 ` Mark Brown 4 siblings, 2 replies; 13+ messages in thread From: jinyoungp @ 2011-05-12 5:58 UTC (permalink / raw) To: broonie; +Cc: Peter.Hsiang, alsa-devel, Jin Park, lrg From: Jin Park <jinyoungp@nvidia.com> Added digital mute function in DAI1 and DAI2. Signed-off-by: Jin Park <jinyoungp@nvidia.com> --- sound/soc/codecs/max98088.c | 32 ++++++++++++++++++++++++++++++++ sound/soc/codecs/max98088.h | 13 +++++++++++++ 2 files changed, 45 insertions(+), 0 deletions(-) diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index c520093..b5ccf37 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -1582,6 +1582,36 @@ static int max98088_dai2_set_fmt(struct snd_soc_dai *codec_dai, return 0; } +static int max98088_dai1_digital_mute(struct snd_soc_dai *codec_dai, int mute) +{ + struct snd_soc_codec *codec = codec_dai->codec; + int reg; + + if (mute) + reg = M98088_DAI_MUTE; + else + reg = 0; + + snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, + M98088_DAI_MUTE_MASK, reg); + return 0; +} + +static int max98088_dai2_digital_mute(struct snd_soc_dai *codec_dai, int mute) +{ + struct snd_soc_codec *codec = codec_dai->codec; + int reg; + + if (mute) + reg = M98088_DAI_MUTE; + else + reg = 0; + + snd_soc_update_bits(codec, M98088_REG_31_LVL_DAI2_PLAY, + M98088_DAI_MUTE_MASK, reg); + return 0; +} + static void max98088_sync_cache(struct snd_soc_codec *codec) { u16 *reg_cache = codec->reg_cache; @@ -1643,12 +1673,14 @@ static struct snd_soc_dai_ops max98088_dai1_ops = { .set_sysclk = max98088_dai_set_sysclk, .set_fmt = max98088_dai1_set_fmt, .hw_params = max98088_dai1_hw_params, + .digital_mute = max98088_dai1_digital_mute, }; static struct snd_soc_dai_ops max98088_dai2_ops = { .set_sysclk = max98088_dai_set_sysclk, .set_fmt = max98088_dai2_set_fmt, .hw_params = max98088_dai2_hw_params, + .digital_mute = max98088_dai2_digital_mute, }; static struct snd_soc_dai_driver max98088_dai[] = { diff --git a/sound/soc/codecs/max98088.h b/sound/soc/codecs/max98088.h index 56554c7..be89a4f 100644 --- a/sound/soc/codecs/max98088.h +++ b/sound/soc/codecs/max98088.h @@ -133,6 +133,19 @@ #define M98088_REC_LINEMODE (1<<7) #define M98088_REC_LINEMODE_MASK (1<<7) +/* M98088_REG_2D_MIX_SPK_CNTL */ + #define M98088_MIX_SPKR_GAIN_MASK (3<<2) + #define M98088_MIX_SPKR_GAIN_SHIFT 2 + #define M98088_MIX_SPKL_GAIN_MASK (3<<0) + #define M98088_MIX_SPKL_GAIN_SHIFT 0 + +/* M98088_REG_2F_LVL_DAI1_PLAY, M98088_REG_31_LVL_DAI2_PLAY */ + #define M98088_DAI_MUTE (1<<7) + #define M98088_DAI_MUTE_MASK (1<<7) + #define M98088_DAI_VOICE_GAIN_MASK (3<<4) + #define M98088_DAI_ATTENUATION_MASK (0xF<<0) + #define M98088_DAI_ATTENUATION_SHIFT 0 + /* M98088_REG_35_LVL_MIC1, M98088_REG_36_LVL_MIC2 */ #define M98088_MICPRE_MASK (3<<5) #define M98088_MICPRE_SHIFT 5 -- 1.7.1 ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 2011-05-12 5:58 ` [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 jinyoungp @ 2011-05-12 20:43 ` Seungwhan Youn 2011-05-13 6:53 ` Jinyoung Park 2011-05-13 7:06 ` Jassi Brar 2011-05-13 7:11 ` Jassi Brar 1 sibling, 2 replies; 13+ messages in thread From: Seungwhan Youn @ 2011-05-12 20:43 UTC (permalink / raw) To: jinyoungp; +Cc: Peter.Hsiang, alsa-devel, broonie, lrg Hi Mr. Jin Park, On Thu, May 12, 2011 at 2:58 PM, <jinyoungp@nvidia.com> wrote: > From: Jin Park <jinyoungp@nvidia.com> > > Added digital mute function in DAI1 and DAI2. > > Signed-off-by: Jin Park <jinyoungp@nvidia.com> > --- > sound/soc/codecs/max98088.c | 32 ++++++++++++++++++++++++++++++++ > sound/soc/codecs/max98088.h | 13 +++++++++++++ > 2 files changed, 45 insertions(+), 0 deletions(-) > > diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c > index c520093..b5ccf37 100644 > --- a/sound/soc/codecs/max98088.c > +++ b/sound/soc/codecs/max98088.c > @@ -1582,6 +1582,36 @@ static int max98088_dai2_set_fmt(struct snd_soc_dai *codec_dai, > return 0; > } > > +static int max98088_dai1_digital_mute(struct snd_soc_dai *codec_dai, int mute) > +{ > + struct snd_soc_codec *codec = codec_dai->codec; > + int reg; > + > + if (mute) > + reg = M98088_DAI_MUTE; > + else > + reg = 0; In this case, ~M98088_DAI_MUTE is more nice then "0" to me. > + > + snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, > + M98088_DAI_MUTE_MASK, reg); > + return 0; IMHO, we need to chance to get errors during update bits like below. return snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, M98088_DAI_MUTE_MASK, reg); > +} > + > +static int max98088_dai2_digital_mute(struct snd_soc_dai *codec_dai, int mute) > +{ > + struct snd_soc_codec *codec = codec_dai->codec; > + int reg; > + > + if (mute) > + reg = M98088_DAI_MUTE; > + else > + reg = 0; Same. > + > + snd_soc_update_bits(codec, M98088_REG_31_LVL_DAI2_PLAY, > + M98088_DAI_MUTE_MASK, reg); > + return 0; Also. > +} > + > static void max98088_sync_cache(struct snd_soc_codec *codec) > { > u16 *reg_cache = codec->reg_cache; > @@ -1643,12 +1673,14 @@ static struct snd_soc_dai_ops max98088_dai1_ops = { > .set_sysclk = max98088_dai_set_sysclk, > .set_fmt = max98088_dai1_set_fmt, > .hw_params = max98088_dai1_hw_params, > + .digital_mute = max98088_dai1_digital_mute, > }; > > static struct snd_soc_dai_ops max98088_dai2_ops = { > .set_sysclk = max98088_dai_set_sysclk, > .set_fmt = max98088_dai2_set_fmt, > .hw_params = max98088_dai2_hw_params, > + .digital_mute = max98088_dai2_digital_mute, > }; > > static struct snd_soc_dai_driver max98088_dai[] = { > diff --git a/sound/soc/codecs/max98088.h b/sound/soc/codecs/max98088.h > index 56554c7..be89a4f 100644 > --- a/sound/soc/codecs/max98088.h > +++ b/sound/soc/codecs/max98088.h > @@ -133,6 +133,19 @@ > #define M98088_REC_LINEMODE (1<<7) > #define M98088_REC_LINEMODE_MASK (1<<7) > > +/* M98088_REG_2D_MIX_SPK_CNTL */ > + #define M98088_MIX_SPKR_GAIN_MASK (3<<2) > + #define M98088_MIX_SPKR_GAIN_SHIFT 2 > + #define M98088_MIX_SPKL_GAIN_MASK (3<<0) > + #define M98088_MIX_SPKL_GAIN_SHIFT 0 Are these macros really need to add in this patch? Because, there is nothing to use this macros, and also doesn't have any relatives with "digital mute". > + > +/* M98088_REG_2F_LVL_DAI1_PLAY, M98088_REG_31_LVL_DAI2_PLAY */ > + #define M98088_DAI_MUTE (1<<7) > + #define M98088_DAI_MUTE_MASK (1<<7) > + #define M98088_DAI_VOICE_GAIN_MASK (3<<4) > + #define M98088_DAI_ATTENUATION_MASK (0xF<<0) > + #define M98088_DAI_ATTENUATION_SHIFT 0 > + Same to "M98088_DAI_VOICE_GAIN_MASK", "M98088_DAI_ATTENUATION_MASK" and "M98088_DAI_ATTENUATION_SHIFT". _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 2011-05-12 20:43 ` Seungwhan Youn @ 2011-05-13 6:53 ` Jinyoung Park 2011-05-13 7:06 ` Jassi Brar 1 sibling, 0 replies; 13+ messages in thread From: Jinyoung Park @ 2011-05-13 6:53 UTC (permalink / raw) To: 'Seungwhan Youn' Cc: Peter.Hsiang@maxim-ic.com, alsa-devel@alsa-project.org, broonie@opensource.wolfsonmicro.com, lrg@slimlogic.co.uk Hi Seungwhan, > In this case, ~M98088_DAI_MUTE is more nice then "0" to me. if using "~M98088_DAI_MUTE" instead "0" as you mentioned, it will make wrong value. Below is snd_soc_update_bits(). int snd_soc_update_bits(struct snd_soc_codec *codec, unsigned short reg, unsigned int mask, unsigned int value) { int change; unsigned int old, new; old = snd_soc_read(codec, reg); new = (old & ~mask) | value; change = old != new; if (change) snd_soc_write(codec, reg, new); return change; } M98088_DAI_MUTE is (1<<7), it means 0x80, so ~ M98088_DAI_MUTE is 0x7F. If it call snd_soc_update_bits() with "~ M98088_DAI_MUTE" and old value is 0x00 in target reg(M98088_REG_2F_LVL_DAI1_PLAY), the new value will be 0x7F. And then this invaild value will write into target reg. But I want to clear only bit7, in this case, new value should be 0x00. So your comment is not right. And also next your comment... > IMHO, we need to chance to get errors during update bits like below. > > return snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, > M98088_DAI_MUTE_MASK, reg); The snd_soc_update_bits() is returned value what means changed(1) or unchanged(0), it is not update success or fail. So this also is not right. > Are these macros really need to add in this patch? Because, there is nothing to use this macros, > and also doesn't have any relatives with "digital mute". > Same to "M98088_DAI_VOICE_GAIN_MASK", "M98088_DAI_ATTENUATION_MASK" > and "M98088_DAI_ATTENUATION_SHIFT". I have defined all meaningful bits in register, although it does not use any where. Thanks, Jin Park. -----Original Message----- From: Seungwhan Youn [mailto:claude.youn@gmail.com] Sent: Friday, May 13, 2011 5:44 AM To: Jinyoung Park Cc: broonie@opensource.wolfsonmicro.com; Peter.Hsiang@maxim-ic.com; alsa-devel@alsa-project.org; lrg@slimlogic.co.uk Subject: Re: [alsa-devel] [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 Hi Mr. Jin Park, On Thu, May 12, 2011 at 2:58 PM, <jinyoungp@nvidia.com> wrote: > From: Jin Park <jinyoungp@nvidia.com> > > Added digital mute function in DAI1 and DAI2. > > Signed-off-by: Jin Park <jinyoungp@nvidia.com> > --- > sound/soc/codecs/max98088.c | 32 ++++++++++++++++++++++++++++++++ > sound/soc/codecs/max98088.h | 13 +++++++++++++ > 2 files changed, 45 insertions(+), 0 deletions(-) > > diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c > index c520093..b5ccf37 100644 > --- a/sound/soc/codecs/max98088.c > +++ b/sound/soc/codecs/max98088.c > @@ -1582,6 +1582,36 @@ static int max98088_dai2_set_fmt(struct > snd_soc_dai *codec_dai, > return 0; > } > > +static int max98088_dai1_digital_mute(struct snd_soc_dai *codec_dai, > +int mute) { > + struct snd_soc_codec *codec = codec_dai->codec; > + int reg; > + > + if (mute) > + reg = M98088_DAI_MUTE; > + else > + reg = 0; In this case, ~M98088_DAI_MUTE is more nice then "0" to me. > + > + snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, > + M98088_DAI_MUTE_MASK, reg); > + return 0; IMHO, we need to chance to get errors during update bits like below. return snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, M98088_DAI_MUTE_MASK, reg); > +} > + > +static int max98088_dai2_digital_mute(struct snd_soc_dai *codec_dai, > +int mute) { > + struct snd_soc_codec *codec = codec_dai->codec; > + int reg; > + > + if (mute) > + reg = M98088_DAI_MUTE; > + else > + reg = 0; Same. > + > + snd_soc_update_bits(codec, M98088_REG_31_LVL_DAI2_PLAY, > + M98088_DAI_MUTE_MASK, reg); > + return 0; Also. > +} > + > static void max98088_sync_cache(struct snd_soc_codec *codec) > { > u16 *reg_cache = codec->reg_cache; @@ -1643,12 +1673,14 @@ > static struct snd_soc_dai_ops max98088_dai1_ops = { > .set_sysclk = max98088_dai_set_sysclk, > .set_fmt = max98088_dai1_set_fmt, > .hw_params = max98088_dai1_hw_params, > + .digital_mute = max98088_dai1_digital_mute, > }; > > static struct snd_soc_dai_ops max98088_dai2_ops = { > .set_sysclk = max98088_dai_set_sysclk, > .set_fmt = max98088_dai2_set_fmt, > .hw_params = max98088_dai2_hw_params, > + .digital_mute = max98088_dai2_digital_mute, > }; > > static struct snd_soc_dai_driver max98088_dai[] = { diff --git > a/sound/soc/codecs/max98088.h b/sound/soc/codecs/max98088.h index > 56554c7..be89a4f 100644 > --- a/sound/soc/codecs/max98088.h > +++ b/sound/soc/codecs/max98088.h > @@ -133,6 +133,19 @@ > #define M98088_REC_LINEMODE (1<<7) > #define M98088_REC_LINEMODE_MASK (1<<7) > > +/* M98088_REG_2D_MIX_SPK_CNTL */ > + #define M98088_MIX_SPKR_GAIN_MASK (3<<2) > + #define M98088_MIX_SPKR_GAIN_SHIFT 2 > + #define M98088_MIX_SPKL_GAIN_MASK (3<<0) > + #define M98088_MIX_SPKL_GAIN_SHIFT 0 Are these macros really need to add in this patch? Because, there is nothing to use this macros, and also doesn't have any relatives with "digital mute". > + > +/* M98088_REG_2F_LVL_DAI1_PLAY, M98088_REG_31_LVL_DAI2_PLAY */ > + #define M98088_DAI_MUTE (1<<7) > + #define M98088_DAI_MUTE_MASK (1<<7) > + #define M98088_DAI_VOICE_GAIN_MASK (3<<4) > + #define M98088_DAI_ATTENUATION_MASK (0xF<<0) > + #define M98088_DAI_ATTENUATION_SHIFT 0 > + Same to "M98088_DAI_VOICE_GAIN_MASK", "M98088_DAI_ATTENUATION_MASK" and "M98088_DAI_ATTENUATION_SHIFT". ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 2011-05-12 20:43 ` Seungwhan Youn 2011-05-13 6:53 ` Jinyoung Park @ 2011-05-13 7:06 ` Jassi Brar 1 sibling, 0 replies; 13+ messages in thread From: Jassi Brar @ 2011-05-13 7:06 UTC (permalink / raw) To: Seungwhan Youn; +Cc: Peter.Hsiang, alsa-devel, broonie, jinyoungp, lrg On Fri, May 13, 2011 at 2:13 AM, Seungwhan Youn <claude.youn@gmail.com> wrote: > Hi Mr. Jin Park, > > On Thu, May 12, 2011 at 2:58 PM, <jinyoungp@nvidia.com> wrote: >> From: Jin Park <jinyoungp@nvidia.com> >> >> Added digital mute function in DAI1 and DAI2. >> >> Signed-off-by: Jin Park <jinyoungp@nvidia.com> >> --- >> sound/soc/codecs/max98088.c | 32 ++++++++++++++++++++++++++++++++ >> sound/soc/codecs/max98088.h | 13 +++++++++++++ >> 2 files changed, 45 insertions(+), 0 deletions(-) >> >> diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c >> index c520093..b5ccf37 100644 >> --- a/sound/soc/codecs/max98088.c >> +++ b/sound/soc/codecs/max98088.c >> @@ -1582,6 +1582,36 @@ static int max98088_dai2_set_fmt(struct snd_soc_dai *codec_dai, >> return 0; >> } >> >> +static int max98088_dai1_digital_mute(struct snd_soc_dai *codec_dai, int mute) >> +{ >> + struct snd_soc_codec *codec = codec_dai->codec; >> + int reg; >> + >> + if (mute) >> + reg = M98088_DAI_MUTE; >> + else >> + reg = 0; > > In this case, ~M98088_DAI_MUTE is more nice then "0" to me. No need. The snd_soc_update_bits takes care of masking. Better still, one could simply init reg to 0 and discard the else clause. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 2011-05-12 5:58 ` [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 jinyoungp 2011-05-12 20:43 ` Seungwhan Youn @ 2011-05-13 7:11 ` Jassi Brar 2011-05-13 8:25 ` Jinyoung Park 1 sibling, 1 reply; 13+ messages in thread From: Jassi Brar @ 2011-05-13 7:11 UTC (permalink / raw) To: jinyoungp; +Cc: Peter.Hsiang, alsa-devel, broonie, lrg On Thu, May 12, 2011 at 11:28 AM, <jinyoungp@nvidia.com> wrote: > +static int max98088_dai1_digital_mute(struct snd_soc_dai *codec_dai, int mute) > +{ > + struct snd_soc_codec *codec = codec_dai->codec; > + int reg; > + > + if (mute) > + reg = M98088_DAI_MUTE; > + else > + reg = 0; > + > + snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, > + M98088_DAI_MUTE_MASK, reg); > + return 0; > +} > + > +static int max98088_dai2_digital_mute(struct snd_soc_dai *codec_dai, int mute) > +{ > + struct snd_soc_codec *codec = codec_dai->codec; > + int reg; > + > + if (mute) > + reg = M98088_DAI_MUTE; > + else > + reg = 0; > + > + snd_soc_update_bits(codec, M98088_REG_31_LVL_DAI2_PLAY, > + M98088_DAI_MUTE_MASK, reg); > + return 0; > +} max98088_dai1_digital_mute max98088_dai2_digital_mute max98088_dai1_set_fmt max98088_dai2_set_fmt max98088_dai1_hw_params max98088_dai2_hw_params These pairs are essentially same function operating on different registers. Can't we compress them ? _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 2011-05-13 7:11 ` Jassi Brar @ 2011-05-13 8:25 ` Jinyoung Park 2011-05-13 8:37 ` Jassi Brar 2011-05-18 0:28 ` Peter Hsiang 0 siblings, 2 replies; 13+ messages in thread From: Jinyoung Park @ 2011-05-13 8:25 UTC (permalink / raw) To: 'Jassi Brar', Peter.Hsiang@maxim-ic.com Cc: alsa-devel@alsa-project.org, broonie@opensource.wolfsonmicro.com, lrg@slimlogic.co.uk Jassi, I agree with you and it is not difficult. If Peter Hsiang agrees that compress them, I will do that. He is author of max98088 driver. Peter, How do you think about that? Thanks, Jinyoung. -----Original Message----- From: Jassi Brar [mailto:jassisinghbrar@gmail.com] Sent: Friday, May 13, 2011 4:11 PM To: Jinyoung Park Cc: broonie@opensource.wolfsonmicro.com; Peter.Hsiang@maxim-ic.com; alsa-devel@alsa-project.org; lrg@slimlogic.co.uk Subject: Re: [alsa-devel] [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 On Thu, May 12, 2011 at 11:28 AM, <jinyoungp@nvidia.com> wrote: > +static int max98088_dai1_digital_mute(struct snd_soc_dai *codec_dai, > +int mute) { > + struct snd_soc_codec *codec = codec_dai->codec; > + int reg; > + > + if (mute) > + reg = M98088_DAI_MUTE; > + else > + reg = 0; > + > + snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, > + M98088_DAI_MUTE_MASK, reg); > + return 0; > +} > + > +static int max98088_dai2_digital_mute(struct snd_soc_dai *codec_dai, > +int mute) { > + struct snd_soc_codec *codec = codec_dai->codec; > + int reg; > + > + if (mute) > + reg = M98088_DAI_MUTE; > + else > + reg = 0; > + > + snd_soc_update_bits(codec, M98088_REG_31_LVL_DAI2_PLAY, > + M98088_DAI_MUTE_MASK, reg); > + return 0; > +} max98088_dai1_digital_mute max98088_dai2_digital_mute max98088_dai1_set_fmt max98088_dai2_set_fmt max98088_dai1_hw_params max98088_dai2_hw_params These pairs are essentially same function operating on different registers. Can't we compress them ? ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 2011-05-13 8:25 ` Jinyoung Park @ 2011-05-13 8:37 ` Jassi Brar 2011-05-18 0:28 ` Peter Hsiang 1 sibling, 0 replies; 13+ messages in thread From: Jassi Brar @ 2011-05-13 8:37 UTC (permalink / raw) To: Jinyoung Park Cc: Peter.Hsiang@maxim-ic.com, alsa-devel@alsa-project.org, broonie@opensource.wolfsonmicro.com, lrg@slimlogic.co.uk On Fri, May 13, 2011 at 1:55 PM, Jinyoung Park <jinyoungp@nvidia.com> wrote: > Jassi, > I agree with you and it is not difficult. > If Peter Hsiang agrees that compress them, I will do that. > He is author of max98088 driver. Ok you can ask. Though in such trivially good cases, it is better to ask for ack with the final work. Also, please avoid top posting. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 2011-05-13 8:25 ` Jinyoung Park 2011-05-13 8:37 ` Jassi Brar @ 2011-05-18 0:28 ` Peter Hsiang 1 sibling, 0 replies; 13+ messages in thread From: Peter Hsiang @ 2011-05-18 0:28 UTC (permalink / raw) To: Jinyoung Park, 'Jassi Brar' Cc: alsa-devel@alsa-project.org, broonie@opensource.wolfsonmicro.com, lrg@slimlogic.co.uk On Friday, May 13, 2011, <jinyoungp@nvidia.com> wrote: > Peter, > How do you think about that? > > On Friday, May 13, 2011, Jassi Brar wrote: > > +static int max98088_dai1_digital_mute(struct snd_soc_dai *codec_dai, > > +int mute) { > > + struct snd_soc_codec *codec = codec_dai->codec; > > + int reg; > > + > > + if (mute) > > + reg = M98088_DAI_MUTE; > > + else > > + reg = 0; > > + > > + snd_soc_update_bits(codec, M98088_REG_2F_LVL_DAI1_PLAY, > > + M98088_DAI_MUTE_MASK, reg); > > + return 0; > > +} > > + > > max98088_dai1_digital_mute max98088_dai2_digital_mute > > max98088_dai1_set_fmt max98088_dai2_set_fmt > > max98088_dai1_hw_params max98088_dai2_hw_params > > > > These pairs are essentially same function operating on different > > registers. Can't we compress them ? Hi Jinyoung, Thank you for looking at this. The digital mute function is relatively small. It is ok as separate functions. regval = mute ? M98088_DAI_MUTE : 0; For hw_params, I had wanted to combine them too initially. However, there are quite a few different registers in there. For only 2 instantiations, the extra conditional code to enable combining them will somewhat diminish the benefit. I am leaning toward keeping them the way they are. But it's ok with me if you think of a good way to do it. The set_fmt is a possibility. Peter _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes 2011-05-12 5:58 [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes jinyoungp ` (2 preceding siblings ...) 2011-05-12 5:58 ` [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 jinyoungp @ 2011-05-12 8:42 ` Liam Girdwood 2011-05-16 4:33 ` Mark Brown 4 siblings, 0 replies; 13+ messages in thread From: Liam Girdwood @ 2011-05-12 8:42 UTC (permalink / raw) To: jinyoungp; +Cc: Peter.Hsiang, alsa-devel, broonie On Thu, 2011-05-12 at 14:58 +0900, jinyoungp@nvidia.com wrote: > From: Jin Park <jinyoungp@nvidia.com> > > Recently, I working on max98088 and changed something in max98088 asoc driver. > The MAX98088 driver had some bug and needed modifies. > The changes was confirmed by Peter Hsiang who author of max98088 driver and > maxim engineer. > Please review my changes. > > Thanks, > Jin Park. > > Jin Park (3): > ASoC: codecs: max98088: Fixed invalid register definitions in mixer > controls > ASoC: codecs: max98088: Moved the EX Limiter Mode from dapm widget to > control > ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 > > sound/soc/codecs/max98088.c | 62 +++++++++++++++++++++++++++++++------------ > sound/soc/codecs/max98088.h | 13 +++++++++ > 2 files changed, 58 insertions(+), 17 deletions(-) > > All Acked-by: Liam Girdwood <lrg@ti.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes 2011-05-12 5:58 [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes jinyoungp ` (3 preceding siblings ...) 2011-05-12 8:42 ` [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes Liam Girdwood @ 2011-05-16 4:33 ` Mark Brown 4 siblings, 0 replies; 13+ messages in thread From: Mark Brown @ 2011-05-16 4:33 UTC (permalink / raw) To: jinyoungp; +Cc: Peter.Hsiang, alsa-devel, lrg On Thu, May 12, 2011 at 02:58:35PM +0900, jinyoungp@nvidia.com wrote: > Recently, I working on max98088 and changed something in max98088 asoc driver. > The MAX98088 driver had some bug and needed modifies. > The changes was confirmed by Peter Hsiang who author of max98088 driver and > maxim engineer. Applied all, thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-05-18 0:30 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-05-12 5:58 [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes jinyoungp 2011-05-12 5:58 ` [PATCH 1/3] ASoC: codecs: max98088: Fixed invalid register definitions in mixer controls jinyoungp 2011-05-12 5:58 ` [PATCH 2/3] ASoC: codecs: max98088: Moved the EX Limiter Mode from dapm widget to control jinyoungp 2011-05-12 5:58 ` [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2 jinyoungp 2011-05-12 20:43 ` Seungwhan Youn 2011-05-13 6:53 ` Jinyoung Park 2011-05-13 7:06 ` Jassi Brar 2011-05-13 7:11 ` Jassi Brar 2011-05-13 8:25 ` Jinyoung Park 2011-05-13 8:37 ` Jassi Brar 2011-05-18 0:28 ` Peter Hsiang 2011-05-12 8:42 ` [PATCH 0/3] ASoC: codecs: max98088: bug fixes and changes Liam Girdwood 2011-05-16 4:33 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).