Am Dienstag, den 05.07.2011, 13:44 -0400 schrieb Mike Frysinger:
On Tuesday, July 05, 2011 12:59:16 Andreas Pretzsch wrote:
As of today (2011.06), the generic gpio command (common/cmd_gpio.c) gpio <input|set|clear|toggle> <port><pin> - input/set/clear/toggle the specified pin (e.g. PF10) always returns the value read or set.
While this is sensible for read (input) and maybe (questionable) for toggle, I don't see an usage for write (set/clear).
the trouble with toggle is that there's no way to get the value without changing it to the input. we'd have to add another field that has no
True, at least on devices without hardware toggle support. And you never know if you read back the real pin state, the output latch or something invalid. And a "short" switch to input isn't always legal wrt to the attached hardware. Great when setting 1 bit is worth 10 bit of problems ;-)
Personally, I'd shift the gpio_toggle to the underlying hardware specific code. And print an error if it's not supported.
unintentional side effects like "gpio value". then we could change all the others to return 0/1 based on whether they succeeded, not based on the level of the gpio pin.
Didn't quite get that. In terms of "gpio value" = "give me the current (output latch) value without setting it to input if it's an output" ?
We can't change the return value of "gpio input", as it's out in the wild and would break existing scripts.
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.
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 or gpio clear => set to output, write 0, return 0 gpio set => set to output, write 1, return 1 gpio 0 => set to output, write 0, return success gpio 1 => set to output, write 1, return success gpio toggle => (set to output), toggle output, return new_val gpio input => set to input, return pin value gpio value => return current pin/latch/whatever value
Not perfectly symmetric, but the best way out I can think of. Maybe "get" instead of "value", as it's more usual. OTOH, get (to some people) implies "set to input", so value is more explicit.
Also, this leads to unexpected side effects with complex constructs, e.g. consider this environment: setA=gpio set PF1 setB=gpio clear PF2 setAB_separate=env run setA ; env run setB setAB_concatenated=env run setA setB setBA_concatenated=env run setB setA
While executing "setAB_separate" and "setBA_concatenated" work as expected, "setAB_concatenated" will only run setA, but not setB, as setA "failed" (ret=1). [1] The same would apply to e.g. && constructs.
ive never used the shell in u-boot, but couldnt the first logic be: setA=gpio set PF1 || : setB=gpio clear PF2 || :
Not fully, you'd need "gpio set PF1 || true". Not that this makes it less ugly... BTW, nobody would mind that without notice. Stumbled over it by pure accident myself.
As it works this way in the release and hence the behaviour is essentially fixed, changing the return value is probably not an option.
not really. poor behavior can be adjusted. -mike
Fine from my side, but I'm not the one to decide.