All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [PATCH 1/1 v1] cmd: gpio: Correct do_gpio() return value
Date: Wed, 29 Jan 2020 19:17:09 -0700	[thread overview]
Message-ID: <CAPnjgZ2HrWvK4NWWXqinu-zCFdLWBU4axS52-Ozb89Oaan6WYg@mail.gmail.com> (raw)
In-Reply-To: <20200123211204.GC26536@bill-the-cat>

Hi Tom,

On Thu, 23 Jan 2020 at 14:12, Tom Rini <trini@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 <trini@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.kovacic@sartura.hr>
> > > > Tested-by: Robert Marko <robert.marko@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.

Regards,
Simon

  parent reply	other threads:[~2020-01-30  2:17 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-05 19:10 [PATCH 0/1 v1] cmd: gpio: Correct do_gpio() return value Luka Kovacic
2020-01-05 19:10 ` [PATCH 1/1 " Luka Kovacic
2020-01-23 12:31   ` Tom Rini
2020-01-23 21:04     ` Luka Kovačič
2020-01-23 21:12       ` Tom Rini
2020-01-27  6:30         ` [PATCH 1/2 v1] cmd: doc: Update command return values luka.kovacic at sartura.hr
2020-01-27  6:30         ` [PATCH 2/2 v1] cmd: gpio: Add the command return value to the help text luka.kovacic at sartura.hr
2020-01-30  2:17         ` Simon Glass [this message]
2020-01-30 18:52           ` [PATCH 1/1 v1] cmd: gpio: Correct do_gpio() return value Tom Rini
2020-01-31  2:27             ` Simon Glass
2020-01-31 20:59               ` Tom Rini
2020-02-02 22:50                 ` Simon Glass
2020-02-08  0:05   ` Tom Rini
2020-03-10  9:47     ` Alex Kiernan
2020-03-10 12:37       ` Tom Rini
2020-03-10 13:23         ` Alex Kiernan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPnjgZ2HrWvK4NWWXqinu-zCFdLWBU4axS52-Ozb89Oaan6WYg@mail.gmail.com \
    --to=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.