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

[David Brownell]

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.


> > Don't use AT91-specific GPIO calls except for things that
> > the generic calls don't support ... like enabling open-drain
> > outputs, the deglitching support, or input pullups.
> 
> This call does port configuration, which you convinced me a long time
> ago was a fundamentally different thing from GPIO.

Yes, pin/port config is certainly part of what the platform's
code to set up devices should handle.  That can include making
sure a given pin is configured as a GPIO ... and in the normal
case where it's dedicated to that task, it simplifies the driver
to have it pre-allocated and configured for I/O/both.

I'm pulling in some discussion from a different email thread
earlier, which proposed doing the right thing and finally
getting rid of the at91-specific GPIO calls except in the few
cases they could not be avoided.

It might be that AT91 needs to add some pin config calls which
resemble what you did for AT32AP7 chips.

- Dave


> If the pin really 
> requires one of those features, this would definitely be the place to
> set it up.
> 
> Haavard
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/