[alsa-devel] [PATCH] General fix for Palm27x aSoC driver

[Marek Vasut]

On Wednesday 15 of April 2009 04:23:49 Eric Miao wrote:
> On Wed, Apr 15, 2009 at 5:42 AM, Marek Vasut <marek.vasut at gmail.com> wrote:
> > On Tuesday 14 of April 2009 21:50:36 Mark Brown wrote:
> >> On Sun, Apr 12, 2009 at 08:51:19PM +0200, Marek Vasut wrote:
> >> > +static struct pxa2xx_ac97_platform_data palmld_ac97_pdata = {
> >> > +   .reset_gpio     = 95,
> >> > +};
> >>
> >> The type of this will need changing to reflect the patch that got merged
> >> for this but other than that minor point this approach is fine.
> >
> > OK, shall I change it and resend (ps. to what if you dont mind telling
> > me?) ? Also, do you want to push it through also tree or ARM tree ? I'm
> > for the second option as it's more of a bugfix suitable for that tree.
>
> Sorry, late on this. The changes to the platform part look OK to me,
>
> and some minor things you may have another look:
> > -static int __init palm27x_asoc_init(void)
> > +static int palm27x_asoc_probe(struct platform_device *pdev)
>
> __devinit

Thanks, true, will revise it later today and resend ...
>
> >  {
> >  	int ret;
> >
> > @@ -208,6 +208,10 @@ static int __init palm27x_asoc_init(void)
> >  		machine_is_palmld()))
> >  		return -ENODEV;
> >
> > +	if (pdev->dev.platform_data)
> > +		palm27x_ep_gpio = ((struct palm27x_asoc_info *)
> > +			(pdev->dev.platform_data))->jack_gpio;
> > +
>
> This is not so readable, I'd prefer to introduce a variable for the
> 'struct palm27x_asoc_info *' pointer.

Come on, we are not doing the kernel only for ub...u so this should be OK for 
everyone who can code in C. :-)
>
> >  	ret = gpio_request(palm27x_ep_gpio, "Headphone Jack");
> >  	if (ret)
> >  		return ret;