[Alsa-devel] [RFC 2/8] snd-aoa: add aoa core

[Johannes Berg]

On Fri, 2006-06-02 at 15:42 +0200, Takashi Iwai wrote:
> > +static void attach_codec_to_fabric(struct aoa_codec *c)
> 
> Doesn't this need to return an error?
> I'm afraid that module count is unblanced in the error path of
> aoa_corec_[un]register().

Not sure, I'll have to take a look.

> > +		printk("snd-aoa: fabric didn't like codec %s\n", c->name);
> 
> Add KERN_* prefix.

Uh, right.

> > +	rt->implementation_private = 0;
> > +	INIT_WORK(&rt->headphone_notify.work, pmf_handle_notify, &rt->headphone_notify);
> > +	INIT_WORK(&rt->line_in_notify.work, pmf_handle_notify, &rt->line_in_notify);
> > +	INIT_WORK(&rt->line_out_notify.work, pmf_handle_notify, &rt->line_out_notify);
> 
> Too long lines.

Heh, I knew I'd get this at lots of places, I'll work on it.

> > +	pmf_gpio_all_amps_off(rt);
> > +	rt->implementation_private = 0;
> > +	if (rt->headphone_notify.gpio_private)
> > +		pmf_unregister_irq_client(rt->headphone_notify.gpio_private);
> > +	if (rt->line_in_notify.gpio_private)
> > +		pmf_unregister_irq_client(rt->line_in_notify.gpio_private);
> > +	if (rt->line_out_notify.gpio_private)
> > +		pmf_unregister_irq_client(rt->line_out_notify.gpio_private);
> 
> Don't need kfree(gpio_private)?

Indeed, forgot that.

> > +	strlcpy(alsa_card->driver, "AppleOnbdAudio", sizeof(alsa_card->driver)-1);
> > +	strlcpy(alsa_card->shortname, name, sizeof(alsa_card->shortname)-1);
> > +	strlcpy(alsa_card->longname, name, sizeof(alsa_card->longname)-1);
> > +	strlcpy(alsa_card->mixername, name, sizeof(alsa_card->mixername)-1);
> 
> Pass sizeof() without -1.  strlcpy() takes the size of the buffer
> including nul-terminator.

Yeah, I was confused about the API. Will fix those too.

Thanks,
johannes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 793 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20060602/611cd5cf/attachment.pgp>