[alsa-devel] [PATCH] cs4236: detect chip in one pass

[Krzysztof Helt]

On Mon, 02 Nov 2009 11:20:26 +0100
Takashi Iwai <tiwai at suse.de> wrote:

> At Sun, 1 Nov 2009 16:50:38 +0100,
> Krzysztof Helt wrote:
> > 
> > From: Krzysztof Helt <krzysztof.h1 at wp.pl>
> > 
> > The cs4236 was two step detection with call to the snd_wss_free()
> > between two steps. The snd_wss_free() did not free a sound device
> > created in the snd_wss_create(). This caused an OOPS during module
> > removal as the same sound device was released twice. The same OOPS
> > happened if the cs4236 module loading failed.
> > 
> > Fix this by adapting the snd_cs4236_create() to correctly work with
> > chips less capable then cs4236. The snd_cs4236_create() behaves the
> > same as the snd_wss_create() if the chip is less capable than the cs4236.
> > 
> > Signed-off-by: Krzysztof Helt <krzysztof.h1 at wp.pl>
> > ---
> > This is the second version of the patch with added snd_device_free() call
> > in error path.
> 
> Thanks.  The change looks good, but...
> 
> > @@ -281,83 +285,89 @@ int snd_cs4236_create(struct snd_card *card,
> ...
> >  	err = snd_wss_create(card, port, cport,
> >  			     irq, dma1, dma2, hardware, hwshare, &chip);
> >  	if (err < 0)
> >  		return err;
> >  
> > -	if (!(chip->hardware & WSS_HW_CS4236B_MASK)) {
> > -		snd_printk(KERN_ERR "CS4236+: MODE3 and extended registers "
> > -			   "not available, hardware=0x%x\n", chip->hardware);
> > -		snd_device_free(card, chip);
> > -		return -ENODEV;
> 
> I'd just return 0 here instead of error (with *rchip = chip, of course).
> Then the code flow gets simpler and the patch becomes smaller.
> 

These lines now inside if (CS4236B) clause. If one removes this condition, 
the CS4236B cannot be fully initialized and the CS4236 controls do not work 
(some of them require control port access).

If a driver does not want to use full capabilities of the CS4236B due to lack of 
the control port, it should force compatible codec type which does not require
the control port (e.g. CS4232).

Regards,
Krzysztof

Tanie rozmowy telefoniczne!
Sprawdz >> http://link.interia.pl/f2410