[alsa-devel] [PATCH 2/2] Add support for Cyrix/NatSemi Geode SC5530 (VSA1)

[Ash Willis]

> Avoid the style "if ((err = xx) < 0)" in new codes as much as
> possible.  Just write:
> 	err = pci_enable(pci);
> 	if (err < 0)
> 		return err;
> Now the kernel coding style recommends this.


Sorry, I wasn't aware this had changed :)


> > +	if (dma8 & 0x20)
> > +		dma16 = 5;
> > +	else if (dma8 & 0x40)
> > +		dma16 = 6;
> > +	else if (dma8 & 0x80)
> > +		dma16 = 7;
> > +	else
> > +	{
> > +		printk(KERN_ERR "CS5530: No 16bit DMA enabled\n");
> > +		snd_cs5530_free(chip);
> > +		return -ENODEV;
> > +	}
> 
> Ditto.
> (BTW, is dma16 really necessary to be zero-initialized at beginning?)
> 

It is, I think. It supresses a compiler warning, due the to fact that
the final else path does not set dma16 and so if this path is taken the
variable is uninitialied...not that we care, but we certainly don't
need a compiler warning.

> 
> Is the driver confirmed to work with the recent kernel?
> If yes, we can put cs5530.c to alsa-kernel so that it'll be merged to
> the next linux kernel.  alsa-driver tree still needs cs5530.c just
> including alsa-kernel code, though.
> Also, don't forget to add a description to ALSA-Configuration.txt.
> 
> Could you fix these issues and resend the patch?

It was devleoped on 2.6.21.1. OK, I'll make all the necessary changes.

Thanks,

Ash


=