From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Mon, 11 Aug 2014 10:40:38 +0200 Subject: [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place In-Reply-To: <1406680154.29414.257.camel@snotra.buserror.net> References: <1405323547-32588-1-git-send-email-hs@denx.de> <1405323547-32588-3-git-send-email-hs@denx.de> <1406680154.29414.257.camel@snotra.buserror.net> Message-ID: <53E88186.9000807@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Scott, Am 30.07.2014 02:29, schrieb Scott Wood: > On Mon, 2014-07-14 at 09:39 +0200, Heiko Schocher 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 >> Cc: Scott Wood >> Cc: Tom Rini >> --- >> 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 > > ACK the concept, some nits below. > > Have you given any more thought to formally taking over the > custodianship? If not, Tom, are you expecting another pull request from > me or were you going to apply Heiko's patches directly? I thinking about it, yes, but I am really busy ... but it seems nobody else want to volunteer ... I discussed with Wolfgang, if it would make sense to create a mtd branch ... so, if you could not longer maintain the nand branch, we can set it to orphaned (in the hope, someone will volunteer ...) and I can also handle the nand patches ... but Wolfgang is currently on vacation, so this step maybe take a while ... >> @@ -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; >> + } > > MTD_DEV_TYPE_NAND should be aligned with argv[2] (if you're going to do > alignment rather than just a couple tabs), not arg_off. Likewise > elsewhere. fixed. >> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c >> new file mode 100644 >> index 0000000..66e49c3 >> --- /dev/null >> +++ b/drivers/mtd/mtd_uboot.c >> @@ -0,0 +1,114 @@ >> +/* >> + * (C) Copyright 2014 >> + * Heiko Schocher, DENX Software Engineering, hs at denx.de. >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> +#include >> +#include >> +#include > > What do you use from the jffs2 header? Yes, good question ... because missing common mtd defines, like: struct mtd_device { found in include/jffs2/load_kernel.h used in common/cmd_nand.c for example ... its really time to cleanup the mtd subsystem! Where to move this common defines? include/linux/mtd/mtd.h ? But before the mtd/ubi/ubifs sync patches are not in mainline, I do not want change this ... maybe it is okay to do this in a second step? >> +inline int str2off(const char *p, loff_t *num) >> +{ >> + char *endptr; >> + >> + *num = simple_strtoull(p,&endptr, 16); >> + return *p != '\0'&& *endptr == '\0'; >> +} >> + >> +inline int str2long(const char *p, ulong *num) >> +{ >> + char *endptr; >> + >> + *num = simple_strtoul(p,&endptr, 16); >> + return *p != '\0'&& *endptr == '\0'; >> +} > > Should drop the inline -- it didn't make much sense in the old location > as it wasn't in a header file (GCC can auto-inline where appropriate), > and it makes even less sense if it's not static. removed, thanks for the review! bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany