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

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.