From mboxrd@z Thu Jan  1 00:00:00 1970
From: Takashi Iwai <tiwai@suse.de>
Subject: Re: Patch for suspend/resume in ICE1724 for
	Audiotrak	Prodigy HD2
Date: Wed, 24 Jun 2009 12:13:01 +0200
Message-ID: <s5hljni9byq.wl%tiwai@suse.de>
References: <75570e410906192337p1e9aea9cn9edbea987f6ffbb4@mail.gmail.com>
	<s5h3a9scja2.wl%tiwai@suse.de>
	<75570e410906221246l3125dega728835cb407ba4@mail.gmail.com>
	<s5hprcvb4nz.wl%tiwai@suse.de>
	<75570e410906231550g47bffd92gfaa24d988bbcdca2@mail.gmail.com>
Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <alsa-devel-bounces@alsa-project.org>
Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15])
	by alsa0.perex.cz (Postfix) with ESMTP id 38A56103832
	for <alsa-devel@alsa-project.org>;
	Wed, 24 Jun 2009 12:13:02 +0200 (CEST)
In-Reply-To: <75570e410906231550g47bffd92gfaa24d988bbcdca2@mail.gmail.com>
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: Igor Chernyshev <igor.ch75+alsa@gmail.com>
Cc: alsa-devel@alsa-project.org
List-Id: alsa-devel@alsa-project.org

At Tue, 23 Jun 2009 15:50:44 -0700,
Igor Chernyshev wrote:
> =

> >> + =A0 =A0 unsigned long pm_saved_route;
> >
> > This shouldn't be long but int. =A0Long can be 64bit unnecessarily.
> =

> Done. However, note that existing code uses results of
> "inl(ICEMT1724(ice, ROUTE_PLAYBACK))" as both int and long.
> =

> >> + =A0 =A0 case SNDRV_PCM_TRIGGER_SUSPEND:
> >> + =A0 =A0 case SNDRV_PCM_TRIGGER_RESUME:
> >> + =A0 =A0 =A0 =A0 =A0 =A0 break;
> >
> > At least, TRIGGER_SUSPEND neees to stop the PCM stream.
> =

> Consolidated with SNDRV_PCM_TRIGGER_STOP processing.
> =

> >> + =A0 =A0 outb(VT1724_MULTI_FIFO_ERR, ICEMT1724(ice, DMA_INT_MASK));
> >> +
> >
> > This is fine, but you need to remove __devinit from this function.
> =

> Done.
> =

> >> + =A0 =A0 if (ice->ac97)
> >> + =A0 =A0 =A0 =A0 =A0 =A0 snd_ac97_suspend(ice->ac97);
> >
> > No need for NULL check here. =A0All these check NULL by themselves.
> =

> Removed all 4 checks (pcm's and ac97).
> =

> >> + =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irq(&ice->reg_lock);
> >> + =A0 =A0 =A0 =A0 =A0 =A0 snd_vt1724_set_pro_rate(ice, ice->pro_rate_d=
efault, 1);
> >> + =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irq(&ice->reg_lock);
> >> + =A0 =A0 }
> >
> > These are basically no need to protect with spinlock, at least,
> > in this function...
> =

> Removed locking in "resume" only.
> =

> > Last but not least, try to run $LINUX/scripts/checkpatch.pl to your
> > patch once and fix the issues suggested there.
> =

> Thanks for the pointer. Fixed 3 code style problems. It also complains
> about missing "signed off" line, but I hope that line here will
> suffice:
> =

> Signed-off-by: Igor Chernyshev <igor.ch75+alsa at gmail.com>

Thanks, this looks better.  But, I got some rejects when applying
to the latest tree, and I guess your patch is based on 1.0.20 release.

Please try to rebase to the latest alsa-driver snapshot and repost
the patch, either against sound git tree or alsa-driver-snapshot
tarball below:
  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git
  ftp://ftp.kernel.org/pub/linux/kernel/people/tiwai/snapshot/alsa-driver-s=
napshot.tar.gz


thanks,

Takashi