[PATCH 3/4] ALSA: pxa27x: ac97 controller driver requests gpio

[Igor Grinberg]

On 01/07/13 16:10, Mike Dunn wrote:
> On 01/07/2013 01:36 AM, Igor Grinberg wrote:
>> On 01/06/13 21:13, Mike Dunn wrote:
>>> During initialization, the ac97 driver must obtain the gpio for the mfp that is
>>> used by the ac97 controller as the AC97_nRESET line, or else gpiolib will issue
>>> a warning and stack dump.  Gpiolib may change this to a hard error soon.  The
>>> line is used during an ac97 warm reset for working around a hardware bug in the
>>> controller.
>>>
>>> Tested on a palm treo 680 machine.
>>>
>>> Signed-off-by: Mike Dunn <mikedunn at newsguy.com>
>>> ---
>>>  sound/arm/pxa2xx-ac97-lib.c |   10 +++++++++-
>>>  1 files changed, 9 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/sound/arm/pxa2xx-ac97-lib.c b/sound/arm/pxa2xx-ac97-lib.c
>>> index 1ecd0a66..416d2e3 100644
>>> --- a/sound/arm/pxa2xx-ac97-lib.c
>>> +++ b/sound/arm/pxa2xx-ac97-lib.c
>>> @@ -18,6 +18,7 @@
>>>  #include <linux/delay.h>
>>>  #include <linux/module.h>
>>>  #include <linux/io.h>
>>> +#include <linux/gpio.h>
>>>  
>>>  #include <sound/ac97_codec.h>
>>>  #include <sound/pxa2xx-lib.h>
>>> @@ -344,7 +345,12 @@ int pxa2xx_ac97_hw_probe(struct platform_device *dev)
>>>  	}
>>>  
>>>  	if (cpu_is_pxa27x()) {
>>> -		/* Use GPIO 113 as AC97 Reset on Bulverde */
>>> +		ret = gpio_request(reset_gpio, "pxa27x ac97 reset");
>>> +		if (ret < 0) {
>>> +			pr_err("%s: gpio_request() failed: %d\n",
>>> +			       __func__, ret);
>>> +			goto err_conf;
>>> +		}
>>
>> I think if we are fine with the request in the driver,
>> then we should move the direction and value configuration also
>> to the driver, but let the pxa27x_assert_ac97reset() only switch
>> the AF.
> 
> 
> Well, I won't make a fuss on this point, but with code that runs very
> infrequently, I thought that the cost of a call to gpio_direction_output() was
> worth the clarity and insurance.

This is not about how frequent the code runs, but the fact that the arch code
relies on sound code (outside of arch) to request the GPIO for it.
I would recommend to place the GPIO request call along with the
direction/value calls - in the same subsystem.

Also, the patch ordering is not good:
patch 2 drives the GPIO, and patch 3 requests it,
whereas it should be the other way around, so I would recommend to
combine both into one.

-- 
Regards,
Igor.