Re: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

Re: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

[Mark Brown]

On Mon, May 12, 2008 at 06:45:12PM +0800, Bryan Wu wrote:
> From: Cliff Cai <cliff.cai@analog.com>
> 
> Signed-off-by: Cliff Cai <cliff.cai@analog.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>

Thanks, I've applied this to the ASoC git tree.  CCing in
alsa-devel@alsa-project.org - ALSA patches should go via there.

> +static int ad1980_soc_suspend(struct platform_device *pdev,
> +	pm_message_t state)
> +{
> +	return 0;
> +}
> +
> +static int ad1980_soc_resume(struct platform_device *pdev)
> +{
> +	return 0;
> +}

Are you sure about these?  I would expect the suspend and resume
functions to either do some register writes or be omitted if they don't
do anything.  Standard AC97 codecs would have some power management via
register 0x26 if they were doing anything.

Re: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

[Mark Brown]

On Mon, May 12, 2008 at 12:54:16PM +0100, Mark Brown wrote:

> Thanks, I've applied this to the ASoC git tree.  CCing in

...actually, it's already there so I've not applied it - sorry for the
noise.

RE: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

[Cai, Cliff]

 ok,we will implement these two functions later.

Best Regards
Cliff Cai

-----Original Message-----
From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com] 
Sent: Monday, May 12, 2008 7:54 PM
To: Bryan Wu
Cc: liam.girdwood@wolfsonmicro.com; linux-kernel@vger.kernel.org; Cliff
Cai; alsa-devel@alsa-project.org
Subject: Re: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

On Mon, May 12, 2008 at 06:45:12PM +0800, Bryan Wu wrote:
> From: Cliff Cai <cliff.cai@analog.com>
> 
> Signed-off-by: Cliff Cai <cliff.cai@analog.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>

Thanks, I've applied this to the ASoC git tree.  CCing in
alsa-devel@alsa-project.org - ALSA patches should go via there.

> +static int ad1980_soc_suspend(struct platform_device *pdev,
> +	pm_message_t state)
> +{
> +	return 0;
> +}
> +
> +static int ad1980_soc_resume(struct platform_device *pdev) {
> +	return 0;
> +}

Are you sure about these?  I would expect the suspend and resume
functions to either do some register writes or be omitted if they don't
do anything.  Standard AC97 codecs would have some power management via
register 0x26 if they were doing anything.

Re: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

[Mark Brown]

On Tue, May 13, 2008 at 11:00:58AM +0800, Cai, Cliff wrote:

>  ok,we will implement these two functions later.

So they can be removed for now?

What's the current status of merging the Blackfin ASoC support?  We've
had patches in the ASoC git tree for some time (along with the AD1980
driver) - it'd be good to get everything merged into ALSA.

Re: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

[Bryan Wu]

On Tue, May 13, 2008 at 10:01 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, May 13, 2008 at 11:00:58AM +0800, Cai, Cliff wrote:
>
>  >  ok,we will implement these two functions later.
>
>  So they can be removed for now?
>
>  What's the current status of merging the Blackfin ASoC support?

We plan to cleanup the Blackfin ASoC code, after that we will send out
the code for merging.

> We've had patches in the ASoC git tree for some time (along with the AD1980
>  driver) - it'd be good to get everything merged into ALSA.
>

Do you mean there is another version AD1980 in ASoC git tree?

Thanks
-Bryan

Re: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

[Mark Brown]

On Tue, May 13, 2008 at 11:07:16PM +0800, Bryan Wu wrote:
> On Tue, May 13, 2008 at 10:01 PM, Mark Brown

> > We've had patches in the ASoC git tree for some time (along with the AD1980
> >  driver) - it'd be good to get everything merged into ALSA.

> Do you mean there is another version AD1980 in ASoC git tree?

We're carrying both AD1980 and Blackfin platform code.  The AD1980
driver is currently identical to the one you just sent.

Everything is in the dev branch of:

	git://opensource.wolfsonmicro.com/linux-2.6-asoc

Re: [PATCH 1/1] [ASOC]: AD1980 audio codec ASOC driver

[Takashi Iwai]

At Mon, 12 May 2008 18:45:12 +0800,
Bryan Wu wrote:
> diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
(snip)
> +static int ad1980_add_controls(struct snd_soc_codec *codec)
> +{
> +	int err, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ad1980_snd_ac97_controls); i++) {
> +		err = snd_ctl_add(codec->card, snd_soc_cnew( \

The backslash isn't needed.

> +static unsigned int ac97_read(struct snd_soc_codec *codec,
> +	unsigned int reg)
> +{
> +	u16 *cache = codec->reg_cache;
> +
> +	if (reg == AC97_RESET || reg == AC97_INT_PAGING || \
> +			reg == AC97_POWERDOWN || reg == AC97_EXTENDED_STATUS  \
> +			|| reg == AC97_VENDOR_ID1 || reg == AC97_VENDOR_ID2)

Ditto.  Maybe a switch is a better choice here.

> +		return soc_ac97_ops.read(codec->ac97, reg);
> +	else {
> +		reg = reg >> 1;
> +
> +		if (reg > (ARRAY_SIZE(ad1980_reg)))

Isn't it reg >= ARRAY_SIZE(ad1980_reg) ??

> +static int ac97_write(struct snd_soc_codec *codec, unsigned int reg,
> +	unsigned int val)
> +{
> +	u16 *cache = codec->reg_cache;
> +
> +	soc_ac97_ops.write(codec->ac97, reg, val);
> +	reg = reg >> 1;
> +	if (reg <= (ARRAY_SIZE(ad1980_reg)))

And reg < ARRAY_SIZE(ad1980_reg)


thanks,

Takashi