From mboxrd@z Thu Jan  1 00:00:00 1970
From: Takashi Iwai <tiwai@suse.de>
Subject: Re: [PATCH1/5] ASoC S3C24xx machine drivers -
	s3c2443-AC97
Date: Thu, 10 May 2007 17:25:28 +0200
Message-ID: <s5h3b24k8ev.wl%tiwai@suse.de>
References: <1178802013.10518.69.camel@a10072.wolfsonmicro.main>
Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <alsa-devel-bounces@alsa-project.org>
Received: from mx1.suse.de (ns.suse.de [195.135.220.2])
	by alsa0.perex.cz (Postfix) with ESMTP id D394D2439F
	for <alsa-devel@alsa-project.org>;
	Thu, 10 May 2007 17:25:28 +0200 (CEST)
In-Reply-To: <1178802013.10518.69.camel@a10072.wolfsonmicro.main>
List-Unsubscribe: <http://mailman.alsa-project.org/mailman/listinfo/alsa-devel>,
	<mailto:alsa-devel-request@alsa-project.org?subject=unsubscribe>
List-Archive: <http://mailman.alsa-project.org/pipermail/alsa-devel>
List-Post: <mailto:alsa-devel@alsa-project.org>
List-Help: <mailto:alsa-devel-request@alsa-project.org?subject=help>
List-Subscribe: <http://mailman.alsa-project.org/mailman/listinfo/alsa-devel>,
	<mailto:alsa-devel-request@alsa-project.org?subject=subscribe>
Sender: alsa-devel-bounces@alsa-project.org
Errors-To: alsa-devel-bounces@alsa-project.org
To: Liam Girdwood <lg@opensource.wolfsonmicro.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>, Graeme Gregory <gg@opensource.wolfsonmicro.com>
List-Id: alsa-devel@alsa-project.org

At Thu, 10 May 2007 14:00:13 +0100,
Liam Girdwood wrote:
> 
> Sorry for the lateness in the current merge window of this patch
> series.  

Well, I guess the patches will be unlikely merge to 2.6.22 at this
moment...  Anyway, I'll try to merge into ALSA tree.


> From: Graeme Gregory <gg@opensource.wolfsonmicro.com>
> 
> This patch adds AC97 support to the Samsung S3C2443 CPU. 
> 
> Signed-off-by: Graeme Gregory <gg@opensource.wolfsonmicro.com>
> Signed-off-by: Liam Girdwood <lg@opensource.wolfsonmicro.com>
> [2 s3c2443-ac97.patch <text/x-patch; UTF-8 (7bit)>]
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/soc/s3c24xx/s3c2443-ac97.c	Wed May 09 13:54:02 2007 +0100
> @@ -0,0 +1,405 @@
(snip)
> +static struct completion ac97_completion;

Use DECLARE_COMPLETION() rather than calling init_completion() at each
time.  Since this is accessed in the irq handler, it must be
initialized properly before anything happens.

> +	if (addr != reg)
> +		printk(KERN_ERR "s3c24xx-ac97: req addr = %02x, rep addr = %02x\n", reg, addr);

Try to keep within 80 chars.

> +static void s3c2443_ac97_warm_reset(struct snd_ac97 *ac97)
> +{
> +	u32 ac_glbctrl;
> +
> +	ac_glbctrl = readl(s3c24xx_ac97.regs + S3C_AC97_GLBCTRL);
> +	ac_glbctrl = S3C_AC97_GLBCTRL_WARMRESET;
> +	writel(ac_glbctrl, s3c24xx_ac97.regs + S3C_AC97_GLBCTRL);
> +	udelay(1000);

msleep() can be better here.

> +	ac_glbctrl = 0;
> +	writel(ac_glbctrl, s3c24xx_ac97.regs + S3C_AC97_GLBCTRL);
> +	udelay(1000);

Ditto (also in s3c2443_ac97_cold_reset and s3c2443_ac97_probe()).

> +static irqreturn_t s3c2443_ac97_irq(int irq, void *dev_id)
> +{
> +	int status;
> +	u32 ac_glbctrl;
> +
> +	status = readl(s3c24xx_ac97.regs + S3C_AC97_GLBSTAT) & codec_ready;
> +
> +	if (status) {
> +	    ac_glbctrl = readl(s3c24xx_ac97.regs + S3C_AC97_GLBCTRL);

Keep the indentation correctly.


thanks,

Takashi