* Who maintains drivers/sound/i810_audio.c? @ 2003-09-03 12:22 Mehmet Ceyran 2003-09-03 12:29 ` Marc-Christian Petersen 2003-09-03 15:00 ` Who maintains drivers/sound/i810_audio.c? Jeff Garzik 0 siblings, 2 replies; 9+ messages in thread From: Mehmet Ceyran @ 2003-09-03 12:22 UTC (permalink / raw) To: linux-kernel Hello, I found and fixed a little bug in the "Intel ICH (i8xx), SiS 7012, NVidia nForce Audio or AMD 768/811x" driver (kernel 2.4.23-pre2) that occured on my laptop with SiS 7012 onBoard sound and wanted to contribute it to the official kernel sources. In the maintainers file that came with the kernel I couldn't find the maintainer of that particular driver so I'd appreciate if someone lead me to the correct mailing list so I can post the bug and my patch to the right place. I wrote to linux-sound@vger.kernel.org yesterday but got no answer yet. Best regards, Mehmet Ceyran ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Who maintains drivers/sound/i810_audio.c? 2003-09-03 12:22 Who maintains drivers/sound/i810_audio.c? Mehmet Ceyran @ 2003-09-03 12:29 ` Marc-Christian Petersen 2003-09-03 23:31 ` drivers/sound/i810_audio.c bug and patch Mehmet Ceyran 2003-09-03 15:00 ` Who maintains drivers/sound/i810_audio.c? Jeff Garzik 1 sibling, 1 reply; 9+ messages in thread From: Marc-Christian Petersen @ 2003-09-03 12:29 UTC (permalink / raw) To: Mehmet Ceyran, linux-kernel On Wednesday 03 September 2003 14:22, Mehmet Ceyran wrote: Hi Mehmet, > I found and fixed a little bug in the "Intel ICH (i8xx), SiS 7012, > NVidia nForce Audio or AMD 768/811x" driver (kernel 2.4.23-pre2) that > occured on my laptop with SiS 7012 onBoard sound and wanted to > contribute it to the official kernel sources. > In the maintainers file that came with the kernel I couldn't find the > maintainer of that particular driver so I'd appreciate if someone lead > me to the correct mailing list so I can post the bug and my patch to the > right place. It seems Alan is maintaining i8xx audio, but Alan is away for 1 year now. Anyway, you could send him your patch CC'ing LKML. ciao, Marc ^ permalink raw reply [flat|nested] 9+ messages in thread
* drivers/sound/i810_audio.c bug and patch 2003-09-03 12:29 ` Marc-Christian Petersen @ 2003-09-03 23:31 ` Mehmet Ceyran 2003-09-04 1:43 ` Mike Fedyk 0 siblings, 1 reply; 9+ messages in thread From: Mehmet Ceyran @ 2003-09-03 23:31 UTC (permalink / raw) To: linux-kernel; +Cc: alan [-- 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); ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: drivers/sound/i810_audio.c bug and patch 2003-09-03 23:31 ` drivers/sound/i810_audio.c bug and patch Mehmet Ceyran @ 2003-09-04 1:43 ` Mike Fedyk 2003-09-04 2:55 ` AW: " Mehmet Ceyran ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Mike Fedyk @ 2003-09-04 1:43 UTC (permalink / raw) To: Mehmet Ceyran; +Cc: linux-kernel, alan On Thu, Sep 04, 2003 at 01:31:40AM +0200, Mehmet Ceyran wrote: > 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: Why busy wait especially when you can sleep 1ms each time and poll less? ^ permalink raw reply [flat|nested] 9+ messages in thread
* AW: drivers/sound/i810_audio.c bug and patch 2003-09-04 1:43 ` Mike Fedyk @ 2003-09-04 2:55 ` Mehmet Ceyran 2003-09-04 11:01 ` Alan Cox 2003-09-04 11:01 ` Alan Cox 2 siblings, 0 replies; 9+ messages in thread From: Mehmet Ceyran @ 2003-09-04 2:55 UTC (permalink / raw) To: 'Mike Fedyk'; +Cc: linux-kernel, alan Hi Mike, >> 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: > Why busy wait especially when you can sleep 1ms each time and poll less? Well, I can tell you why I chose "busy wait" (kind of brainstorming): - Why did the originator of the driver made the loop with 10 turns, which is "busy wait" in your definition too? - Imagine you have workers and you give them a job that's supposed to be finished in six minutes. But these workers actually need seven minutes, which you don't know. If that job is urgent, would you rather look if it's done yet every 60 minutes or every six minutes? In the first case (that's your model), you would get your positive result in one hour, in the second case (my model), you'd get it in 12 minutes. - Seeing the constant of HZ/20 in "schedule_timeout(HZ/20);" gives me the feeling that this value is taken from some specification. Perhaps it's from a i810 datasheet (I will look at that later because it's very late now), but my fix is for SiS 7012 which could have slightly different characteristics. In this case, by doing my "busy wait", you don't modify the existing code which will initialize the chip in less than or equal to 10 turns. - When the driver is loaded at boot time I think it's the only piece of code which is running at that moment, I don't think there are any other threads working during driver init. If it's loaded as a module, the one who loads it waits for its results anyway. It's almost impossible that doing this "busy wait" will affect a running system with many concurrently running processes at a high system load _noticeably_. It will instead come to a result faster if the hardware is available (see point 1). Best Regards, Mehmet ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: drivers/sound/i810_audio.c bug and patch 2003-09-04 1:43 ` Mike Fedyk 2003-09-04 2:55 ` AW: " Mehmet Ceyran @ 2003-09-04 11:01 ` Alan Cox 2003-09-04 17:53 ` Mike Fedyk 2003-09-04 11:01 ` Alan Cox 2 siblings, 1 reply; 9+ messages in thread From: Alan Cox @ 2003-09-04 11:01 UTC (permalink / raw) To: Mike Fedyk; +Cc: Mehmet Ceyran, Linux Kernel Mailing List On Iau, 2003-09-04 at 02:43, Mike Fedyk wrote: > > set_current_state(TASK_UNINTERRUPTIBLE); > > schedule_timeout(HZ/20); > Why busy wait especially when you can sleep 1ms each time and poll less? I think you read it wrong - its sleeping... ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: drivers/sound/i810_audio.c bug and patch 2003-09-04 11:01 ` Alan Cox @ 2003-09-04 17:53 ` Mike Fedyk 0 siblings, 0 replies; 9+ messages in thread From: Mike Fedyk @ 2003-09-04 17:53 UTC (permalink / raw) To: Alan Cox; +Cc: Mehmet Ceyran, Linux Kernel Mailing List On Thu, Sep 04, 2003 at 12:01:43PM +0100, Alan Cox wrote: > On Iau, 2003-09-04 at 02:43, Mike Fedyk wrote: > > > set_current_state(TASK_UNINTERRUPTIBLE); > > > schedule_timeout(HZ/20); > > > Why busy wait especially when you can sleep 1ms each time and poll less? > > I think you read it wrong - its sleeping... Yep, you're right. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: drivers/sound/i810_audio.c bug and patch 2003-09-04 1:43 ` Mike Fedyk 2003-09-04 2:55 ` AW: " Mehmet Ceyran 2003-09-04 11:01 ` Alan Cox @ 2003-09-04 11:01 ` Alan Cox 2 siblings, 0 replies; 9+ messages in thread From: Alan Cox @ 2003-09-04 11:01 UTC (permalink / raw) To: Mike Fedyk; +Cc: Mehmet Ceyran, Linux Kernel Mailing List On Iau, 2003-09-04 at 02:43, Mike Fedyk wrote: > > set_current_state(TASK_UNINTERRUPTIBLE); > > schedule_timeout(HZ/20); > Why busy wait especially when you can sleep 1ms each time and poll less? I think you read it wrong - its sleeping... ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Who maintains drivers/sound/i810_audio.c? 2003-09-03 12:22 Who maintains drivers/sound/i810_audio.c? Mehmet Ceyran 2003-09-03 12:29 ` Marc-Christian Petersen @ 2003-09-03 15:00 ` Jeff Garzik 1 sibling, 0 replies; 9+ messages in thread From: Jeff Garzik @ 2003-09-03 15:00 UTC (permalink / raw) To: Mehmet Ceyran; +Cc: linux-kernel Mehmet Ceyran wrote: > Hello, > > I found and fixed a little bug in the "Intel ICH (i8xx), SiS 7012, > NVidia nForce Audio or AMD 768/811x" driver (kernel 2.4.23-pre2) that > occured on my laptop with SiS 7012 onBoard sound and wanted to > contribute it to the official kernel sources. > > In the maintainers file that came with the kernel I couldn't find the > maintainer of that particular driver so I'd appreciate if someone lead > me to the correct mailing list so I can post the bug and my patch to the > right place. Please post the patch here, to linux-kernel, and CC Alan Cox <alan@lxorguk.ukuu.org.uk> Jeff ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2003-09-04 17:55 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2003-09-03 12:22 Who maintains drivers/sound/i810_audio.c? Mehmet Ceyran 2003-09-03 12:29 ` Marc-Christian Petersen 2003-09-03 23:31 ` drivers/sound/i810_audio.c bug and patch Mehmet Ceyran 2003-09-04 1:43 ` Mike Fedyk 2003-09-04 2:55 ` AW: " Mehmet Ceyran 2003-09-04 11:01 ` Alan Cox 2003-09-04 17:53 ` Mike Fedyk 2003-09-04 11:01 ` Alan Cox 2003-09-03 15:00 ` Who maintains drivers/sound/i810_audio.c? Jeff Garzik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).