Re: [RFC PATCH 03/16] ASoC: Intel: sof-pcm512x: use gpiod for LED

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Apr 14, 2020 at 12:57:35PM -0500, Pierre-Louis Bossart wrote:

...

> > > +		GPIO_LOOKUP("pcm512x-gpio", 3, "PCM512x-GPIO4", GPIO_ACTIVE_HIGH),
> > 
> > It says GPIO 4 and here is number 3.
> > Does this 4 come from hardware documentation?
> 
> Yes TI count from 1 to 6 in their documentation. The initial HifiBerry DAC+
> also counts from 1 to 6. I can add a comment here.

Okay!

...

> > > +	ctx->gpio_4 = devm_gpiod_get(&pdev->dev, "PCM512x-GPIO4",
> > > +				     GPIOD_OUT_LOW);
> > 
> > Can driver work without this GPIO? If so, perhaps devm_gpiod_get_optional().
> 
> that part yes, it's only for the LED, but if this fails then probably the
> rest of the code will also fail.

The problem with above code that it's setting the hard dependency to a LED
gpio. Is it crucial to get codec working? I bet no. In case
gpiod_get_optional() fails, it will be correct to bail out, because it will
mean other kind of errors not related to optionality of the GPIO (rather it's
present, but something went wrong).

> > > +	if (IS_ERR(ctx->gpio_4)) {
> > > +		dev_err(&pdev->dev, "gpio4 not found\n");
> > > +		ret = PTR_ERR(ctx->gpio_4);
> > > +		return ret;
> > > +	}

-- 
With Best Regards,
Andy Shevchenko