[U-Boot-Users] [PATCH] USB Storage, add meaningful return value
This patch changes the "usb storage" command to return success iff it finds a USB storage device, otherwise it returns error.
This enables you to check for a USB storage device before trying to access it:
if usb storage; then fatload usb 0 $loadaddr $filename; fi
______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________
Dear Aras,
in message 47E836F8.6090702@magtech.com.au you wrote:
This patch changes the "usb storage" command to return success iff it finds a USB storage device, otherwise it returns error.
Thanks. I appreciate your contribution, but please fix the coding style:
@@ -196,9 +196,13 @@ for (i = 0; i < usb_max_devs; i++) {
There is probably a { missing before this line (as this is not a single-line statement).
printf (" Device %d: ", i); dev_print(&usb_dev_desc[i]);
return 0;- {
...and then this should read
} else {
Thanks.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Aras,
in message 47E836F8.6090702@magtech.com.au you wrote:
This patch changes the "usb storage" command to return success iff it finds a USB storage device, otherwise it returns error.
Thanks. I appreciate your contribution, but please fix the coding style:
@@ -196,9 +196,13 @@ for (i = 0; i < usb_max_devs; i++) {
There is probably a { missing before this line (as this is not a single-line statement).
printf (" Device %d: ", i); dev_print(&usb_dev_desc[i]);
return 0;- {
...and then this should read
} else {
Thanks.
Best regards,
Wolfgang Denk
Just in case Wolfgang was too terse, I'll add to his critique... the original code is the origin of the coding violations:
void usb_stor_info(void) { int i;
if (usb_max_devs > 0) for (i = 0; i < usb_max_devs; i++) { printf (" Device %d: ", i); dev_print(&usb_dev_desc[i]); } else printf("No storage devices, perhaps not 'usb start'ed..?\n"); }
It doesn't have braces on the "if (usb_max_devs > 0)" which is syntactically OK but the source of the coding violation. Please add braces after the "if" and "} else {" per Wolfgang's comments.
Thanks for making the code a little better and a little prettier, ;-) gvb
Jerry Van Baren wrote:
Wolfgang Denk wrote:
Dear Aras,
in message 47E836F8.6090702@magtech.com.au you wrote:
This patch changes the "usb storage" command to return success iff it finds a USB storage device, otherwise it returns error.
Thanks. I appreciate your contribution, but please fix the coding style:
Done.
______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________
Aras Vaichas arasv@magtech.com.au writes:
Jerry Van Baren wrote:
Wolfgang Denk wrote:
Dear Aras,
in message 47E836F8.6090702@magtech.com.au you wrote:
This patch changes the "usb storage" command to return success iff it finds a USB storage device, otherwise it returns error.
Thanks. I appreciate your contribution, but please fix the coding style:
Done.
Thanks, i'll pick this one up. Please remember to add a proper Signed-off-by: line next time.
Best regards
Markus Klotzbuecher
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
Aras Vaichas arasv@magtech.com.au writes:
This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________--- a/include/usb.h 2008-03-19 13:47:18.000000000 +1100
Its my fault for not paying attention correctly, but this line essentially broke git-am so that this chunk didn't get applied correctly.
-void usb_stor_info(void) +int usb_stor_info(void) { int i;
- if (usb_max_devs > 0)
- if (usb_max_devs > 0) { for (i = 0; i < usb_max_devs; i++) { printf (" Device %d: ", i); dev_print(&usb_dev_desc[i]);
return 0;
Returning here will result in only the first of all storage devices to be printed.
- else
- } else { printf("No storage devices, perhaps not 'usb start'ed..?\n");
return 1;- }
}
/*********************************************************************************
I committed the following patch to fix this
diff --git a/common/usb_storage.c b/common/usb_storage.c index 81d2f92..d263b6c 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -196,12 +196,12 @@ int usb_stor_info(void) for (i = 0; i < usb_max_devs; i++) { printf (" Device %d: ", i); dev_print(&usb_dev_desc[i]); - return 0; } - } else { - printf("No storage devices, perhaps not 'usb start'ed..?\n"); - return 1; + return 0; } + + printf("No storage devices, perhaps not 'usb start'ed..?\n"); + return 1; }
/*********************************************************************************
Best regards
Markus
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
Jerry Van Baren wrote:
Just in case Wolfgang was too terse, I'll add to his critique... the original code is the origin of the coding violations:
void usb_stor_info(void) { int i;
if (usb_max_devs > 0) for (i = 0; i < usb_max_devs; i++) { printf (" Device %d: ", i); dev_print(&usb_dev_desc[i]); } else printf("No storage devices, perhaps not 'usbstart'ed..?\n"); }
It doesn't have braces on the "if (usb_max_devs > 0)" which is syntactically OK but the source of the coding violation. Please add braces after the "if" and "} else {" per Wolfgang's comments.
Thanks for making the code a little better and a little prettier, ;-) gvb
Hey Stefan,
What do you think? Wouldn't a policy of _always_ using braces even for single sub-statements have just made this a _correct_ no-brainer from the onset? :-)
jdl
PS -- No, no point. Why do you ask?
Scott Wood wrote:
On Tue, Mar 25, 2008 at 09:23:43AM -0500, Jon Loeliger wrote:
What do you think? Wouldn't a policy of _always_ using braces even for single sub-statements have just made this a _correct_ no-brainer from the onset? :-)
Yeah, but it'd be ugly. :-)
Luckily, everyone is entitled to their own wrong opinion. :-)
jdl
On Tuesday 25 March 2008, Jon Loeliger wrote:
It doesn't have braces on the "if (usb_max_devs > 0)" which is syntactically OK but the source of the coding violation. Please add braces after the "if" and "} else {" per Wolfgang's comments.
Thanks for making the code a little better and a little prettier, ;-) gvb
Hey Stefan,
Why me? :)
What do you think? Wouldn't a policy of _always_ using braces even for single sub-statements have just made this a _correct_ no-brainer from the onset? :-)
I think I read some time on LKML, that braces should be used on all multi-line paragraphs. So even one statement with a comment should have braces. That's what I would prefer. And braces on both parts of the "else" if one needs them. So something like is what I would vote for:
if (a == b) { /* some comment */ c = 1; } else { c = 2; }
But I still prefer using no braces on single line paragraph:
if (a == b) c = 1; else c = 2;
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
participants (7)
-
Aras Vaichas -
Jerry Van Baren -
Jon Loeliger -
Markus Klotzbücher -
Scott Wood -
Stefan Roese -
Wolfgang Denk