Re: [PATCH 2/2] at91/USB: at91sam9g45 series USB host integration

[Andrew Victor <avictor.za@gmail.com>]

hi David,

> On Friday 19 June 2009, Haavard Skinnemoen wrote:
>> David Brownell wrote:
>> > > --- a/arch/arm/mach-at91/at91sam9g45_devices.c
>> > > +++ b/arch/arm/mach-at91/at91sam9g45_devices.c
>>
>> > > + /* Enable VBus control for UHP ports */
>> > > + for (i = 0; i < data->ports; i++) {
>> > > +         if (data->vbus_pin[i])
>> > > +                 at91_set_gpio_output(data->vbus_pin[i], 0);
>> >
>> > This should gpio_request() and gpio_direction_output().
>>
>> Hmm...I thought the driver was supposed to call gpio_request(), not the
>> platform code?
>
> In some cases.  This isn't a good case for that.  Especially
> if it's going to call gpio_direction_output() ... which needs
> gpio_request() to have been done first.

I have to agree with Haavard on this - it's really not clear if
gpio_request() should be called in the platform-code or in the driver.

If the platform code performs a gpio_request() then surely it needs to
call a gpio_free() after configuring the pin.
Otherwise the driver's initialization code performs another
gpio_request() for that pin, but it is still "in-use" by the platform
code.

Also Documentation/gpio.txt doesn't say if a GPIO pin should even
retain it's configured state across after a gpio_free().


So for the core AT91 platform code, I'd continue to use the
AT91-specific GPIO configuration functions.
The drivers should perform the gpio_request() / gpio_free(), and it
can still call gpio_direction_output() if necessary.


Regards,
  Andrew Victor