Re: [PATCH 2/2] sound: soc: tlv320aic32x4: Add rstn gpio to platform data.

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1.1: Type: text/plain, Size: 885 bytes --]

On Tue, Oct 30, 2012 at 04:29:34PM +0100, Javier Martin wrote:
> Add the possibility to specify a gpio through platform data
> so that a HW reset can be issued to the codec.

Please use subject lines appropriate for the subsystem you're submitting
against.

> +		ret = gpio_request(aic32x4->rstn_gpio, "tlv320aic32x4 rstn");
> +		if (ret != 0)
> +			return ret;

Should be devm_gpio_request_one(), saving code for cleanup and making
sure there aren't any leaks (I think you have some in error cases here).
gpio_request_one() is better style in general.

> +		gpio_direction_output(aic32x4->rstn_gpio, 1);
> +		gpio_set_value(aic32x4->rstn_gpio, 0);
> +		ndelay(10);
> +		gpio_set_value(aic32x4->rstn_gpio, 1);

This looks weird - you request the GPIO active high then immediately
transition it to low.  I'd expect the code to set the output low when
putting the GPIO into output mode.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]