Dear Kyungmin Park,
In message <[EMAIL PROTECTED]> you wrote:
> UBI command support
>
> Signed-off-by: Kyungmin Park <[EMAIL PROTECTED]>
...
> +++ 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
--
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: [EMAIL PROTECTED]
A stone was placed at a ford in a river with the inscription:
"When this stone is covered it is dangerous to ford here."
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
