[alsa-devel] [PATCH] ALSA: Support Media Vision Jazz16 chips

[Rask Ingemann Lambertsen]

On Tue, Mar 27, 2007 at 12:45:41AM +0200, Rene Herman wrote:
> On 03/26/2007 11:23 PM, Rask Ingemann Lambertsen wrote:
> 
> >+	if (enable[dev_num])
> >+		card = snd_card_new(index[dev_num], id[dev_num], 
> >THIS_MODULE, 0);
> >+	else
> >+		card = NULL;
> >+	dev_num ++;
> >+	if (card == NULL)
> >+		return -ENOMEM;
> 
> -ENOMEM is not a very good return value for !enable. -ENODEV would be 
> better. Also please dev_num++ without the space.

   Oops. Well spotted.

> The hardware fundamentally supports just one card per system due to the 
> fixed config port, so you might as well get rid of the dev_num (and the 
> SNDRV_CARDS-wide arrays) it seems?. If you want to keep them that way 
> just so that it's more generic looking... sure.

   Also, if someone, somehow, manages to have two of these cards, I would
find it unforgivable if it didn't just work. According to this press
release
<URL:http://findarticles.com/p/articles/mi_m0NEW/is_1993_July_23/ai_14099965>,
there should be motherboards with Jazz16 chips on them, presumably to be
configured using the PnPBIOS (the specification for which was released in
1994). It might still be possible to have a stand-alone Jazz16 card in
addition to an on-board one.

> >+	if (err < 0) {
> >+		snd_printk(PFX_WARN "No OPL device found, skipping.\n");
> >+	} else {
> 
> Single statement branches don't have braces in the kernel coding style.

   OK. Documentation/CodingStyle should probably say so.

> And hope Adam comments on the model...

   Me too.

-- 
Rask Ingemann Lambertsen