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;- }