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.
+/* 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[];
+/* 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.
+static int ustrtoul(const char *cp, char **endp, unsigned int base)
Maybe we should add this to some global place, like lib_generic ?
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.
+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.
- tbuf = vmalloc(tbuf_size);
Why do we need new alloc() stuff?
- vfree(tbuf);
Ditto?
+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.
Best regards,
Wolfgang Denk