Re: [PATCH 1/3] gpiolib: make gpio_to_chip() public

[Laurent Pinchart <laurentp@cse-semaphore.com>]

[-- Attachment #1: Type: text/plain, Size: 2461 bytes --]

On Thursday 14 August 2008, Anton Vorontsov wrote:
> On Thu, Aug 14, 2008 at 04:45:52PM +0200, Laurent Pinchart wrote:
> > On Thursday 14 August 2008, Anton Vorontsov wrote:
> > > On Thu, Aug 14, 2008 at 04:04:18PM +0200, Laurent Pinchart wrote:
> > > > On Friday 08 August 2008, Anton Vorontsov wrote:
> > > > > We'll need this function to write platform-specific hooks to deal
> > > > > with pin's dedicated functions. Quite obviously this will work only
> > > > > for the platforms with 1-to-1 GPIO to PIN mapping.
> > > > > 
> > > > > This is stopgap solution till we think out and implement a proper
> > > > > api (pinlib?).
> > > > 
> > > > How do you support reverting the GPIO mode to non-dedicated ?
> > > 
> > > As we always do with the GPIO API: gpio_direction_*() calls.
> > 
> > So the proper sequence to configure a pin in dedicated mode is to set
> > the direction first (which will unset the dedicated mode bit) and
> > then set dedicated mode (which will not touch the direction bit) ?
> 
> Not exactly. But you can do this way, if you need to preserve
> a direction. What I did is a bit different though.
> 
> qe_gpio_set_dedicated() actually just restores a mode that
> firmware had set up, including direction (since direction could
> be a part of dedicated configuration).
> 
> That is, upon GPIO controller registration, we save all registers,
> then driver can set up a pin to a GPIO mode via standard API, and
> then it can _revert_ a pin to a dedicated function via
> qe_gpio_set_dedicated() call. Dedicated function is specified by
> the firmware (or board file), we're just restoring it.

The semantic of the set_dedicated() operation needs to be clearly defined then. I can live with this behaviour, but it might not be acceptable for everybody.

Your patch requires the firmware to set a pin in dedicated mode at bootup in order to be used later in dedicated mode. If, for some reason (driver not loaded, ...), no GPIO user shows up for that pin, it will stay configured in dedicated mode.

It might be better to set the PAR bit unconditionally in qe_gpio_set_dedicated() instead of restoring its value. That way the board initialization code could just set the DIR, DAT and ODR bits for dedicated mode but still configure the pin in GPIO mode.

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]