[alsa-devel] [PATCH 3/3] ASoC: codecs: max98088: Added digital mute function in DAI1 and DAI2
Jinyoung Park
jinyoungp at nvidia.com
Fri May 13 08:53:45 CEST 2011
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 at gmail.com]
Sent: Friday, May 13, 2011 5:44 AM
To: Jinyoung Park
Cc: broonie at opensource.wolfsonmicro.com; Peter.Hsiang at maxim-ic.com; alsa-devel at alsa-project.org; lrg at 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 at nvidia.com> wrote:
> From: Jin Park <jinyoungp at nvidia.com>
>
> Added digital mute function in DAI1 and DAI2.
>
> Signed-off-by: Jin Park <jinyoungp at 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.
-----------------------------------------------------------------------------------
More information about the Alsa-devel
mailing list