UBI command support
PATCH 1/1 UBI command support
UBI command support
--- common/Makefile | 1 + common/cmd_ubi.c | 535 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 536 insertions(+), 0 deletions(-) create mode 100644 common/cmd_ubi.c
common/Makefile +1 -0
@@ -139,6 +139,7 @@ COBJS-$(CONFIG_CMD_SETEXPR) += cmd_setexpr.o COBJS-$(CONFIG_CMD_SPI) += cmd_spi.o COBJS-$(CONFIG_CMD_STRINGS) += cmd_strings.o COBJS-$(CONFIG_CMD_TERMINAL) += cmd_terminal.o +COBJS-$(CONFIG_CMD_UBI) += cmd_ubi.o COBJS-$(CONFIG_CMD_UNIVERSE) += cmd_universe.o ifdef CONFIG_CMD_USB COBJS-y += cmd_usb.o
common/cmd_ubi.c new file +535 -0
535 lines not on this page · raw message
Signature · 1 line
1.5.3.3
PATCH 1/1 UBI command support
Dear Kyungmin Park,
Quoted 3 lines · In message <20081021091859.GA29306@july> you wrote:
UBI command support
Signed-off-by Kyungmin Park <[email protected]>
...
common/cmd_ubi.c line 6
Quoted 8 lines
+++
b/common/cmd_ubi.ccommon/cmd_ubi.c new file +6
-0@@ -0,0 +1,535 @@ +/* +* Unsorted Block Image commands +*/ + +#include <common.h> +#include <command.h>
GPL header missing.
common/cmd_ubi.c line 13
Quoted 1 line
common/cmd_ubi.c new file +1
-0+/* board/\*\/ubi.c */
What's the '\'s there?
common/cmd_ubi.c line 14
Quoted 1 line
common/cmd_ubi.c new file +1
-0+extern int ubi_board_scan (void );
Please move the prototypes to some header file.
Quoted 12 lines
common/cmd_ubi.c new file +12
-0+/* drivers/mtd/ubi/build.c */ +extern struct ubi_device *ubi_devices[]; + +/* Private own data */ +static struct ubi_device *ubi; +static int ubi_initialized; + +static void ubi_dump_vol_info (const struct ubi_volume *vol) +{ +dbg_msg ("volume information dump:" ); +dbg_msg ("vol_id %d" , vol->vol_id); +dbg_msg ("reserved_pebs %d" , vol->reserved_pebs);
common/cmd_ubi.c line 37
Quoted 10 lines
common/cmd_ubi.c new file +10
-0+dbg_msg ("alignment %d" , vol->alignment); +dbg_msg ("data_pad %d" , vol->data_pad); +dbg_msg ("vol_type %d" , vol->vol_type); +dbg_msg ("name_len %d" , vol->name_len); +dbg_msg ("usable_leb_size %d" , vol->usable_leb_size); +dbg_msg ("used_ebs %d" , vol->used_ebs); +dbg_msg ("used_bytes %lld" , vol->used_bytes); +dbg_msg ("last_eb_bytes %d" , vol->last_eb_bytes); +dbg_msg ("corrupted %d" , vol->corrupted); +dbg_msg ("upd_marker %d" , vol->upd_marker);
Please use the standard debug() macro.
common/cmd_ubi.c line 96
Quoted 1 line
common/cmd_ubi.c new file +1
-0+static int ustrtoul (const char *cp,char **endp,unsigned int base)
Maybe we should add this to some global place, like lib_generic ?
common/cmd_ubi.c line 101
Quoted 4 lines
common/cmd_ubi.c new file +4
-0+unsigned long result =simple_strtoul (cp, endp, base); +switch (**endp) { +case 'G' : + result *=1024 ;
Please add a
/* fall through */
comment to make clkear that it is intentional not to have a "break;" here.
Review line 37
Quoted 2 lines
(quoted diff) +2
-0+ case 'M': + result *= 1024;
Ditto.
Quoted 3 lines
common/cmd_ubi.c new file +3
-0+static int verify_mkvol_req (const struct ubi_device *ubi, +const struct ubi_mkvol_req *req) +{
...
common/cmd_ubi.c line 169
Quoted 4 lines
common/cmd_ubi.c new file +4
-0+bad: +printf ("bad volume creation request" ); +// ubi_dbg_dump_mkvol_req(req); +return err;
No C++ comments, please. And please don;t add dead code either.
common/cmd_ubi.c line 373
Quoted 1 line
common/cmd_ubi.c new file +1
-0+ tbuf =vmalloc (tbuf_size);
Why do we need new alloc() stuff?
common/cmd_ubi.c line 410
Quoted 2 lines
common/cmd_ubi.c new file +2
-0+ +vfree (tbuf);
Ditto?
common/cmd_ubi.c line 423
Quoted 10 lines
common/cmd_ubi.c new file +10
-0+static int do_ubi (cmd_tbl_t * cmdtp,int flag,int argc,char *argv[]) +{ +size_t size =0 ; +ulong addr =0 ; +int err =0 ; + +if (!ubi_initialized) { + err =ubi_board_scan (); +if (err) { +printf ("ubi initialize error %d\n" , err);
"UBI init error" -- maybe we can decode the error number into some string?
common/cmd_ubi.c line 451
Quoted 6 lines
common/cmd_ubi.c new file +6
-0+/* E.g., create volume size type */ +if (argc ==5 ) { +if (strncmp (argv[4 ],"s" ,1 ) ==0 ) + dynamic =0 ; +else if (strncmp (argv[4 ],"d" ,1 ) !=0 ) { +printf ("You give wrong type\n" );
"Incorrect type". It would also be helpful if the incorrect parameter gets printed, so the user sees what he passed.
common/cmd_ubi.c line 460
Quoted 9 lines
common/cmd_ubi.c new file +9
-0+return 1 ; + } + argc--; + } +/* E.g., create volume size */ +if (argc ==4 ) { + err =parse_num (&size, argv[3 ]); +if (err) { +printf ("You give correct size\n" );
"Incorrect size". Please also print incorrect argument value.
common/cmd_ubi.c line 479
Quoted 3 lines
common/cmd_ubi.c new file +3
-0+if (strncmp (argv[1 ],"write" ,5 ) ==0 ) { +if (argc <5 ) { +printf ("You give wrong parameters\n" );
Print usage message instead.
common/cmd_ubi.c line 486
Quoted 6 lines
common/cmd_ubi.c new file +6
-0+ } + + addr =simple_strtoul (argv[2 ],NULL ,16 ); + err =parse_num (&size, argv[4 ]); +if (err) { +printf ("You give wrong size\n" );
See above.
Review
Quoted 2 lines
(quoted diff) +2
-0+ if (err) { + printf("You give wrong size\n");
Ditto.
common/cmd_ubi.c line 515
Quoted 1 line
common/cmd_ubi.c new file +1
-0+printf ("Unknown UBI command or invalid number of arguments\n" );
Print usage message instead.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de A stone was placed at a ford in a river with the inscription: "When this stone is covered it is dangerous to ford here."
PATCH 1/1 UBI command support
Dear Wolfgang,
Quoted 7 lines · Wolfgang Denk
Dear Kyungmin Park,
Quoted 3 lines · In message <20081021091859.GA29306@july> you wrote:
UBI command support
Signed-off-by Kyungmin Park <[email protected]>...
Quoted 10 lines
common/cmd_ubi.c line 6
Quoted 8 lines
+++
b/common/cmd_ubi.ccommon/cmd_ubi.c new file +6
-0@@ -0,0 +1,535 @@ +/* +* Unsorted Block Image commands +*/ + +#include <common.h> +#include <command.h> GPL header missing.
Okay I added.
Quoted 11 lines
common/cmd_ubi.c line 13
Quoted 2 lines
common/cmd_ubi.c new file +1
-0+/* board/\*\/ubi.c */ What's the '\'s there?
common/cmd_ubi.c line 14
Quoted 1 line
common/cmd_ubi.c new file +1
-0+extern int ubi_board_scan (void );Please move the prototypes to some header file.
common/cmd_ubi.c line 17
Quoted 2 lines
common/cmd_ubi.c new file +2
-0+/* drivers/mtd/ubi/build.c */ +extern struct ubi_device *ubi_devices[];
Moved to proper header files.
Quoted 14 lines
Quoted 14 lines
common/cmd_ubi.c new file +14
-0+ +/* Private own data */ +static struct ubi_device *ubi; +static int ubi_initialized; + +static void ubi_dump_vol_info (const struct ubi_volume *vol) +{ +dbg_msg ("volume information dump:" ); +dbg_msg ("vol_id %d" , vol->vol_id); +dbg_msg ("reserved_pebs %d" , vol->reserved_pebs); +dbg_msg ("alignment %d" , vol->alignment); +dbg_msg ("data_pad %d" , vol->data_pad); +dbg_msg ("vol_type %d" , vol->vol_type); +dbg_msg ("name_len %d" , vol->name_len);
Quoted 8 lines
common/cmd_ubi.c line 37
Quoted 6 lines
common/cmd_ubi.c new file +6
-0+dbg_msg ("usable_leb_size %d" , vol->usable_leb_size); +dbg_msg ("used_ebs %d" , vol->used_ebs); +dbg_msg ("used_bytes %lld" , vol->used_bytes); +dbg_msg ("last_eb_bytes %d" , vol->last_eb_bytes); +dbg_msg ("corrupted %d" , vol->corrupted); +dbg_msg ("upd_marker %d" , vol->upd_marker);Please use the standard
debug()macro.
It's not debug message, it's ubi msg. I fixed.
Quoted 4 lines
common/cmd_ubi.c line 96
Quoted 2 lines
common/cmd_ubi.c new file +1
-0+static int ustrtoul (const char *cp,char **endp,unsigned int base)Maybe we should add this to some global place, like
lib_generic?
Move to lib_generic/vsprintf.c
Quoted 7 lines
common/cmd_ubi.c line 101
Quoted 5 lines
common/cmd_ubi.c new file +4
-0+unsigned long result =simple_strtoul (cp, endp, base); +switch (**endp) { +case 'G' : + result *=1024 ;Please add a
Quoted 8 lines
/* fall through */comment to make clkear that it is intentional not to have a "break;" here.
Review line 37
Quoted 2 lines
(quoted diff) +2
-0+ case 'M': + result *= 1024;Ditto.
Did
Quoted 11 lines
Quoted 4 lines
common/cmd_ubi.c new file +3
-0+static int verify_mkvol_req (const struct ubi_device *ubi, +const struct ubi_mkvol_req *req) +{...
common/cmd_ubi.c line 169
Quoted 4 lines
common/cmd_ubi.c new file +4
-0+bad: +printf ("bad volume creation request" ); +// ubi_dbg_dump_mkvol_req(req); +return err;No C++ comments, please. And please don;t add dead code either.
Yes I remove it
Quoted 9 lines
Quoted 2 lines
+ tbuf =
vmalloc(tbuf_size);Why do we need new
alloc()stuff?common/cmd_ubi.c line 410
Quoted 2 lines
common/cmd_ubi.c new file +2
-0+ +vfree (tbuf);Ditto?
Use malloc & free.
Quoted 52 lines
common/cmd_ubi.c line 423
Quoted 11 lines
common/cmd_ubi.c new file +10
-0+static int do_ubi (cmd_tbl_t * cmdtp,int flag,int argc,char *argv[]) +{ +size_t size =0 ; +ulong addr =0 ; +int err =0 ; + +if (!ubi_initialized) { + err =ubi_board_scan (); +if (err) { +printf ("ubi initialize error %d\n" , err);"UBI init error" -- maybe we can decode the error number into some string?
common/cmd_ubi.c line 451
Quoted 6 lines
common/cmd_ubi.c new file +6
-0+/* E.g., create volume size type */ +if (argc ==5 ) { +if (strncmp (argv[4 ],"s" ,1 ) ==0 ) + dynamic =0 ; +else if (strncmp (argv[4 ],"d" ,1 ) !=0 ) { +printf ("You give wrong type\n" );"Incorrect type". It would also be helpful if the incorrect parameter gets printed, so the user sees what he passed.
common/cmd_ubi.c line 460
Quoted 9 lines
common/cmd_ubi.c new file +9
-0+return 1 ; + } + argc--; + } +/* E.g., create volume size */ +if (argc ==4 ) { + err =parse_num (&size, argv[3 ]); +if (err) { +printf ("You give correct size\n" );"Incorrect size". Please also print incorrect argument value.
common/cmd_ubi.c line 479
Quoted 3 lines
common/cmd_ubi.c new file +3
-0+if (strncmp (argv[1 ],"write" ,5 ) ==0 ) { +if (argc <5 ) { +printf ("You give wrong parameters\n" );Print usage message instead.
common/cmd_ubi.c line 486
Quoted 6 lines
common/cmd_ubi.c new file +6
-0+ } + + addr =simple_strtoul (argv[2 ],NULL ,16 ); + err =parse_num (&size, argv[4 ]); +if (err) { +printf ("You give wrong size\n" );See above.
Quoted 9 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ if (err) { + printf("You give wrong size\n");Ditto.
Quoted 1 line
+
printf("Unknown UBI command or invalid number of arguments\n");Print usage message instead.
How to display usage?
Thank you,
Kyungmin Park
PATCH 1/1 UBI command support
Dear Kyungmin Park,
In message <[email protected]> you wrote:
...
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
+
printf("Unknown UBI command or invalid number of arguments\n");Print usage message instead.
How to display usage?
printf ("Usage:\n%s\n", cmdtp->usage);Best regards,
Wolfgang Denk
Signature · 7 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Misquotation is, in fact, the pride and privilege of the learned. A
widely-read man never quotes accurately, for the rather obvious
reason that he has read too widely.
- Hesketh Pearson _Common Misquotations_ introduction