| Message ID | 1409895536-18092-3-git-send-email-hs@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi Heiko Schocher, Nice pick - On 5 September 2014 11:08, Heiko Schocher <hs@denx.de> wrote: > move common functions from cmd_nand.c (for calculating offset > and size from cmdline paramter) to common place, so they could > used from other commands which use mtd partitions. > > For onenand the arg_off_size() is left in common/cmd_onenand.c. > It should use now the common arg_off() function, but as I could > not test onenand I let it there ... > > Signed-off-by: Heiko Schocher <hs@denx.de> > Cc: Scott Wood <scottwood@freescale.com> > Cc: Tom Rini <trini@ti.com> > > --- > - changes for v2: > none > - changes for v3: > - add comments from scott wood: > - align MTD_DEV_TYPE_NAND correct > - remove unnecessary inline > - rework "jffs2 header" problem later > - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6 > --- > common/cmd_nand.c | 140 +++++++++--------------------------------------- > common/cmd_onenand.c | 19 +++---- > drivers/mtd/Makefile | 4 +- > drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/mtd.h | 7 +++ > 5 files changed, 154 insertions(+), 130 deletions(-) > create mode 100644 drivers/mtd/mtd_uboot.c > > diff --git a/common/cmd_nand.c b/common/cmd_nand.c > index f9ced9d..099ba00 100644 > --- a/common/cmd_nand.c > +++ b/common/cmd_nand.c > @@ -133,115 +133,6 @@ static int set_dev(int dev) > return 0; > } > > -static inline int str2off(const char *p, loff_t *num) > -{ > - char *endptr; > - > - *num = simple_strtoull(p, &endptr, 16); > - return *p != '\0' && *endptr == '\0'; > -} > - > -static inline int str2long(const char *p, ulong *num) > -{ > - char *endptr; > - > - *num = simple_strtoul(p, &endptr, 16); > - return *p != '\0' && *endptr == '\0'; > -} > - > -static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size, > - loff_t *maxsize) > -{ > -#ifdef CONFIG_CMD_MTDPARTS > - struct mtd_device *dev; > - struct part_info *part; > - u8 pnum; > - int ret; > - > - ret = mtdparts_init(); > - if (ret) > - return ret; > - > - ret = find_dev_and_part(partname, &dev, &pnum, &part); > - if (ret) > - return ret; > - > - if (dev->id->type != MTD_DEV_TYPE_NAND) { > - puts("not a NAND device\n"); > - return -1; > - } > - > - *off = part->offset; > - *size = part->size; > - *maxsize = part->size; > - *idx = dev->id->num; > - > - ret = set_dev(*idx); > - if (ret) > - return ret; > - > - return 0; > -#else > - puts("offset is not a number\n"); > - return -1; > -#endif > -} > - > -static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size, > - loff_t *maxsize) > -{ > - if (!str2off(arg, off)) > - return get_part(arg, idx, off, size, maxsize); > - > - if (*off >= nand_info[*idx].size) { > - puts("Offset exceeds device limit\n"); > - return -1; > - } > - > - *maxsize = nand_info[*idx].size - *off; > - *size = *maxsize; > - return 0; > -} > - > -static int arg_off_size(int argc, char *const argv[], int *idx, > - loff_t *off, loff_t *size, loff_t *maxsize) > -{ > - int ret; > - > - if (argc == 0) { > - *off = 0; > - *size = nand_info[*idx].size; > - *maxsize = *size; > - goto print; > - } > - > - ret = arg_off(argv[0], idx, off, size, maxsize); > - if (ret) > - return ret; > - > - if (argc == 1) > - goto print; > - > - if (!str2off(argv[1], size)) { > - printf("'%s' is not a number\n", argv[1]); > - return -1; > - } > - > - if (*size > *maxsize) { > - puts("Size exceeds partition or device limit\n"); > - return -1; > - } > - > -print: > - printf("device %d ", *idx); > - if (*size == nand_info[*idx].size) > - puts("whole chip\n"); > - else > - printf("offset 0x%llx, size 0x%llx\n", > - (unsigned long long)*off, (unsigned long long)*size); > - return 0; > -} > - > #ifdef CONFIG_CMD_NAND_LOCK_UNLOCK > static void print_status(ulong start, ulong end, ulong erasesize, int status) > { > @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[]) > goto usage; > > /* We don't care about size, or maxsize. */ > - if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) { > + if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize, > + MTD_DEV_TYPE_NAND, nand_info[idx].size)) { > + puts("Offset or partition name expected\n"); > + return 1; > + } > + if (set_dev(idx)) { > puts("Offset or partition name expected\n"); > return 1; > } > @@ -592,7 +488,10 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > printf("\nNAND %s: ", cmd); > /* skip first two or three arguments, look for offset and size */ > if (arg_off_size(argc - o, argv + o, &dev, &off, &size, > - &maxsize) != 0) > + &maxsize, MTD_DEV_TYPE_NAND, nand_info[dev].size) != 0) > + return 1; > + > + if (set_dev(dev)) > return 1; > > nand = &nand_info[dev]; > @@ -654,7 +553,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > if (s && !strcmp(s, ".raw")) { > raw = 1; > > - if (arg_off(argv[3], &dev, &off, &size, &maxsize)) > + if (arg_off(argv[3], &dev, &off, &size, &maxsize, > + MTD_DEV_TYPE_NAND, nand_info[dev].size)) > + return 1; > + > + if (set_dev(dev)) > return 1; > > if (argc > 4 && !str2long(argv[4], &pagecount)) { > @@ -669,8 +572,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > rwsize = pagecount * (nand->writesize + nand->oobsize); > } else { > - if (arg_off_size(argc - 3, argv + 3, &dev, > - &off, &size, &maxsize) != 0) > + if (arg_off_size(argc - 3, argv + 3, &dev, &off, &size, > + &maxsize, MTD_DEV_TYPE_NAND, > + nand_info[dev].size) != 0) > + return 1; > + > + if (set_dev(dev)) > return 1; > > /* size is unspecified */ > @@ -816,7 +723,10 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > allexcept = 1; > > if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size, > - &maxsize) < 0) > + &maxsize, MTD_DEV_TYPE_NAND, nand_info[dev].size) < 0) > + return 1; > + > + if (set_dev(dev)) > return 1; > > if (!nand_unlock(&nand_info[dev], off, size, allexcept)) { > diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c > index 06cc140..feab01a 100644 > --- a/common/cmd_onenand.c > +++ b/common/cmd_onenand.c > @@ -24,15 +24,8 @@ static struct mtd_info *mtd; > static loff_t next_ofs; > static loff_t skip_ofs; > > -static inline int str2long(char *p, ulong *num) > -{ > - char *endptr; > - > - *num = simple_strtoul(p, &endptr, 16); > - return (*p != '\0' && *endptr == '\0') ? 1 : 0; > -} > - > -static int arg_off_size(int argc, char * const argv[], ulong *off, size_t *size) > +static int arg_off_size_onenand(int argc, char * const argv[], ulong *off, > + size_t *size) > { > if (argc >= 1) { > if (!(str2long(argv[0], off))) { > @@ -399,7 +392,7 @@ static int do_onenand_read(cmd_tbl_t * cmdtp, int flag, int argc, char * const a > addr = (ulong)simple_strtoul(argv[1], NULL, 16); > > printf("\nOneNAND read: "); > - if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0) > + if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0) > return 1; > > ret = onenand_block_read(ofs, len, &retlen, (u8 *)addr, oob); > @@ -425,7 +418,7 @@ static int do_onenand_write(cmd_tbl_t * cmdtp, int flag, int argc, char * const > addr = (ulong)simple_strtoul(argv[1], NULL, 16); > > printf("\nOneNAND write: "); > - if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0) > + if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0) Is this a new function call arg_off_size_onenand again, i guess it's common call arg_off_size() Please check? > return 1; > > ret = onenand_block_write(ofs, len, &retlen, (u8 *)addr, withoob); > @@ -461,7 +454,7 @@ static int do_onenand_erase(cmd_tbl_t * cmdtp, int flag, int argc, char * const > printf("\nOneNAND erase: "); > > /* skip first two or three arguments, look for offset and size */ > - if (arg_off_size(argc, argv, &ofs, &len) != 0) > + if (arg_off_size_onenand(argc, argv, &ofs, &len) != 0) > return 1; > > ret = onenand_block_erase(ofs, len, force); > @@ -486,7 +479,7 @@ static int do_onenand_test(cmd_tbl_t * cmdtp, int flag, int argc, char * const a > printf("\nOneNAND test: "); > > /* skip first two or three arguments, look for offset and size */ > - if (arg_off_size(argc - 1, argv + 1, &ofs, &len) != 0) > + if (arg_off_size_onenand(argc - 1, argv + 1, &ofs, &len) != 0) > return 1; > > ret = onenand_block_test(ofs, len); > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile > index 5467a951..a623f4c 100644 > --- a/drivers/mtd/Makefile > +++ b/drivers/mtd/Makefile > @@ -5,8 +5,8 @@ > # SPDX-License-Identifier: GPL-2.0+ > # > > -ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND))) > -obj-y += mtdcore.o > +ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF))) > +obj-y += mtdcore.o mtd_uboot.o I'm thinking its better to be this file in common instead of drivers/mtd because this more reusable code for command stuff instead of mtd core. And name something like mtd_common or make sense to appear command's usage instead of main mtd stuff, IMHO. > endif > obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o > obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o > diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c > new file mode 100644 > index 0000000..a70d40a > --- /dev/null > +++ b/drivers/mtd/mtd_uboot.c > @@ -0,0 +1,114 @@ > +/* > + * (C) Copyright 2014 > + * Heiko Schocher, DENX Software Engineering, hs@denx.de. > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > +#include <common.h> > +#include <linux/mtd/mtd.h> > +#include <jffs2/jffs2.h> > + > +int str2off(const char *p, loff_t *num) > +{ > + char *endptr; > + > + *num = simple_strtoull(p, &endptr, 16); > + return *p != '\0' && *endptr == '\0'; > +} > + > +int str2long(const char *p, ulong *num) > +{ > + char *endptr; > + > + *num = simple_strtoul(p, &endptr, 16); > + return *p != '\0' && *endptr == '\0'; > +} > + > +static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size, > + loff_t *maxsize, int devtype) > +{ > +#ifdef CONFIG_CMD_MTDPARTS > + struct mtd_device *dev; > + struct part_info *part; > + u8 pnum; > + int ret; > + > + ret = mtdparts_init(); > + if (ret) > + return ret; > + > + ret = find_dev_and_part(partname, &dev, &pnum, &part); > + if (ret) > + return ret; > + > + if (dev->id->type != devtype) { > + printf("not same typ %d != %d\n", dev->id->type, devtype); > + return -1; > + } > + > + *off = part->offset; > + *size = part->size; > + *maxsize = part->size; > + *idx = dev->id->num; > + > + return 0; > +#else > + puts("offset is not a number\n"); > + return -1; > +#endif > +} > + > +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size, > + loff_t *maxsize, int devtype, int chipsize) > +{ > + if (!str2off(arg, off)) > + return get_part(arg, idx, off, size, maxsize, devtype); > + > + if (*off >= chipsize) { > + puts("Offset exceeds device limit\n"); > + return -1; > + } > + > + *maxsize = chipsize - *off; > + *size = *maxsize; > + return 0; > +} > + > +int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off, > + loff_t *size, loff_t *maxsize, int devtype, int chipsize) > +{ > + int ret; > + > + if (argc == 0) { > + *off = 0; > + *size = chipsize; > + *maxsize = *size; > + goto print; > + } > + > + ret = arg_off(argv[0], idx, off, size, maxsize, devtype, chipsize); > + if (ret) > + return ret; > + > + if (argc == 1) > + goto print; > + > + if (!str2off(argv[1], size)) { > + printf("'%s' is not a number\n", argv[1]); > + return -1; > + } > + > + if (*size > *maxsize) { > + puts("Size exceeds partition or device limit\n"); > + return -1; > + } > + > +print: > + printf("device %d ", *idx); > + if (*size == chipsize) > + puts("whole chip\n"); > + else > + printf("offset 0x%llx, size 0x%llx\n", > + (unsigned long long)*off, (unsigned long long)*size); > + return 0; > +} > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > index 1526d07..423c346 100644 > --- a/include/linux/mtd/mtd.h > +++ b/include/linux/mtd/mtd.h > @@ -483,5 +483,12 @@ int add_mtd_device(struct mtd_info *mtd); > int del_mtd_device(struct mtd_info *mtd); > int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int); > int del_mtd_partitions(struct mtd_info *); > + > +int str2off(const char *p, loff_t *num); > +int str2long(const char *p, ulong *num); > +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size, > + loff_t *maxsize, int devtype, int chipsize); > +int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off, > + loff_t *size, loff_t *maxsize, int devtype, int chipsize); > #endif > #endif /* __MTD_MTD_H__ */ > -- > 1.8.3.1 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot thanks!
Hello Jagan, Am 04.11.2014 21:55, schrieb Jagan Teki: > Hi Heiko Schocher, > > Nice pick - > > On 5 September 2014 11:08, Heiko Schocher<hs@denx.de> wrote: >> move common functions from cmd_nand.c (for calculating offset >> and size from cmdline paramter) to common place, so they could >> used from other commands which use mtd partitions. >> >> For onenand the arg_off_size() is left in common/cmd_onenand.c. >> It should use now the common arg_off() function, but as I could >> not test onenand I let it there ... >> >> Signed-off-by: Heiko Schocher<hs@denx.de> >> Cc: Scott Wood<scottwood@freescale.com> >> Cc: Tom Rini<trini@ti.com> >> >> --- >> - changes for v2: >> none >> - changes for v3: >> - add comments from scott wood: >> - align MTD_DEV_TYPE_NAND correct >> - remove unnecessary inline >> - rework "jffs2 header" problem later >> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6 >> --- >> common/cmd_nand.c | 140 +++++++++--------------------------------------- >> common/cmd_onenand.c | 19 +++---- >> drivers/mtd/Makefile | 4 +- >> drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++ >> include/linux/mtd/mtd.h | 7 +++ >> 5 files changed, 154 insertions(+), 130 deletions(-) >> create mode 100644 drivers/mtd/mtd_uboot.c >> >> diff --git a/common/cmd_nand.c b/common/cmd_nand.c >> index f9ced9d..099ba00 100644 >> --- a/common/cmd_nand.c >> +++ b/common/cmd_nand.c >> @@ -133,115 +133,6 @@ static int set_dev(int dev) >> return 0; >> } >> >> -static inline int str2off(const char *p, loff_t *num) >> -{ >> - char *endptr; >> - >> - *num = simple_strtoull(p,&endptr, 16); >> - return *p != '\0'&& *endptr == '\0'; >> -} >> - >> -static inline int str2long(const char *p, ulong *num) >> -{ >> - char *endptr; >> - >> - *num = simple_strtoul(p,&endptr, 16); >> - return *p != '\0'&& *endptr == '\0'; >> -} >> - >> -static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size, >> - loff_t *maxsize) >> -{ >> -#ifdef CONFIG_CMD_MTDPARTS >> - struct mtd_device *dev; >> - struct part_info *part; >> - u8 pnum; >> - int ret; >> - >> - ret = mtdparts_init(); >> - if (ret) >> - return ret; >> - >> - ret = find_dev_and_part(partname,&dev,&pnum,&part); >> - if (ret) >> - return ret; >> - >> - if (dev->id->type != MTD_DEV_TYPE_NAND) { >> - puts("not a NAND device\n"); >> - return -1; >> - } >> - >> - *off = part->offset; >> - *size = part->size; >> - *maxsize = part->size; >> - *idx = dev->id->num; >> - >> - ret = set_dev(*idx); >> - if (ret) >> - return ret; >> - >> - return 0; >> -#else >> - puts("offset is not a number\n"); >> - return -1; >> -#endif >> -} >> - >> -static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size, >> - loff_t *maxsize) >> -{ >> - if (!str2off(arg, off)) >> - return get_part(arg, idx, off, size, maxsize); >> - >> - if (*off>= nand_info[*idx].size) { >> - puts("Offset exceeds device limit\n"); >> - return -1; >> - } >> - >> - *maxsize = nand_info[*idx].size - *off; >> - *size = *maxsize; >> - return 0; >> -} >> - >> -static int arg_off_size(int argc, char *const argv[], int *idx, >> - loff_t *off, loff_t *size, loff_t *maxsize) >> -{ >> - int ret; >> - >> - if (argc == 0) { >> - *off = 0; >> - *size = nand_info[*idx].size; >> - *maxsize = *size; >> - goto print; >> - } >> - >> - ret = arg_off(argv[0], idx, off, size, maxsize); >> - if (ret) >> - return ret; >> - >> - if (argc == 1) >> - goto print; >> - >> - if (!str2off(argv[1], size)) { >> - printf("'%s' is not a number\n", argv[1]); >> - return -1; >> - } >> - >> - if (*size> *maxsize) { >> - puts("Size exceeds partition or device limit\n"); >> - return -1; >> - } >> - >> -print: >> - printf("device %d ", *idx); >> - if (*size == nand_info[*idx].size) >> - puts("whole chip\n"); >> - else >> - printf("offset 0x%llx, size 0x%llx\n", >> - (unsigned long long)*off, (unsigned long long)*size); >> - return 0; >> -} >> - >> #ifdef CONFIG_CMD_NAND_LOCK_UNLOCK >> static void print_status(ulong start, ulong end, ulong erasesize, int status) >> { >> @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[]) >> goto usage; >> >> /* We don't care about size, or maxsize. */ >> - if (arg_off(argv[2],&idx,&addr,&maxsize,&maxsize)) { >> + if (arg_off(argv[2],&idx,&addr,&maxsize,&maxsize, >> + MTD_DEV_TYPE_NAND, nand_info[idx].size)) { >> + puts("Offset or partition name expected\n"); >> + return 1; >> + } >> + if (set_dev(idx)) { >> puts("Offset or partition name expected\n"); >> return 1; >> } >> @@ -592,7 +488,10 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> printf("\nNAND %s: ", cmd); >> /* skip first two or three arguments, look for offset and size */ >> if (arg_off_size(argc - o, argv + o,&dev,&off,&size, >> -&maxsize) != 0) >> +&maxsize, MTD_DEV_TYPE_NAND, nand_info[dev].size) != 0) >> + return 1; >> + >> + if (set_dev(dev)) >> return 1; >> >> nand =&nand_info[dev]; >> @@ -654,7 +553,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> if (s&& !strcmp(s, ".raw")) { >> raw = 1; >> >> - if (arg_off(argv[3],&dev,&off,&size,&maxsize)) >> + if (arg_off(argv[3],&dev,&off,&size,&maxsize, >> + MTD_DEV_TYPE_NAND, nand_info[dev].size)) >> + return 1; >> + >> + if (set_dev(dev)) >> return 1; >> >> if (argc> 4&& !str2long(argv[4],&pagecount)) { >> @@ -669,8 +572,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> >> rwsize = pagecount * (nand->writesize + nand->oobsize); >> } else { >> - if (arg_off_size(argc - 3, argv + 3,&dev, >> -&off,&size,&maxsize) != 0) >> + if (arg_off_size(argc - 3, argv + 3,&dev,&off,&size, >> +&maxsize, MTD_DEV_TYPE_NAND, >> + nand_info[dev].size) != 0) >> + return 1; >> + >> + if (set_dev(dev)) >> return 1; >> >> /* size is unspecified */ >> @@ -816,7 +723,10 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> allexcept = 1; >> >> if (arg_off_size(argc - 2, argv + 2,&dev,&off,&size, >> -&maxsize)< 0) >> +&maxsize, MTD_DEV_TYPE_NAND, nand_info[dev].size)< 0) >> + return 1; >> + >> + if (set_dev(dev)) >> return 1; >> >> if (!nand_unlock(&nand_info[dev], off, size, allexcept)) { >> diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c >> index 06cc140..feab01a 100644 >> --- a/common/cmd_onenand.c >> +++ b/common/cmd_onenand.c >> @@ -24,15 +24,8 @@ static struct mtd_info *mtd; >> static loff_t next_ofs; >> static loff_t skip_ofs; >> >> -static inline int str2long(char *p, ulong *num) >> -{ >> - char *endptr; >> - >> - *num = simple_strtoul(p,&endptr, 16); >> - return (*p != '\0'&& *endptr == '\0') ? 1 : 0; >> -} >> - >> -static int arg_off_size(int argc, char * const argv[], ulong *off, size_t *size) >> +static int arg_off_size_onenand(int argc, char * const argv[], ulong *off, >> + size_t *size) >> { >> if (argc>= 1) { >> if (!(str2long(argv[0], off))) { >> @@ -399,7 +392,7 @@ static int do_onenand_read(cmd_tbl_t * cmdtp, int flag, int argc, char * const a >> addr = (ulong)simple_strtoul(argv[1], NULL, 16); >> >> printf("\nOneNAND read: "); >> - if (arg_off_size(argc - 2, argv + 2,&ofs,&len) != 0) >> + if (arg_off_size_onenand(argc - 2, argv + 2,&ofs,&len) != 0) >> return 1; >> >> ret = onenand_block_read(ofs, len,&retlen, (u8 *)addr, oob); >> @@ -425,7 +418,7 @@ static int do_onenand_write(cmd_tbl_t * cmdtp, int flag, int argc, char * const >> addr = (ulong)simple_strtoul(argv[1], NULL, 16); >> >> printf("\nOneNAND write: "); >> - if (arg_off_size(argc - 2, argv + 2,&ofs,&len) != 0) >> + if (arg_off_size_onenand(argc - 2, argv + 2,&ofs,&len) != 0) > > Is this a new function call arg_off_size_onenand again, i guess it's > common call arg_off_size() > Please check? The arg_off_size() differs from the arg_off_size_onenand(), thats the reason why I let the "arg_off_size_onenand()" in ./common/cmd_onenand.c I have no board with onenand availiable for testing, so it is to risky to me, to change the code in ./common/cmd_onenand.c This should be done from someone, which can really test it! >> return 1; >> >> ret = onenand_block_write(ofs, len,&retlen, (u8 *)addr, withoob); >> @@ -461,7 +454,7 @@ static int do_onenand_erase(cmd_tbl_t * cmdtp, int flag, int argc, char * const >> printf("\nOneNAND erase: "); >> >> /* skip first two or three arguments, look for offset and size */ >> - if (arg_off_size(argc, argv,&ofs,&len) != 0) >> + if (arg_off_size_onenand(argc, argv,&ofs,&len) != 0) >> return 1; >> >> ret = onenand_block_erase(ofs, len, force); >> @@ -486,7 +479,7 @@ static int do_onenand_test(cmd_tbl_t * cmdtp, int flag, int argc, char * const a >> printf("\nOneNAND test: "); >> >> /* skip first two or three arguments, look for offset and size */ >> - if (arg_off_size(argc - 1, argv + 1,&ofs,&len) != 0) >> + if (arg_off_size_onenand(argc - 1, argv + 1,&ofs,&len) != 0) >> return 1; >> >> ret = onenand_block_test(ofs, len); >> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile >> index 5467a951..a623f4c 100644 >> --- a/drivers/mtd/Makefile >> +++ b/drivers/mtd/Makefile >> @@ -5,8 +5,8 @@ >> # SPDX-License-Identifier: GPL-2.0+ >> # >> >> -ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND))) >> -obj-y += mtdcore.o >> +ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF))) >> +obj-y += mtdcore.o mtd_uboot.o > > I'm thinking its better to be this file in common instead of > drivers/mtd because this more reusable code > for command stuff instead of mtd core. > > And name something like mtd_common or make sense to appear command's > usage instead of > main mtd stuff, IMHO. Currently this is only possible on mtd partitions ... for example: if ((*off + *size) > mtd->size) { printf("total chip size (0x%llx) exceeded!\n", mtd->size); return -1; } if (*size == mtd->size) puts("whole chip\n"); This works only for mtd devices ... so it should be in drivers/mtd ... the name of the file ... Hmm... I have no real preference ... "mtd_uboot.c" "mtd_common.c" "mtd_uboot_common.c" What do others think? [...] bye, Heiko
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index f9ced9d..099ba00 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -133,115 +133,6 @@ static int set_dev(int dev) return 0; } -static inline int str2off(const char *p, loff_t *num) -{ - char *endptr; - - *num = simple_strtoull(p, &endptr, 16); - return *p != '\0' && *endptr == '\0'; -} - -static inline int str2long(const char *p, ulong *num) -{ - char *endptr; - - *num = simple_strtoul(p, &endptr, 16); - return *p != '\0' && *endptr == '\0'; -} - -static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size, - loff_t *maxsize) -{ -#ifdef CONFIG_CMD_MTDPARTS - struct mtd_device *dev; - struct part_info *part; - u8 pnum; - int ret; - - ret = mtdparts_init(); - if (ret) - return ret; - - ret = find_dev_and_part(partname, &dev, &pnum, &part); - if (ret) - return ret; - - if (dev->id->type != MTD_DEV_TYPE_NAND) { - puts("not a NAND device\n"); - return -1; - } - - *off = part->offset; - *size = part->size; - *maxsize = part->size; - *idx = dev->id->num; - - ret = set_dev(*idx); - if (ret) - return ret; - - return 0; -#else - puts("offset is not a number\n"); - return -1; -#endif -} - -static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size, - loff_t *maxsize) -{ - if (!str2off(arg, off)) - return get_part(arg, idx, off, size, maxsize); - - if (*off >= nand_info[*idx].size) { - puts("Offset exceeds device limit\n"); - return -1; - } - - *maxsize = nand_info[*idx].size - *off; - *size = *maxsize; - return 0; -} - -static int arg_off_size(int argc, char *const argv[], int *idx, - loff_t *off, loff_t *size, loff_t *maxsize) -{ - int ret; - - if (argc == 0) { - *off = 0; - *size = nand_info[*idx].size; - *maxsize = *size; - goto print; - } - - ret = arg_off(argv[0], idx, off, size, maxsize); - if (ret) - return ret; - - if (argc == 1) - goto print; - - if (!str2off(argv[1], size)) { - printf("'%s' is not a number\n", argv[1]); - return -1; - } - - if (*size > *maxsize) { - puts("Size exceeds partition or device limit\n"); - return -1; - } - -print: - printf("device %d ", *idx); - if (*size == nand_info[*idx].size) - puts("whole chip\n"); - else - printf("offset 0x%llx, size 0x%llx\n", - (unsigned long long)*off, (unsigned long long)*size); - return 0; -} - #ifdef CONFIG_CMD_NAND_LOCK_UNLOCK static void print_status(ulong start, ulong end, ulong erasesize, int status) { @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[]) goto usage; /* We don't care about size, or maxsize. */ - if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) { + if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize, + MTD_DEV_TYPE_NAND, nand_info[idx].size)) { + puts("Offset or partition name expected\n"); + return 1; + } + if (set_dev(idx)) { puts("Offset or partition name expected\n"); return 1; } @@ -592,7 +488,10 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) printf("\nNAND %s: ", cmd); /* skip first two or three arguments, look for offset and size */ if (arg_off_size(argc - o, argv + o, &dev, &off, &size, - &maxsize) != 0) + &maxsize, MTD_DEV_TYPE_NAND, nand_info[dev].size) != 0) + return 1; + + if (set_dev(dev)) return 1; nand = &nand_info[dev]; @@ -654,7 +553,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (s && !strcmp(s, ".raw")) { raw = 1; - if (arg_off(argv[3], &dev, &off, &size, &maxsize)) + if (arg_off(argv[3], &dev, &off, &size, &maxsize, + MTD_DEV_TYPE_NAND, nand_info[dev].size)) + return 1; + + if (set_dev(dev)) return 1; if (argc > 4 && !str2long(argv[4], &pagecount)) { @@ -669,8 +572,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) rwsize = pagecount * (nand->writesize + nand->oobsize); } else { - if (arg_off_size(argc - 3, argv + 3, &dev, - &off, &size, &maxsize) != 0) + if (arg_off_size(argc - 3, argv + 3, &dev, &off, &size, + &maxsize, MTD_DEV_TYPE_NAND, + nand_info[dev].size) != 0) + return 1; + + if (set_dev(dev)) return 1; /* size is unspecified */ @@ -816,7 +723,10 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) allexcept = 1; if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size, - &maxsize) < 0) + &maxsize, MTD_DEV_TYPE_NAND, nand_info[dev].size) < 0) + return 1; + + if (set_dev(dev)) return 1; if (!nand_unlock(&nand_info[dev], off, size, allexcept)) { diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 06cc140..feab01a 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -24,15 +24,8 @@ static struct mtd_info *mtd; static loff_t next_ofs; static loff_t skip_ofs; -static inline int str2long(char *p, ulong *num) -{ - char *endptr; - - *num = simple_strtoul(p, &endptr, 16); - return (*p != '\0' && *endptr == '\0') ? 1 : 0; -} - -static int arg_off_size(int argc, char * const argv[], ulong *off, size_t *size) +static int arg_off_size_onenand(int argc, char * const argv[], ulong *off, + size_t *size) { if (argc >= 1) { if (!(str2long(argv[0], off))) { @@ -399,7 +392,7 @@ static int do_onenand_read(cmd_tbl_t * cmdtp, int flag, int argc, char * const a addr = (ulong)simple_strtoul(argv[1], NULL, 16); printf("\nOneNAND read: "); - if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0) + if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0) return 1; ret = onenand_block_read(ofs, len, &retlen, (u8 *)addr, oob); @@ -425,7 +418,7 @@ static int do_onenand_write(cmd_tbl_t * cmdtp, int flag, int argc, char * const addr = (ulong)simple_strtoul(argv[1], NULL, 16); printf("\nOneNAND write: "); - if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0) + if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0) return 1; ret = onenand_block_write(ofs, len, &retlen, (u8 *)addr, withoob); @@ -461,7 +454,7 @@ static int do_onenand_erase(cmd_tbl_t * cmdtp, int flag, int argc, char * const printf("\nOneNAND erase: "); /* skip first two or three arguments, look for offset and size */ - if (arg_off_size(argc, argv, &ofs, &len) != 0) + if (arg_off_size_onenand(argc, argv, &ofs, &len) != 0) return 1; ret = onenand_block_erase(ofs, len, force); @@ -486,7 +479,7 @@ static int do_onenand_test(cmd_tbl_t * cmdtp, int flag, int argc, char * const a printf("\nOneNAND test: "); /* skip first two or three arguments, look for offset and size */ - if (arg_off_size(argc - 1, argv + 1, &ofs, &len) != 0) + if (arg_off_size_onenand(argc - 1, argv + 1, &ofs, &len) != 0) return 1; ret = onenand_block_test(ofs, len); diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile index 5467a951..a623f4c 100644 --- a/drivers/mtd/Makefile +++ b/drivers/mtd/Makefile @@ -5,8 +5,8 @@ # SPDX-License-Identifier: GPL-2.0+ # -ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND))) -obj-y += mtdcore.o +ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF))) +obj-y += mtdcore.o mtd_uboot.o endif obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c new file mode 100644 index 0000000..a70d40a --- /dev/null +++ b/drivers/mtd/mtd_uboot.c @@ -0,0 +1,114 @@ +/* + * (C) Copyright 2014 + * Heiko Schocher, DENX Software Engineering, hs@denx.de. + * + * SPDX-License-Identifier: GPL-2.0+ + */ +#include <common.h> +#include <linux/mtd/mtd.h> +#include <jffs2/jffs2.h> + +int str2off(const char *p, loff_t *num) +{ + char *endptr; + + *num = simple_strtoull(p, &endptr, 16); + return *p != '\0' && *endptr == '\0'; +} + +int str2long(const char *p, ulong *num) +{ + char *endptr; + + *num = simple_strtoul(p, &endptr, 16); + return *p != '\0' && *endptr == '\0'; +} + +static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size, + loff_t *maxsize, int devtype) +{ +#ifdef CONFIG_CMD_MTDPARTS + struct mtd_device *dev; + struct part_info *part; + u8 pnum; + int ret; + + ret = mtdparts_init(); + if (ret) + return ret; + + ret = find_dev_and_part(partname, &dev, &pnum, &part); + if (ret) + return ret; + + if (dev->id->type != devtype) { + printf("not same typ %d != %d\n", dev->id->type, devtype); + return -1; + } + + *off = part->offset; + *size = part->size; + *maxsize = part->size; + *idx = dev->id->num; + + return 0; +#else + puts("offset is not a number\n"); + return -1; +#endif +} + +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size, + loff_t *maxsize, int devtype, int chipsize) +{ + if (!str2off(arg, off)) + return get_part(arg, idx, off, size, maxsize, devtype); + + if (*off >= chipsize) { + puts("Offset exceeds device limit\n"); + return -1; + } + + *maxsize = chipsize - *off; + *size = *maxsize; + return 0; +} + +int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off, + loff_t *size, loff_t *maxsize, int devtype, int chipsize) +{ + int ret; + + if (argc == 0) { + *off = 0; + *size = chipsize; + *maxsize = *size; + goto print; + } + + ret = arg_off(argv[0], idx, off, size, maxsize, devtype, chipsize); + if (ret) + return ret; + + if (argc == 1) + goto print; + + if (!str2off(argv[1], size)) { + printf("'%s' is not a number\n", argv[1]); + return -1; + } + + if (*size > *maxsize) { + puts("Size exceeds partition or device limit\n"); + return -1; + } + +print: + printf("device %d ", *idx); + if (*size == chipsize) + puts("whole chip\n"); + else + printf("offset 0x%llx, size 0x%llx\n", + (unsigned long long)*off, (unsigned long long)*size); + return 0; +} diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 1526d07..423c346 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -483,5 +483,12 @@ int add_mtd_device(struct mtd_info *mtd); int del_mtd_device(struct mtd_info *mtd); int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int); int del_mtd_partitions(struct mtd_info *); + +int str2off(const char *p, loff_t *num); +int str2long(const char *p, ulong *num); +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size, + loff_t *maxsize, int devtype, int chipsize); +int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off, + loff_t *size, loff_t *maxsize, int devtype, int chipsize); #endif #endif /* __MTD_MTD_H__ */
move common functions from cmd_nand.c (for calculating offset and size from cmdline paramter) to common place, so they could used from other commands which use mtd partitions. For onenand the arg_off_size() is left in common/cmd_onenand.c. It should use now the common arg_off() function, but as I could not test onenand I let it there ... Signed-off-by: Heiko Schocher <hs@denx.de> Cc: Scott Wood <scottwood@freescale.com> Cc: Tom Rini <trini@ti.com> --- - changes for v2: none - changes for v3: - add comments from scott wood: - align MTD_DEV_TYPE_NAND correct - remove unnecessary inline - rework "jffs2 header" problem later - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6 --- common/cmd_nand.c | 140 +++++++++--------------------------------------- common/cmd_onenand.c | 19 +++---- drivers/mtd/Makefile | 4 +- drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++ include/linux/mtd/mtd.h | 7 +++ 5 files changed, 154 insertions(+), 130 deletions(-) create mode 100644 drivers/mtd/mtd_uboot.c