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

[Takashi Iwai]

At Wed,  4 Feb 2009 12:48:34 +0100,
Hans-Christian Egtvedt wrote:
> 
> This patch adds ALSA support for the AC97 controller found on Atmel
> AVR32 devices.
> 
> Tested on ATSTK1006 + ATSTK1000 with a development board with a AC97
> codec.
> 
> Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt at atmel.com>
> ---
>  include/sound/atmel-ac97c.h |   40 ++
>  sound/avr32/Kconfig         |    8 +
>  sound/avr32/Makefile        |    2 +
>  sound/avr32/ac97c.c         |  994 +++++++++++++++++++++++++++++++++++++++++++
>  sound/avr32/ac97c.h         |   71 +++
>  5 files changed, 1115 insertions(+), 0 deletions(-)
>  create mode 100644 include/sound/atmel-ac97c.h
>  create mode 100644 sound/avr32/ac97c.c
>  create mode 100644 sound/avr32/ac97c.h
> 
> 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.

> 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.

> +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.

> +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().


thanks,

Takashi