[alsa-devel] [PATCH 3/3] Add ALSA driver for Atmel AC97 controller

[Hans-Christian Egtvedt]

On Wed, 04 Feb 2009 13:09:37 +0100
Takashi Iwai <tiwai at suse.de> wrote:

> At Wed,  4 Feb 2009 12:48:34 +0100,
> Hans-Christian Egtvedt wrote:

<snipp>

> > diff --git a/include/sound/atmel-ac97c.h
> > b/include/sound/atmel-ac97c.h new file mode 100644
> > index 0000000..d5daddd
> > --- /dev/null
> > +++ b/include/sound/atmel-ac97c.h
> 
> Is it needed to be in include/sound directory?
> If it's just for a local driver, better to put in the local directory.
> 

Yes, the header must be available for the boards so they can see the
platform data struct. This is how the DMA slave and GPIO for reset line
is passed to the AC97C driver.

> > diff --git a/sound/avr32/ac97c.c b/sound/avr32/ac97c.c
> > new file mode 100644
> > index 0000000..218d666
> > --- /dev/null
> > +++ b/sound/avr32/ac97c.c
> > @@ -0,0 +1,994 @@
> ...
> > +static int atmel_ac97c_playback_hw_params(struct snd_pcm_substream
> > *substream,
> > +		struct snd_pcm_hw_params *hw_params)
> > +{
> > +	struct atmel_ac97c *chip =
> > snd_pcm_substream_chip(substream);
> > +	int retval;
> > +
> > +	retval = snd_pcm_lib_malloc_pages(substream,
> > +
> > params_buffer_bytes(hw_params));
> > +	if (retval)
> > +		return retval;
> 
> Should be a negative-check.
> 

OK

> > +static int atmel_ac97c_capture_hw_params(struct snd_pcm_substream
> > *substream,
> > +		struct snd_pcm_hw_params *hw_params)
> > +{
> > +	struct atmel_ac97c *chip =
> > snd_pcm_substream_chip(substream);
> > +	int retval;
> > +
> > +	retval = snd_pcm_lib_malloc_pages(substream,
> > +
> > params_buffer_bytes(hw_params));
> > +	if (retval)
> > +		return retval;
> 
> Ditto.
> 

OK

> > +static int __devinit atmel_ac97c_probe(struct platform_device
> > *pdev)
> ...
> > +	mutex_init(&opened_mutex);
> 
> This is superfluous.
> 
> > +
> > +	retval = -ENOMEM;
> > +	card = snd_card_new(SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
> > +			THIS_MODULE, sizeof(struct atmel_ac97c));
> 
> Use snd_card_create().
> 

OK, I'll convert to using this for both drivers.

-- 
Best regards,
Hans-Christian Egtvedt