[U-Boot] [PATCH v3 2/3] mtd, nand: move common functions from cmd_nand.c to common place
Heiko Schocher
hs at denx.de
Wed Nov 5 13:38:26 CET 2014
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 at 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 at denx.de>
>> Cc: Scott Wood<scottwood at freescale.com>
>> Cc: Tom Rini<trini at 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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
More information about the U-Boot
mailing list