On Mon, Jul 07, 2008 at 05:57:06PM +0200, Takashi Iwai wrote:
Thanks for the patch. Below is a quick review.
thank you.
- wmb();
- val = readq(&mace->perif.audio.codec_control); /* flush bus */
- udelay(200);
It'd be nicer if this long delay can be avoided. Or, could it be mutex?
This isn't a fatal problem, so it's OK as is, though.
I'm not quite sure, what delay really is sufficient at the moment. It worked without delay on my slow O2, but caused problems on faster maschines. I want to figure out a good value and if it's just a few microsecons simply use an udelay(). If a longer delay is needed I'll switch over to something more kernel friendly.
- /* allocate IRQs */
- for (i = 0; i < ARRAY_SIZE(snd_sgio2_isr_table); i++) {
if (request_irq(snd_sgio2_isr_table[i].irq,snd_sgio2_isr_table[i].isr,IRQF_SHARED,snd_sgio2_isr_table[i].desc,&chip->channel[snd_sgio2_isr_table[i].idx])) {snd_sgio2audio_free(chip);printk(KERN_ERR "sgio2audio: cannot allocate irq %d\n",snd_sgio2_isr_table[i].irq);return -EBUSY;}- }
If there are shared interrupts, they could be called before the initialization below. So, safer to move after the init.
they can't be shared and it's already done before any hardware init. We only check, whether there is a sound module installed before that.
I'll post an updated patch in a couple of minutes.
Thomas,