| Message ID | 20201215165439.13165-1-m.szyprowski@samsung.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | None | expand |
On 12/15/20 5:54 PM, Marek Szyprowski wrote: > Save examined button state in 'button' environment variable to enable > checking button state in the scripts. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > Resend reason: get rid of the Change-Id tag, that was still in v2. > --- > cmd/button.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/cmd/button.c b/cmd/button.c > index 64c5a8fa04..8da911068a 100644 > --- a/cmd/button.c > +++ b/cmd/button.c > @@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev) > ret = button_get_state(dev); > if (ret >= BUTTON_COUNT) > ret = -EINVAL; > - if (ret >= 0) > + if (ret >= 0) { > printf("%s\n", state_label[ret]); > + env_set("button", state_label[ret]); Using a hard coded environment variable does not make much sense to me. The button command has a return value. So just use button mybutton; setenv myvar $? Best regards Heinrich > + } > > return ret; > } >
On 15.12.2020 20:07, Heinrich Schuchardt wrote: > On 12/15/20 5:54 PM, Marek Szyprowski wrote: >> Save examined button state in 'button' environment variable to enable >> checking button state in the scripts. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> Resend reason: get rid of the Change-Id tag, that was still in v2. >> --- >> cmd/button.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/cmd/button.c b/cmd/button.c >> index 64c5a8fa04..8da911068a 100644 >> --- a/cmd/button.c >> +++ b/cmd/button.c >> @@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev) >> ret = button_get_state(dev); >> if (ret >= BUTTON_COUNT) >> ret = -EINVAL; >> - if (ret >= 0) >> + if (ret >= 0) { >> printf("%s\n", state_label[ret]); >> + env_set("button", state_label[ret]); > > Using a hard coded environment variable does not make much sense to me. > The button command has a return value. So just use > > button mybutton; setenv myvar $? > Thanks for the hint, I wasn't aware that uboot supports '$?'. By setting the 'button' env variable I've tried to mimic the behavior of the various network and file related commands, which sets 'filesize' env variable. I will need to add the return value propagation to the button command anyway to make it usable from the scripts. Best regards
On 12/16/20 8:08 AM, Marek Szyprowski wrote: > On 15.12.2020 20:07, Heinrich Schuchardt wrote: >> On 12/15/20 5:54 PM, Marek Szyprowski wrote: >>> Save examined button state in 'button' environment variable to enable >>> checking button state in the scripts. >>> >>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>> --- >>> Resend reason: get rid of the Change-Id tag, that was still in v2. >>> --- >>> cmd/button.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/cmd/button.c b/cmd/button.c >>> index 64c5a8fa04..8da911068a 100644 >>> --- a/cmd/button.c >>> +++ b/cmd/button.c >>> @@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev) >>> ret = button_get_state(dev); >>> if (ret >= BUTTON_COUNT) >>> ret = -EINVAL; >>> - if (ret >= 0) >>> + if (ret >= 0) { >>> printf("%s\n", state_label[ret]); >>> + env_set("button", state_label[ret]); >> >> Using a hard coded environment variable does not make much sense to me. >> The button command has a return value. So just use >> >> button mybutton; setenv myvar $? >> > Thanks for the hint, I wasn't aware that uboot supports '$?'. By setting > the 'button' env variable I've tried to mimic the behavior of the > various network and file related commands, which sets 'filesize' env > variable. > > I will need to add the return value propagation to the button command > anyway to make it usable from the scripts. Nothing to be done for the button command. Since a6bfd71a96201127836d59736abcb54dc2d5e1a5 cmd/button: return button status the button command returns 0 (true) - pressed, on 1 (false) - not pressed, off Best regards Heinrich
On 16.12.2020 08:29, Heinrich Schuchardt wrote: > On 12/16/20 8:08 AM, Marek Szyprowski wrote: >> On 15.12.2020 20:07, Heinrich Schuchardt wrote: >>> On 12/15/20 5:54 PM, Marek Szyprowski wrote: >>>> Save examined button state in 'button' environment variable to enable >>>> checking button state in the scripts. >>>> >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> --- >>>> Resend reason: get rid of the Change-Id tag, that was still in v2. >>>> --- >>>> cmd/button.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/cmd/button.c b/cmd/button.c >>>> index 64c5a8fa04..8da911068a 100644 >>>> --- a/cmd/button.c >>>> +++ b/cmd/button.c >>>> @@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev) >>>> ret = button_get_state(dev); >>>> if (ret >= BUTTON_COUNT) >>>> ret = -EINVAL; >>>> - if (ret >= 0) >>>> + if (ret >= 0) { >>>> printf("%s\n", state_label[ret]); >>>> + env_set("button", state_label[ret]); >>> >>> Using a hard coded environment variable does not make much sense to me. >>> The button command has a return value. So just use >>> >>> button mybutton; setenv myvar $? >>> >> Thanks for the hint, I wasn't aware that uboot supports '$?'. By setting >> the 'button' env variable I've tried to mimic the behavior of the >> various network and file related commands, which sets 'filesize' env >> variable. >> >> I will need to add the return value propagation to the button command >> anyway to make it usable from the scripts. > > Nothing to be done for the button command. Since > > a6bfd71a96201127836d59736abcb54dc2d5e1a5 > cmd/button: return button status > > the button command returns > > 0 (true) - pressed, on > 1 (false) - not pressed, off > Right, I've missed that commit. I've developed my code on top of v2020.10 release. Best regards
Hi Marek, On Tue, 15 Dec 2020 at 09:55, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Save examined button state in 'button' environment variable to enable > checking button state in the scripts. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > Resend reason: get rid of the Change-Id tag, that was still in v2. If you use patman it does that for you. Regards, Simon
diff --git a/cmd/button.c b/cmd/button.c index 64c5a8fa04..8da911068a 100644 --- a/cmd/button.c +++ b/cmd/button.c @@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev) ret = button_get_state(dev); if (ret >= BUTTON_COUNT) ret = -EINVAL; - if (ret >= 0) + if (ret >= 0) { printf("%s\n", state_label[ret]); + env_set("button", state_label[ret]); + } return ret; }
Save examined button state in 'button' environment variable to enable checking button state in the scripts. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- Resend reason: get rid of the Change-Id tag, that was still in v2. --- cmd/button.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)