Hi Tom, On Fri, 31 Jan 2020 at 13:59, Tom Rini <tr...@konsulko.com> wrote: > > On Thu, Jan 30, 2020 at 07:27:57PM -0700, Simon Glass wrote: > > Hi Tom. > > > > On Thu, 30 Jan 2020 at 11:52, Tom Rini <tr...@konsulko.com> wrote: > > > > > > On Wed, Jan 29, 2020 at 07:17:09PM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Thu, 23 Jan 2020 at 14:12, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > > > On Thu, Jan 23, 2020 at 10:04:05PM +0100, Luka Kovačič wrote: > > > > > > > > > > > Hello Tom, > > > > > > > > > > > > thank you for feedback and review. I understand the implications. > > > > > > Would it make sense to document this somewhere to avoid any future > > > > > > confusion? > > > > > > > > > > Yes, along with a standalone patch to update the document to use > > > > > CMD_RET_SUCCESS NOT CMD_SUCCESS. Updating the gpio help text even to > > > > > be > > > > > clear what the return value is would be nice. Thanks! > > > > > > > > > > > > > > > > > Thanks, > > > > > > Luka > > > > > > > > > > > > On Thu, Jan 23, 2020 at 1:31 PM Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > > > > > > > On Sun, Jan 05, 2020 at 08:10:56PM +0100, Luka Kovacic wrote: > > > > > > > > > > > > > > > Use the correct return value in function do_gpio() and update > > > > > > > > commands documentation with the return values from > > > > > > > > command_ret_t enum. > > > > > > > > > > > > > > > > CMD_RET_SUCCESS is returned on command success and > > > > > > > > CMD_RET_FAILURE is > > > > > > > > returned on command failure. > > > > > > > > > > > > > > > > The command was returning the pin value, which caused confusion > > > > > > > > when > > > > > > > > debugging (#define DEBUG). > > > > > > > > > > > > > > > > Signed-off-by: Luka Kovacic <luka.kova...@sartura.hr> > > > > > > > > Tested-by: Robert Marko <robert.ma...@sartura.hr> > > > > > > > > > > > > > > So, I think the problem is that despite this not being an optimal > > > > > > > user > > > > > > > interface, it's what we've had here for "forever". We can't just > > > > > > > go > > > > > > > change it now as there's scripts out in the world (and even > > > > > > > include/configs/) that depend on the current behavior. Sorry, > > > > > > > nak. > > > > > > > > The command is effectively returning a negative value on failure, > > > > which causes the calling shell to try to exit! > > > > > > > > Also 'gpio set' will return failure if you enable a GPIO. I really > > > > can't see that people could be relying too much on the current > > > > behaviour. > > > > > > > > GIven our policy on upstream, if we fix the in-tree scripts do you > > > > think we could fix this problem? > > > > > > > > The 'return -1' is definitely a bug BTW. > > > > > > My first comment is to look at configs/socfpga_vining_fpga_defconfig and > > > include/configs/omap3_beagle.h around 'if gpio' and tell me if I'm > > > simply misunderstanding how things are being used. > > > > > > But if I'm not then I'm not sure just changing the users is OK because > > > it's baked into saved environments. Now I can say that for the Beagle > > > case it might be OK in the end. But I'm not so sure about the socfpga > > > case. Marek? > > > > The omap3 code looks like it is checking if the GPIO is set or not. > > > > Oddly 'if gpio input xx' is true if the GPIO is 0, so it might be > > confusing. Arguably this should be inverted. > > > > So how about we leave the behaviour for 'gpio input' alone, and 'fix' > > the other bits? > > What about the socfpga example? Thanks!
This is also using 'gpio input' so we should be OK. Regards, SImon