Re: [PATCH 1/2] sis7019: support the SiS 7019 Audio Accelerator

[Takashi Iwai <tiwai@suse.de>]

At Wed, 28 Nov 2007 08:03:35 -0500,
Dave Dillow wrote:
> 
> > > +static struct snd_pcm_hardware sis_playback_hw_info = {
> > > +	.info = (SNDRV_PCM_INFO_MMAP |
> > > +		 SNDRV_PCM_INFO_MMAP_VALID |
> > > +		 SNDRV_PCM_INFO_INTERLEAVED |
> > > +		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
> > > +		 SNDRV_PCM_INFO_SYNC_START |
> > > +		 SNDRV_PCM_INFO_RESUME),
> > 
> > SNDRV_PCM_INFO_SYNC_START seems invalid here.  It's not implemented in
> > the driver.
> 
> Perhaps I'm misunderstanding what SYNC_START is for -- I can start
> multiple playback/capture substreams at once in ->trigger(). If it means
> something else, can you point to a driver that does implement it
> properly so that I can see if can be implemented?

Sorry, looks like I typo'ed when I searched strngs (due to a damn
keyboard scan bug in the recent xorg...)
Please disregard my previous comment.

> > > +static void __sis_map_silence(struct sis7019 *sis)
> > 
> > Any reason to use __ prefix?
> 
> It requires locks to be held when it is called, making it an internal
> helper function. I'd prefer to keep the prefix, but it could go away, or
> be merged into the one call-site.

OK, then it's fine.  But add a proper comment.  Otherwise no one else
would understand the difference.


Thanks,

Takashi