[alsa-devel] [PATCH - try2] ASoC: TPA6130A2 amplifier driver

[Peter Ujfalusi]

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