| Message ID | 1473416416-4643-1-git-send-email-jh80.chung@samsung.com |
|---|---|
| State | Rejected |
| Delegated to: | Marek Vasut |
| Headers | show |
+Marek On 9 September 2016 at 04:20, Jaehoon Chung <jh80.chung@samsung.com> wrote: > If USB is stopped, just run 'usb start' instead of printing message. > Then user didn't consider whether usb is started or stopped. > > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> > --- > cmd/usb.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/cmd/usb.c b/cmd/usb.c > index 455127c..4970851 100644 > --- a/cmd/usb.c > +++ b/cmd/usb.c > @@ -651,8 +651,8 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > return 0; > } > if (!usb_started) { > - printf("USB is stopped. Please issue 'usb start' first.\n"); > - return 1; > + printf("USB is stopped. Running 'usb start' first.\n"); > + do_usb_start(); > } > if (strncmp(argv[1], "tree", 4) == 0) { > puts("USB device tree:\n"); > -- > 1.9.1 > Reviewed-by: Simon Glass <sjg@chromium.org>
Hi Marek, On 09/23/2016 01:15 PM, Simon Glass wrote: > +Marek > > On 9 September 2016 at 04:20, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> If USB is stopped, just run 'usb start' instead of printing message. >> Then user didn't consider whether usb is started or stopped. Do you have any other opinion for this? :) Best Regards, Jaehoon Chung >> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >> --- >> cmd/usb.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/cmd/usb.c b/cmd/usb.c >> index 455127c..4970851 100644 >> --- a/cmd/usb.c >> +++ b/cmd/usb.c >> @@ -651,8 +651,8 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> return 0; >> } >> if (!usb_started) { >> - printf("USB is stopped. Please issue 'usb start' first.\n"); >> - return 1; >> + printf("USB is stopped. Running 'usb start' first.\n"); >> + do_usb_start(); >> } >> if (strncmp(argv[1], "tree", 4) == 0) { >> puts("USB device tree:\n"); >> -- >> 1.9.1 >> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > >
Hi Jaehoon, On 28/11/16 14:08, Jaehoon Chung wrote: > Hi Marek, > > On 09/23/2016 01:15 PM, Simon Glass wrote: >> +Marek >> >> On 9 September 2016 at 04:20, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>> If USB is stopped, just run 'usb start' instead of printing message. >>> Then user didn't consider whether usb is started or stopped. > > Do you have any other opinion for this? :) > > Best Regards, > Jaehoon Chung > >>> >>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >>> --- >>> cmd/usb.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/cmd/usb.c b/cmd/usb.c >>> index 455127c..4970851 100644 >>> --- a/cmd/usb.c >>> +++ b/cmd/usb.c >>> @@ -651,8 +651,8 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >>> return 0; >>> } >>> if (!usb_started) { >>> - printf("USB is stopped. Please issue 'usb start' first.\n"); >>> - return 1; >>> + printf("USB is stopped. Running 'usb start' first.\n"); >>> + do_usb_start(); >>> } It seems to ambiguous whether initialization was succeed or not. Thanks, Minkyu Kang.
Hi, On 28-11-16 07:54, Minkyu Kang wrote: > Hi Jaehoon, > > On 28/11/16 14:08, Jaehoon Chung wrote: >> Hi Marek, >> >> On 09/23/2016 01:15 PM, Simon Glass wrote: >>> +Marek >>> >>> On 9 September 2016 at 04:20, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>> If USB is stopped, just run 'usb start' instead of printing message. >>>> Then user didn't consider whether usb is started or stopped. >> >> Do you have any other opinion for this? :) >> >> Best Regards, >> Jaehoon Chung >> >>>> >>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >>>> --- >>>> cmd/usb.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/cmd/usb.c b/cmd/usb.c >>>> index 455127c..4970851 100644 >>>> --- a/cmd/usb.c >>>> +++ b/cmd/usb.c >>>> @@ -651,8 +651,8 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >>>> return 0; >>>> } >>>> if (!usb_started) { >>>> - printf("USB is stopped. Please issue 'usb start' first.\n"); >>>> - return 1; >>>> + printf("USB is stopped. Running 'usb start' first.\n"); >>>> + do_usb_start(); >>>> } > > It seems to ambiguous whether initialization was succeed or not. Right at a minimum it should detect that do_usb_start succeeds. E.g. on an otg port without an otg -> usb-host cable plugged in it will not succeed. Regards, Hans
On 11/28/2016 05:11 PM, Hans de Goede wrote: > Hi, > > On 28-11-16 07:54, Minkyu Kang wrote: >> Hi Jaehoon, >> >> On 28/11/16 14:08, Jaehoon Chung wrote: >>> Hi Marek, >>> >>> On 09/23/2016 01:15 PM, Simon Glass wrote: >>>> +Marek >>>> >>>> On 9 September 2016 at 04:20, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>>> If USB is stopped, just run 'usb start' instead of printing message. >>>>> Then user didn't consider whether usb is started or stopped. >>> >>> Do you have any other opinion for this? :) >>> >>> Best Regards, >>> Jaehoon Chung >>> >>>>> >>>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >>>>> --- >>>>> cmd/usb.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/cmd/usb.c b/cmd/usb.c >>>>> index 455127c..4970851 100644 >>>>> --- a/cmd/usb.c >>>>> +++ b/cmd/usb.c >>>>> @@ -651,8 +651,8 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >>>>> return 0; >>>>> } >>>>> if (!usb_started) { >>>>> - printf("USB is stopped. Please issue 'usb start' first.\n"); >>>>> - return 1; >>>>> + printf("USB is stopped. Running 'usb start' first.\n"); >>>>> + do_usb_start(); >>>>> } >> >> It seems to ambiguous whether initialization was succeed or not. > > Right at a minimum it should detect that do_usb_start succeeds. E.g. > on an otg port without an otg -> usb-host cable plugged in it will not > succeed. Got it..Then discard this patch. Thanks for pointing out. Best Regards, Jaehoon Chung > > Regards, > > Hans > > >
diff --git a/cmd/usb.c b/cmd/usb.c index 455127c..4970851 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -651,8 +651,8 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 0; } if (!usb_started) { - printf("USB is stopped. Please issue 'usb start' first.\n"); - return 1; + printf("USB is stopped. Running 'usb start' first.\n"); + do_usb_start(); } if (strncmp(argv[1], "tree", 4) == 0) { puts("USB device tree:\n");
If USB is stopped, just run 'usb start' instead of printing message. Then user didn't consider whether usb is started or stopped. Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> --- cmd/usb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)