On Friday 09 October 2009 21:15:38 ext Mark Brown wrote:
> On Fri, Oct 09, 2009 at 03:55:41PM +0300, Peter Ujfalusi wrote:
> > +struct i2c_client *tpa6130a2_client;
>
> This should be static.
Yes, it should
>
> > +void tpa6130a2_power(int power)
> > +{
>
> This should either be static or have an EXPORT_SYMBOL_GPL() - I'd expect
> the former since this is being managed via DAPM.
Will be static, since it should be handled by the tpa6130a2 driver internally.
>
> > + pdata = (struct tpa6130a2_platform_data *)client->dev.platform_data;
>
> You should never need to cast away from void, and doing so can mask
> actual errors.
Yes.
>
> > +fail:
> > + kfree(data);
> > + i2c_set_clientdata(tpa6130a2_client, NULL);
> > + tpa6130a2_client = 0;
>
> The kernel coding style is to use an explict NULL.
Correct.
>
> Other than that everything looks good so I've applied this with the
> fixes I noted above. If you could send a followup patch for the
> tpa6130a2_power export/static thing that'd be good.
I'll send a patch to make the tpa6130a2_power static.
>
> Thanks!
>
Thanks,
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
