Dear Wolfgang,
On Tue, Oct 21, 2008 at 7:24 PM, Wolfgang Denk wd@denx.de wrote:
Dear Kyungmin Park,
In message 20081021091859.GA29306@july you wrote:
UBI command support
Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
...
+++ b/common/cmd_ubi.c @@ -0,0 +1,535 @@ +/*
- Unsorted Block Image commands
- */
+#include <common.h> +#include <command.h>
GPL header missing.
Okay I added.
+/* board/*/ubi.c */
What's the ''s there?
+extern int ubi_board_scan(void);
Please move the prototypes to some header file.
+/* drivers/mtd/ubi/build.c */ +extern struct ubi_device *ubi_devices[];
Moved to proper header files.
+/* 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);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.
+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
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.
case 'M':result *= 1024;Ditto.
Did
+static int verify_mkvol_req(const struct ubi_device *ubi,
const struct ubi_mkvol_req *req)+{
...
+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
tbuf = vmalloc(tbuf_size);Why do we need new alloc() stuff?
vfree(tbuf);Ditto?
Use malloc & free.
+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?
/* 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.
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.
if (strncmp(argv[1], "write", 5) == 0) {if (argc < 5) {printf("You give wrong parameters\n");Print usage message instead.
}addr = simple_strtoul(argv[2], NULL, 16);err = parse_num(&size, argv[4]);if (err) {printf("You give wrong size\n");See above.
if (err) {printf("You give wrong size\n");Ditto.
printf("Unknown UBI command or invalid number of arguments\n");Print usage message instead.
How to display usage?
Thank you, Kyungmin Park