On Wed, Jun 04, 2008 at 06:50:17PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:
> > +static const u16 wm8510_reg[WM8510_CACHEREGNUM] = {
> > + 0x0000, 0x0000, 0x0000, 0x0000,
> Use tabs here.
> > +static const char *wm8510_companding[] = {"Off", "NC", "u-law", "A-law" };
> > +static const char *wm8510_deemp[] = {"None", "32kHz", "44.1kHz", "48kHz" };
> > +static const char *wm8510_alc[] = {"ALC", "Limiter" };
> No space after '{' but a space before '}'? Oh I'm too picky...
Please fix checkpatch for stuff like this :)
> > + case SNDRV_PCM_RATE_44100:
> > + break;
> Just to be sure -- No 48000 case?
> > +#define WM8510_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 |\
> > + SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 |\
> > + SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000)
>
> ... since I find it here.
Yes (well, it's the same as the 41000 case - nothing to set). I'll add
an empty case for clarity.
> > +#define WM8510_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
> > + SNDRV_PCM_FMTBIT_S24_LE)
> Don't you need SNDRV_PCM_FMTBIT_S32_LE? It's handled in
> wm8510_pcm_hw_params().
Should do, yes.
> Also, is the patch author you or Liam?
We've both worked on the driver; Liam did the original driver and I've
done some fairly invasive changes to it since then, particularly to the
input paths. The Author: is showing as me since that's what git did
when I pulled the driver over into mainline for submission.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
