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

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Thu, Apr 09, 2020 at 02:58:28PM -0500, Pierre-Louis Bossart wrote:
> Remove direct regmap access, use gpios exposed by PCM512x codec
> Keep the codec_init function, this will be used in following patches
> 
> The gpios handling is done with an explicit lookup table. We cannot
> use ACPI-based mappings since we don't have an ACPI device for the
> machine driver, and the gpiochip is created during the probe of the
> PCM512x driver.

...

> +#include <linux/gpio/machine.h>

Okay, it's a board code.

...

> +static struct gpiod_lookup_table pcm512x_gpios_table = {
> +	/* .dev_id set during probe */
> +	.table = {

> +		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?

> +		{ },

No comma for terminator entries.

> +	},
> +};

...

> +	gpiod_add_lookup_table(&pcm512x_gpios_table);

Where is the counterpart gpiod_remove_lookup_table() call?

> +	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().

> +	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