[U-Boot] [RFC] gpio command: return value on write, additional actions

[Andreas Pretzsch]

Am Donnerstag, den 07.07.2011, 12:15 +0200 schrieb Detlev Zundel:
> Hi Wolfgang,
> 
> > Dear Detlev Zundel,
> >
> > In message <m2aacrlsen.fsf at ohwell.denx.de> you wrote:
> >> 
> >> > I'd say clear/set/toggle are changeable, don't see any legit
> >> > return-value-usage here. For 100% backward compatibility, one could
> >> > leave them as they are and use 0|1 as new actions with return 0, as
> >> > proposed.
> >
> > Hm... I think it would be beneficial to bee able to get information
> > about the current settings. For "clear" and "set" the result is known,
> > but for "toggle" it would be helpful if we returned the current port
> > state.  Eventually we should add a "test" command (or "read" ?) that
> > returns the current setting?

Regarding toggle, the typical usage would be clocking or switch some LED
or similar. In all these cases, the new value is irrelevant or could be
read by the "get value" subcommand later.
( Which implies some "get output buffer" or "switch to input", depending
on GPIO hardware structure. But this might be the case also for the
low-level GPIO driver. )
All other "clever" direct usage of the new output value of toggle sounds
to me like "write a driver" cases...
So I'd opt for "return success", as with clear/0 and set/1.

Regarding test/read, see below.


> In general the "current state" actually are three things:
> 
> 1. Port Mode: Disabled (i.e. not connected to an external pin), Input,
>    Output

Already handled by the gpio_request() resp. gpio_direction_input() and
gpio_direction_output() calls. At least, it should be this way, didn't
check.

> 2. Port output value

Read by "gpio value", as output latch is always readable.

> 3. (Actually read-in) input value

Either via "gpio value" (read pin value or output latch, depending on
hardware, essentially the return of gpio_get_value()) or via "gpio
input", which would set the port to input and return its value.

> The current commands mix these aspects, i.e. they implicitely change the
> port mode.  Differentiating between 2 and 3 will not always be possible
> but is - I've seen that on e.g. an i.mx31.

Most better GPIO blocks have a register both for output latch and for
pin real value, as they have hardware toggle support.


> >> > So these variants:
> >> >   gpio clear|0 => set to output, write 0, return success
> >> >   gpio set|1   => set to output, write 1, return success
> >> >   gpio toggle  => (set to output), toggle output, return success
> >> >   gpio input   => set to input, return pin value
> >> >   gpio value   => return current pin/latch/whatever value

As I understand, this proposal is still complete and valid.
The only question (beside toggle return value) is the naming of the last
command, either "value" or "get". I'd opt for value, as get implies for
me (to some extent) switch to input.


> >> I'd propose to fix the commands to be sensible now.  Actually I believe
> >> that they should not be in heavy use "in the wild" and so we should take
> >> the opportunity and declare the current behaviour as buggy and fix it.
> >> Rather now than later ;)
> >
> > Agreed.

Great. So I'll go ahead and send a patch after we decided what exactly
to do.


> >> Actually I would expect the "output" commands to return true when they
> >> were able to do what was requestes from them, i.e. drive the requested
> >> value to the output.  I guess this cannot be done in the general case,
> >> but for a "weak output" that can be read back, this would be the most
> >> sensible thing to do.

Even with a decent GPIO module where you can read back the real pin
value also in output mode, it's not always sufficient to read back
directly (a few ns/us) after you set the output. For example, depending
on the load attached to the output, it might take some time to charge to
target level.
Beside that, you demand from the low-level GPIO layer that it won't
switch back to input by itself for a read.
While it's a neat idea at the first sight, I think it's going too far
for a script-used gpio command.


> > For consistency I would prefer to have all commands return the same
> > type of information, i. e. either an error status (like we do with all
> > other commands - any result values would then have to be passed as
> > environment settings), or we return the current port state (also for
> > the "output" commands -  see my comments for "toggle").
> 
> Actually I would prefer to return an error status for all cases and
> return "valuable information" in the environment so we can easily use it
> subsequently.  As we do not have a back-tick operator in the shell, I
> believe that return codes are only useful as error codes.

True for now, beside one usage:
    if gpio value <x> ; then ; else ; fi
Of course, the same could be achieved with
    gpio value <x> ; if test $? -eq 1 ; then ; else ; fi


While we're at it, adding another optional parameter to store the value
read in an environment variable sounds sensible:
    gpio <input|set|1|clear|0|toggle|value> <port><pin> [env_var]


Another point: I'd like to quiet down the gpio command. Either in
default case or via some other way (like gpio.q or another parameter).
Call me picky, but I like my programs to be quiet unless there is an
error. Especially when running scripted.
Opinions/decisions ?


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch          Tel. +49-(0)731-5521572
Hahnengasse 3                             Fax: +49-(0)731-5521573
89073 Ulm, Germany                        email: apr at cn-eng.de