[alsa-devel] [PATCH] ALSA driver for SGI O2 audio board

[Thomas Bogendoerfer]

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,

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]