* Re: sound: patch_realtek: truncating bits
2010-03-28 13:49 sound: patch_realtek: truncating bits Dan Carpenter
@ 2010-03-29 7:25 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2010-03-29 7:25 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel, kailang
At Sun, 28 Mar 2010 16:49:55 +0300,
Dan Carpenter wrote:
>
> Smatch complains that we are truncating bits here:
>
> sound/pci/hda/patch_realtek.c +12459 alc268_aspire_one_speaker_automute(6)
> warn: value 28800 can't fit into 255 'bits'
> sound/pci/hda/patch_realtek.c +13482 alc269_quanta_fl1_speaker_automute(6)
> warn: value 28800 can't fit into 255 'bits'
> sound/pci/hda/patch_realtek.c +13511 alc269_lifebook_speaker_automute(11)
> warn: value 28800 can't fit into 255 'bits'
> sound/pci/hda/patch_realtek.c +13646 alc269_speaker_automute(8)
> warn: value 28800 can't fit into 255 'bits'
>
> The code looks like this:
> 13643 unsigned char bits;
> 13644
> 13645 present = snd_hda_jack_detect(codec, nid);
> 13646 bits = present ? AMP_IN_MUTE(0) : 0;
>
> "bits" is declared as an unsigned char but AMP_IN_MUTE(0) is 0x7080 so
> we are only using the last 0x80.
>
> I couldn't figure out if it was intended. This was first introduced by
> this commit which change "bits" from unsigned int to unsigned char.
>
> commit 60db6b53fb43421beb2ff3fe3e63412bf81620aa
> Author: Kailang Yang <kailang@realtek.com>
> Date: Tue Aug 26 13:13:00 2008 +0200
>
> ALSA: hda - Add support of Quanta FL1
>
> Added the support of Quanta FL1 with ALC269 code chip.
> Also a bit space clean-ups.
>
> If this is intentional, it would be cleaner to explicitly cast or mask
> away the bits we don't care about.
Good catch. These are really bugs. They should be HDA_AMP_MUTE
instead. Looks like a left-over from the conversion to the cached
access in some versions ago. The fix patch is below.
But, this doesn't affect the real behavior, fortunately, because
the values over 8bits are overridden at actual codec access time.
So, it doesn't need to go stable tree.
thanks,
Takashi
---
>From 5dbd5ec6e1cf2e49128025d80813a275744a7ac5 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Mon, 29 Mar 2010 09:16:24 +0200
Subject: [PATCH] ALSA: hda - Fix invalid bit values passed to snd_hda_codec_amp_stereo()
The mask and value parameters passed to snd_hda_codec_amp_stereo()
should be 8-bit values for mute and volume. Passing AMP_IN_MUTE() is
wrong, which is found in many places in patch_realtek.c as a left-over
from the conversion to snd_hda_codec_amp_stereo().
Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/pci/hda/patch_realtek.c | 52 ++++++++++++++++++++--------------------
1 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 9a23444..bc55c1e 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -12459,11 +12459,11 @@ static void alc268_aspire_one_speaker_automute(struct hda_codec *codec)
unsigned char bits;
present = snd_hda_jack_detect(codec, 0x15);
- bits = present ? AMP_IN_MUTE(0) : 0;
+ bits = present ? HDA_AMP_MUTE : 0;
snd_hda_codec_amp_stereo(codec, 0x0f, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0f, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
}
static void alc268_acer_lc_unsol_event(struct hda_codec *codec,
@@ -13482,11 +13482,11 @@ static void alc269_quanta_fl1_speaker_automute(struct hda_codec *codec)
unsigned char bits;
present = snd_hda_jack_detect(codec, 0x15);
- bits = present ? AMP_IN_MUTE(0) : 0;
+ bits = present ? HDA_AMP_MUTE : 0;
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_write(codec, 0x20, 0,
AC_VERB_SET_COEF_INDEX, 0x0c);
@@ -13511,11 +13511,11 @@ static void alc269_lifebook_speaker_automute(struct hda_codec *codec)
/* Check port replicator headphone socket */
present |= snd_hda_jack_detect(codec, 0x1a);
- bits = present ? AMP_IN_MUTE(0) : 0;
+ bits = present ? HDA_AMP_MUTE : 0;
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_write(codec, 0x20, 0,
AC_VERB_SET_COEF_INDEX, 0x0c);
@@ -13646,11 +13646,11 @@ static void alc269_speaker_automute(struct hda_codec *codec)
unsigned char bits;
present = snd_hda_jack_detect(codec, nid);
- bits = present ? AMP_IN_MUTE(0) : 0;
+ bits = present ? HDA_AMP_MUTE : 0;
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
}
/* unsolicited event for HP jack sensing */
@@ -17115,9 +17115,9 @@ static void alc663_m51va_speaker_automute(struct hda_codec *codec)
present = snd_hda_jack_detect(codec, 0x21);
bits = present ? HDA_AMP_MUTE : 0;
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
}
static void alc663_21jd_two_speaker_automute(struct hda_codec *codec)
@@ -17128,13 +17128,13 @@ static void alc663_21jd_two_speaker_automute(struct hda_codec *codec)
present = snd_hda_jack_detect(codec, 0x21);
bits = present ? HDA_AMP_MUTE : 0;
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0e, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0e, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
}
static void alc663_15jd_two_speaker_automute(struct hda_codec *codec)
@@ -17145,13 +17145,13 @@ static void alc663_15jd_two_speaker_automute(struct hda_codec *codec)
present = snd_hda_jack_detect(codec, 0x15);
bits = present ? HDA_AMP_MUTE : 0;
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0e, HDA_INPUT, 0,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
snd_hda_codec_amp_stereo(codec, 0x0e, HDA_INPUT, 1,
- AMP_IN_MUTE(0), bits);
+ HDA_AMP_MUTE, bits);
}
static void alc662_f5z_speaker_automute(struct hda_codec *codec)
@@ -17190,14 +17190,14 @@ static void alc663_two_hp_m2_speaker_automute(struct hda_codec *codec)
if (present1 || present2) {
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 0,
- AMP_IN_MUTE(0), AMP_IN_MUTE(0));
+ HDA_AMP_MUTE, HDA_AMP_MUTE);
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 1,
- AMP_IN_MUTE(0), AMP_IN_MUTE(0));
+ HDA_AMP_MUTE, HDA_AMP_MUTE);
} else {
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 0,
- AMP_IN_MUTE(0), 0);
+ HDA_AMP_MUTE, 0);
snd_hda_codec_amp_stereo(codec, 0x0c, HDA_INPUT, 1,
- AMP_IN_MUTE(0), 0);
+ HDA_AMP_MUTE, 0);
}
}
--
1.7.0.3
^ permalink raw reply related [flat|nested] 2+ messages in thread