[alsa-devel] [PATCH] wss_lib: do not mess mixer settings during probe
From: Krzysztof Helt krzysztof.h1@wp.pl
Use the wss_dout function which does not mess shadowed register values during chip probing. Otherwise, user ends up with stupid mixer settings after driver loading.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl ---
Especially, the recording settings are messed up like left channel recording from different source than right channel.
diff -urp linux-ref/sound/isa/wss/wss_lib.c linux-2.6/sound/isa/wss/wss_lib.c --- linux-ref/sound/isa/wss/wss_lib.c 2008-08-14 00:05:30.000000000 +0200 +++ linux-2.6/sound/isa/wss/wss_lib.c 2008-08-25 21:13:14.000000000 +0200 @@ -1162,9 +1162,9 @@ static int snd_ad1848_probe(struct snd_w spin_lock_irqsave(&chip->reg_lock, flags);
/* set CS423x MODE 1 */ - snd_wss_out(chip, CS4231_MISC_INFO, 0); + snd_wss_dout(chip, CS4231_MISC_INFO, 0);
- snd_wss_out(chip, CS4231_RIGHT_INPUT, 0x45); /* 0x55 & ~0x10 */ + snd_wss_dout(chip, CS4231_RIGHT_INPUT, 0x45); /* 0x55 & ~0x10 */ r = snd_wss_in(chip, CS4231_RIGHT_INPUT); if (r != 0x45) { /* RMGE always high on AD1847 */ @@ -1174,7 +1174,7 @@ static int snd_ad1848_probe(struct snd_w } hardware = WSS_HW_AD1847; } else { - snd_wss_out(chip, CS4231_LEFT_INPUT, 0xaa); + snd_wss_dout(chip, CS4231_LEFT_INPUT, 0xaa); r = snd_wss_in(chip, CS4231_LEFT_INPUT); /* L/RMGE always low on AT2320 */ if ((r | CS4231_ENABLE_MIC_GAIN) != 0xaa) { @@ -1199,7 +1199,7 @@ static int snd_ad1848_probe(struct snd_w r = snd_wss_in(chip, CS4231_MISC_INFO);
/* set CS423x MODE 2 */ - snd_wss_out(chip, CS4231_MISC_INFO, CS4231_MODE2); + snd_wss_dout(chip, CS4231_MISC_INFO, CS4231_MODE2); for (i = 0; i < 16; i++) { if (snd_wss_in(chip, i) != snd_wss_in(chip, 16 + i)) { /* we have more than 16 registers: check ID */ @@ -1221,7 +1221,7 @@ static int snd_ad1848_probe(struct snd_w else chip->hardware = WSS_HW_AD1848; out_mode: - snd_wss_out(chip, CS4231_MISC_INFO, 0); + snd_wss_dout(chip, CS4231_MISC_INFO, 0); out: spin_unlock_irqrestore(&chip->reg_lock, flags); return err;
---------------------------------------------------------------------- Zobacz galerie - tak wygladaja wampiry!
On 24-08-08 18:08, Krzysztof Helt wrote:
From: Krzysztof Helt krzysztof.h1@wp.pl
Use the wss_dout function which does not mess shadowed register values during chip probing. Otherwise, user ends up with stupid mixer settings after driver loading.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl
Acked-by: Rene Herman rene.herman@gmail.com
Must say that I only now notice that we're doing that init loop there. I believe it could be better to introduce an inline __snd_wss_out() with just the two outb()s and use that from snd_wss_out() and here directly.
And with respect to the mb() ... if anywhere, should't that be between the two outb's really?
Rene.
On Sun, 24 Aug 2008 20:36:56 +0200 Rene Herman rene.herman@keyaccess.nl wrote:
On 24-08-08 18:08, Krzysztof Helt wrote:
From: Krzysztof Helt krzysztof.h1@wp.pl
Use the wss_dout function which does not mess shadowed register values during chip probing. Otherwise, user ends up with stupid mixer settings after driver loading.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl
Acked-by: Rene Herman rene.herman@gmail.com
Must say that I only now notice that we're doing that init loop there.
I noticed it on at least three different cards.
I believe it could be better to introduce an inline __snd_wss_out() with just the two outb()s and use that from snd_wss_out() and here directly.
snd_wss_dout() is almost the same. The waiting loop should never trigger there and it is safer in case some write needs few micro-secs to finish.
And with respect to the mb() ... if anywhere, should't that be between the two outb's really?
_From the Documentation/memory-barriers.txt " (*) inX(), outX(): ... They are guaranteed to be fully ordered with respect to each other.
They are not guaranteed to be fully ordered with respect to other types of memory and I/O operation. ... "
No barriers are needed in this case.
Regards, Krzysztof
---------------------------------------------------------------------- Alergia na seks?
On 24-08-08 21:48, Krzysztof Helt wrote:
From the Documentation/memory-barriers.txt " (*) inX(), outX(): ... They are guaranteed to be fully ordered with respect to each other.
They are not guaranteed to be fully ordered with respect to other types of memory and I/O operation.... "
No barriers are needed in this case.
On x86 certainly not and I would expect these barriers are there for cases/architectures where inb() and outb() are redefined to memory mapped I/O or there would just be no point at all.
In that case, the first outb() writes an index and we'd better make sure that it hits the hardware before we write the value. Hence my suspicion that if at all, the mb() should be in between.
No idea who added those and why...
Rene
On 25-08-08 00:37, Rene Herman wrote:
On 24-08-08 21:48, Krzysztof Helt wrote:
From the Documentation/memory-barriers.txt " (*) inX(), outX(): ... They are guaranteed to be fully ordered with respect to each other.
They are not guaranteed to be fully ordered with respect to othertypes of memory and I/O operation. ... "
No barriers are needed in this case.
On x86 certainly not and I would expect these barriers are there for cases/architectures where inb() and outb() are redefined to memory mapped I/O or there would just be no point at all.
In that case, the first outb() writes an index and we'd better make sure that it hits the hardware before we write the value. Hence my suspicion that if at all, the mb() should be in between.
No idea who added those and why...
Well, thinking about this more, I guess it actually makes some sense.
No need for a wmb() in between because we know that the outbs are fully ordered (supposedly even when redefined) but being a general accessor function we pretend we don't know that the next access is going to be through an inb/outb again -- we are supposedly being careful to allow combining using this accessor with direct MMIO access on other and/or virtualized architectures.
That is... as far as I understand this in the first place. Takashi or Jaroslav, do you perhaps remember barriers being added to cs4231_lib() and why? Could we just get rid of them?
(context was snipped, but this question is about the mb() in snd_cs4231_dout; snd_wss_dout in the new wss_lib form).
Rene.
At Mon, 25 Aug 2008 00:37:52 +0200, Rene Herman wrote:
On 24-08-08 21:48, Krzysztof Helt wrote:
From the Documentation/memory-barriers.txt " (*) inX(), outX(): ... They are guaranteed to be fully ordered with respect to each other.
They are not guaranteed to be fully ordered with respect to other types of memory and I/O operation.... "
No barriers are needed in this case.
On x86 certainly not and I would expect these barriers are there for cases/architectures where inb() and outb() are redefined to memory mapped I/O or there would just be no point at all.
These barriers are mostly for Alpha. Not sure whether they are really needed, though.
Takashi
At Sun, 24 Aug 2008 18:08:03 +0200, Krzysztof Helt wrote:
From: Krzysztof Helt krzysztof.h1@wp.pl
Use the wss_dout function which does not mess shadowed register values during chip probing. Otherwise, user ends up with stupid mixer settings after driver loading.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl
Especially, the recording settings are messed up like left channel recording from different source than right channel.
Thanks, now applied.
Takashi
participants (3)
-
Krzysztof Helt -
Rene Herman -
Takashi Iwai