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. -- 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: a...@cn-eng.de _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot