At Tue, 23 Jun 2009 15:50:44 -0700, Igor Chernyshev wrote: > > >> + unsigned long pm_saved_route; > > > > This shouldn't be long but int. Long can be 64bit unnecessarily. > > Done. However, note that existing code uses results of > "inl(ICEMT1724(ice, ROUTE_PLAYBACK))" as both int and long. > > >> + case SNDRV_PCM_TRIGGER_SUSPEND: > >> + case SNDRV_PCM_TRIGGER_RESUME: > >> + break; > > > > At least, TRIGGER_SUSPEND neees to stop the PCM stream. > > Consolidated with SNDRV_PCM_TRIGGER_STOP processing. > > >> + outb(VT1724_MULTI_FIFO_ERR, ICEMT1724(ice, DMA_INT_MASK)); > >> + > > > > This is fine, but you need to remove __devinit from this function. > > Done. > > >> + if (ice->ac97) > >> + snd_ac97_suspend(ice->ac97); > > > > No need for NULL check here. All these check NULL by themselves. > > Removed all 4 checks (pcm's and ac97). > > >> + spin_unlock_irq(&ice->reg_lock); > >> + snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 1); > >> + spin_lock_irq(&ice->reg_lock); > >> + } > > > > 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-snapshot.tar.gz thanks, Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
