[alsa-devel] ALSA driver for Native Instruments sound hardware

[Takashi Iwai]

At Mon, 26 Mar 2007 17:46:37 +0200,
Daniel Mack wrote:
> 
> On Mon, Mar 26, 2007 at 03:45:47PM +0200, Takashi Iwai wrote:
> > > diff -Nur alsa-kernel-ni/usb/caiaq/caiaq-audio.c alsa-kernel/usb/caiaq/caiaq-audio.c
> > > --- alsa-kernel-ni/usb/caiaq/caiaq-audio.c	1970-01-01 01:00:00.000000000 +0100
> > > +++ alsa-kernel/usb/caiaq/caiaq-audio.c	2007-03-23 17:33:59.000000000 +0100
> > > +
> > > +static int snd_usb_caiaq_pcm_trigger(struct snd_pcm_substream *substream, 
> > > +				     int cmd)
> > > +{
> > > +	struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(substream);
> > > +
> > > +	switch (cmd) {
> > > +		case SNDRV_PCM_TRIGGER_START:
> > > +		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > 
> > Put 'case' with the same indent level as switch.
> > (Ditto for all switch blocks in other places.)
> 
> Done.
> 
> > > +	switch(buf[0]) {
> > > +	case EP1_CMD_GET_DEVICE_INFO:
> > > +	{
> > > +	 	memcpy(&dev->spec, buf+1, sizeof(struct caiaq_device_spec));
> > 
> > Don't you need to convert 16bit values for big-endian?
> 
> The only value which uses more than one byte of storage is the firmware
> revision which is unimportant as no runtime decision depends on it.

Then I'd suggest to fix it up, e.g.
	dev->spec.fw_version = le16_to_cpu(dev->spec.fw_version);
to avoid the future bug.

> All others are chars and need no endianess care.
>
> > Also, some lines are too long.  Please try to keep lines in 80 chars
> > as much as possible.
> 
> Also done. New patch applied.

Great.  Could you fix the above and resubmit the patch?
Then I'll apply it to HG tree.


Thanks,

Takashi