drivers/sound/i810_audio.c bug and patch

["Mehmet Ceyran" <mceyran@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2452 bytes --]

Thanks to Jeff and Marc for your answers.

On my laptop (P4-2533, SiS 7012 onBoard sound) the i810 driver didn't
work in its shipped 2.4.23-pre2 and 2.4.22 (and probably below) form.
The kernel messages related to this were as follows:

---8<---
Intel 810 + AC97 Audio, version 0.24, 19:25:34 Aug 31 2003
PCI: Enabling device 00:02.7 (0000 -> 0001)
i810: SiS 7012 found at IO 0x1800 and 0x1c00, MEM 0x0000 and 0x0000, IRQ
10
i810_audio: Audio Controller supports 6 channels.
i810_audio: Defaulting to base 2 channel mode.
i810_audio: Resetting connection 0
ac97_codec: AC97 Audio codec, id: CRY52 (Cirrus Logic CS4299 rev D)
i810_audio: timed out waiting for codec 0 analog ready.
--->8---

After hesitating for a while (I code in C/C++ and many more languages
but I've never been to kernel and drivers, YET ;) ) I decided to look at
the sources and fix the bug if I could.

I found out that the driver gives the sound chip 10 chances to become
ready and my sound chip fails to do that in time. The following patch
gives the chip 100 tries instead of 10:

---8<---
--- i810_audio.c	2003-09-02 13:58:02.000000000 +0200
+++ i810_audio.c.new	2003-09-02 13:58:12.000000000 +0200
@@ -2727,7 +2727,7 @@
 		      i810_ac97_get(codec, AC97_POWER_CONTROL) &
~0x7f00);
 
 	/* wait for analog ready */
-	for (i=10; i && ((i810_ac97_get(codec, AC97_POWER_CONTROL) &
0xf) != 0xf); i--)
+	for (i=100; i && ((i810_ac97_get(codec, AC97_POWER_CONTROL) &
0xf) != 0xf); i--)
 	{
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout(HZ/20);
--->8---

Well, why not? The loop will only go through it's body 100 times if the
hardware is actually not available or corrupt and even in this case the
whole block won't take much time. It works for me and it should work for
all the other people using this driver too:

---8<---
i810: SiS 7012 found at IO 0x1800 and 0x1c00, MEM 0x0000 and 0x0000, IRQ
10
i810_audio: Audio Controller supports 6 channels.
i810_audio: Defaulting to base 2 channel mode.
i810_audio: Resetting connection 0
ac97_codec: AC97 Audio codec, id: CRY52 (Cirrus Logic CS4299 rev D)
i810_audio: AC'97 codec 0 supports AMAP, total channels = 2
--->8---

Umm, the maintainers list says I should try to include credit lines.
Well yeah, I'd like to have my name and email address in the changelog
if this gets through to the kernel ;). And I'm really interested in
contributing more than just this to the kernel.

In hope for feedback,
Mehmet Ceyran

[-- Attachment #2: i810_audio.patch --]
[-- Type: application/octet-stream, Size: 478 bytes --]

--- i810_audio.c	2003-09-02 13:58:02.000000000 +0200
+++ i810_audio.c.new	2003-09-02 13:58:12.000000000 +0200
@@ -2727,7 +2727,7 @@
 		      i810_ac97_get(codec, AC97_POWER_CONTROL) & ~0x7f00);
 
 	/* wait for analog ready */
-	for (i=10; i && ((i810_ac97_get(codec, AC97_POWER_CONTROL) & 0xf) != 0xf); i--)
+	for (i=100; i && ((i810_ac97_get(codec, AC97_POWER_CONTROL) & 0xf) != 0xf); i--)
 	{
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout(HZ/20);