* Re: ALSA: hda - Use generic parser for Cirrus codec driver
@ 2015-07-24 16:12 Dan Carpenter
2015-07-24 16:18 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2015-07-24 16:12 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel
Hello Takashi Iwai,
The patch 1077a024812d: "ALSA: hda - Use generic parser for Cirrus
codec driver" from Dec 19, 2012, leads to the following static
checker warning:
sound/pci/hda/patch_cirrus.c:1004 cs4210_spdif_automute()
warn: we tested 'spdif_present' before and it was 'true'
sound/pci/hda/patch_cirrus.c
984 static void cs4210_spdif_automute(struct hda_codec *codec,
985 struct hda_jack_callback *tbl)
986 {
987 struct cs_spec *spec = codec->spec;
988 bool spdif_present = false;
989 hda_nid_t spdif_pin = spec->gen.autocfg.dig_out_pins[0];
990
991 /* detect on spdif is specific to CS4210 */
992 if (!spec->spdif_detect ||
993 spec->vendor_nid != CS4210_VENDOR_NID)
994 return;
995
996 spdif_present = snd_hda_jack_detect(codec, spdif_pin);
997 if (spdif_present == spec->spdif_present)
998 return;
999
1000 spec->spdif_present = spdif_present;
1001 /* SPDIF TX on/off */
1002 if (spdif_present)
^^^^^^^^^^^^^
1003 snd_hda_set_pin_ctl(codec, spdif_pin,
1004 spdif_present ? PIN_OUT : 0);
^^^^^^^^^^^^^
I'm not positive but I think this is buggy. We should remove the if
statement.
1005
1006 cs_automute(codec);
1007 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ALSA: hda - Use generic parser for Cirrus codec driver
2015-07-24 16:12 ALSA: hda - Use generic parser for Cirrus codec driver Dan Carpenter
@ 2015-07-24 16:18 ` Takashi Iwai
2015-07-25 0:03 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2015-07-24 16:18 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel
On Fri, 24 Jul 2015 18:12:28 +0200,
Dan Carpenter wrote:
>
> Hello Takashi Iwai,
>
> The patch 1077a024812d: "ALSA: hda - Use generic parser for Cirrus
> codec driver" from Dec 19, 2012, leads to the following static
> checker warning:
>
> sound/pci/hda/patch_cirrus.c:1004 cs4210_spdif_automute()
> warn: we tested 'spdif_present' before and it was 'true'
>
> sound/pci/hda/patch_cirrus.c
> 984 static void cs4210_spdif_automute(struct hda_codec *codec,
> 985 struct hda_jack_callback *tbl)
> 986 {
> 987 struct cs_spec *spec = codec->spec;
> 988 bool spdif_present = false;
> 989 hda_nid_t spdif_pin = spec->gen.autocfg.dig_out_pins[0];
> 990
> 991 /* detect on spdif is specific to CS4210 */
> 992 if (!spec->spdif_detect ||
> 993 spec->vendor_nid != CS4210_VENDOR_NID)
> 994 return;
> 995
> 996 spdif_present = snd_hda_jack_detect(codec, spdif_pin);
> 997 if (spdif_present == spec->spdif_present)
> 998 return;
> 999
> 1000 spec->spdif_present = spdif_present;
> 1001 /* SPDIF TX on/off */
> 1002 if (spdif_present)
> ^^^^^^^^^^^^^
> 1003 snd_hda_set_pin_ctl(codec, spdif_pin,
> 1004 spdif_present ? PIN_OUT : 0);
> ^^^^^^^^^^^^^
>
> I'm not positive but I think this is buggy. We should remove the if
> statement.
Yes, looks so. Could you submit a fix patch?
Thanks!
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch] ALSA: hda - fix cs4210_spdif_automute()
2015-07-24 16:18 ` Takashi Iwai
@ 2015-07-25 0:03 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2015-07-25 0:03 UTC (permalink / raw)
To: Jaroslav Kysela; +Cc: Takashi Iwai, alsa-devel, kernel-janitors
Smatch complains that we have nested checks for "spdif_present". It
turns out the current behavior isn't correct, we should remove the first
check and keep the second.
Fixes: 1077a024812d ('ALSA: hda - Use generic parser for Cirrus codec driver')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 25ccf78..584a034 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -999,9 +999,7 @@ static void cs4210_spdif_automute(struct hda_codec *codec,
spec->spdif_present = spdif_present;
/* SPDIF TX on/off */
- if (spdif_present)
- snd_hda_set_pin_ctl(codec, spdif_pin,
- spdif_present ? PIN_OUT : 0);
+ snd_hda_set_pin_ctl(codec, spdif_pin, spdif_present ? PIN_OUT : 0);
cs_automute(codec);
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [patch] ALSA: hda - fix cs4210_spdif_automute()
@ 2015-07-25 0:03 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2015-07-25 0:03 UTC (permalink / raw)
To: Jaroslav Kysela; +Cc: Takashi Iwai, alsa-devel, kernel-janitors
Smatch complains that we have nested checks for "spdif_present". It
turns out the current behavior isn't correct, we should remove the first
check and keep the second.
Fixes: 1077a024812d ('ALSA: hda - Use generic parser for Cirrus codec driver')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 25ccf78..584a034 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -999,9 +999,7 @@ static void cs4210_spdif_automute(struct hda_codec *codec,
spec->spdif_present = spdif_present;
/* SPDIF TX on/off */
- if (spdif_present)
- snd_hda_set_pin_ctl(codec, spdif_pin,
- spdif_present ? PIN_OUT : 0);
+ snd_hda_set_pin_ctl(codec, spdif_pin, spdif_present ? PIN_OUT : 0);
cs_automute(codec);
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] ALSA: hda - fix cs4210_spdif_automute()
2015-07-25 0:03 ` Dan Carpenter
@ 2015-07-25 6:20 ` Takashi Iwai
-1 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2015-07-25 6:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jaroslav Kysela, alsa-devel, kernel-janitors
On Sat, 25 Jul 2015 02:03:38 +0200,
Dan Carpenter wrote:
>
> Smatch complains that we have nested checks for "spdif_present". It
> turns out the current behavior isn't correct, we should remove the first
> check and keep the second.
>
> Fixes: 1077a024812d ('ALSA: hda - Use generic parser for Cirrus codec driver')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied, thanks.
Takashi
>
> diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
> index 25ccf78..584a034 100644
> --- a/sound/pci/hda/patch_cirrus.c
> +++ b/sound/pci/hda/patch_cirrus.c
> @@ -999,9 +999,7 @@ static void cs4210_spdif_automute(struct hda_codec *codec,
>
> spec->spdif_present = spdif_present;
> /* SPDIF TX on/off */
> - if (spdif_present)
> - snd_hda_set_pin_ctl(codec, spdif_pin,
> - spdif_present ? PIN_OUT : 0);
> + snd_hda_set_pin_ctl(codec, spdif_pin, spdif_present ? PIN_OUT : 0);
>
> cs_automute(codec);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ALSA: hda - fix cs4210_spdif_automute()
@ 2015-07-25 6:20 ` Takashi Iwai
0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2015-07-25 6:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jaroslav Kysela, alsa-devel, kernel-janitors
On Sat, 25 Jul 2015 02:03:38 +0200,
Dan Carpenter wrote:
>
> Smatch complains that we have nested checks for "spdif_present". It
> turns out the current behavior isn't correct, we should remove the first
> check and keep the second.
>
> Fixes: 1077a024812d ('ALSA: hda - Use generic parser for Cirrus codec driver')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied, thanks.
Takashi
>
> diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
> index 25ccf78..584a034 100644
> --- a/sound/pci/hda/patch_cirrus.c
> +++ b/sound/pci/hda/patch_cirrus.c
> @@ -999,9 +999,7 @@ static void cs4210_spdif_automute(struct hda_codec *codec,
>
> spec->spdif_present = spdif_present;
> /* SPDIF TX on/off */
> - if (spdif_present)
> - snd_hda_set_pin_ctl(codec, spdif_pin,
> - spdif_present ? PIN_OUT : 0);
> + snd_hda_set_pin_ctl(codec, spdif_pin, spdif_present ? PIN_OUT : 0);
>
> cs_automute(codec);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-25 6:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-24 16:12 ALSA: hda - Use generic parser for Cirrus codec driver Dan Carpenter
2015-07-24 16:18 ` Takashi Iwai
2015-07-25 0:03 ` [patch] ALSA: hda - fix cs4210_spdif_automute() Dan Carpenter
2015-07-25 0:03 ` Dan Carpenter
2015-07-25 6:20 ` Takashi Iwai
2015-07-25 6:20 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.