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

Laurent Pinchart
2008-08-18T14:45:03Z

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

On Monday 18 August 2008, Anton Vorontsov wrote:
> On Mon, Aug 18, 2008 at 03:58:46PM +0200, Laurent Pinchart wrote:
> [...]
> > > 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.
>
> It is. We set up a dedicated function that firmware (or board file)
> has configured.

A comment in the source would help.

> > I can live with this behaviour, but it might not be
> > acceptable for everybody.
>
> For example?
>
> > Your patch requires the firmware to set a pin in dedicated mode at
> > bootup in order to be used later in dedicated mode.
>
> Yes. On a PowerPC this is always true: firmware should set up PIO
> config. Linux' board file could fixup the firmware though.

That's not what I meant. What if the hardware requires to pin to be configured in GPIO mode with a fixed value until the SOC-specific driver that will drive the GPIO is loaded ? That's not possible with your API.

Until a SOC peripheral is initialized by its associated Linux driver, the behaviour of a GPIO pin in dedicated mode will be undefined. The firmware/board code will probably want to set the pin as a GPIO output with a fixed value until the driver initializes the hardware.

> Another option would be to add some argument to the set_dedicated
> call, thus the software could specify arbitrary dedicated
> function (thus no need to save/restore anything). But this would
> be SOC-model specific, thus no driver can use this argument anyway.

Drivers that require dedicated mode are SOC-specific anyway.

> > If, for some
> > reason (driver not loaded, ...), no GPIO user shows up for that
> > pin, it will stay configured in dedicated mode.
>
> Yes.
>
> > It might be better to set the PAR bit unconditionally in
>
> Why it might be better?

Because, as explained a few lines down, the board initialization code will be able to configure a pin in a known state (PAR not set) at boot time until a driver requests the pin to be switched to dedicated mode.

> That way you may set up wrong _GPIO_
> mode, because you didn't set PAR bit (when PAR bit set
> DIR/ODR/DAT bits are losing their meanings).
>
> > 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 --]