| Message ID | 1316656646-8338-1-git-send-email-marek.vasut@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Scott Wood |
| Headers | show |
On Thu, Sep 22, 2011 at 03:57:26AM +0200, Marek Vasut wrote: > diff --git a/common/cmd_nand.c b/common/cmd_nand.c > index 72d418c..2f8723f 100644 > --- a/common/cmd_nand.c > +++ b/common/cmd_nand.c > @@ -362,15 +362,34 @@ usage: > > #endif > > -static void nand_print_info(int idx) > +static void nand_print_and_set_info(int idx) > { > nand_info_t *nand = &nand_info[idx]; > struct nand_chip *chip = nand->priv; > + const int bufsz = 32; > + char buf[bufsz]; > + > printf("Device %d: ", idx); > if (chip->numchips > 1) > printf("%dx ", chip->numchips); > printf("%s, sector size %u KiB\n", > nand->name, nand->erasesize >> 10); > + printf(" Page size %8d b\n", nand->writesize); > + printf(" OOB size %8d b\n", nand->oobsize); > + printf(" Erase size %8d b\n", nand->erasesize); > + > + /* Set geometry info */ > + memset(buf, 0, bufsz); > + sprintf(buf, "%x", nand->writesize); > + setenv("nand_writesize", buf); > + > + memset(buf, 0, bufsz); > + sprintf(buf, "%x", nand->oobsize); > + setenv("nand_oobsize", buf); > + > + memset(buf, 0, bufsz); > + sprintf(buf, "%x", nand->erasesize); > + setenv("nand_erasesize", buf); Why the memsets? -Scott
On Tuesday, September 27, 2011 09:01:53 PM Scott Wood wrote: > On Thu, Sep 22, 2011 at 03:57:26AM +0200, Marek Vasut wrote: > > diff --git a/common/cmd_nand.c b/common/cmd_nand.c > > index 72d418c..2f8723f 100644 > > --- a/common/cmd_nand.c > > +++ b/common/cmd_nand.c > > > > @@ -362,15 +362,34 @@ usage: > > #endif > > > > -static void nand_print_info(int idx) > > +static void nand_print_and_set_info(int idx) > > > > { > > > > nand_info_t *nand = &nand_info[idx]; > > struct nand_chip *chip = nand->priv; > > > > + const int bufsz = 32; > > + char buf[bufsz]; > > + > > > > printf("Device %d: ", idx); > > if (chip->numchips > 1) > > > > printf("%dx ", chip->numchips); > > > > printf("%s, sector size %u KiB\n", > > > > nand->name, nand->erasesize >> 10); > > > > + printf(" Page size %8d b\n", nand->writesize); > > + printf(" OOB size %8d b\n", nand->oobsize); > > + printf(" Erase size %8d b\n", nand->erasesize); > > + > > + /* Set geometry info */ > > + memset(buf, 0, bufsz); > > + sprintf(buf, "%x", nand->writesize); > > + setenv("nand_writesize", buf); > > + > > + memset(buf, 0, bufsz); > > + sprintf(buf, "%x", nand->oobsize); > > + setenv("nand_oobsize", buf); > > + > > + memset(buf, 0, bufsz); > > + sprintf(buf, "%x", nand->erasesize); > > + setenv("nand_erasesize", buf); > > Why the memsets? To clear the memory from previous usage ? > > -Scott
On 09/27/2011 02:38 PM, Marek Vasut wrote: > On Tuesday, September 27, 2011 09:01:53 PM Scott Wood wrote: >> On Thu, Sep 22, 2011 at 03:57:26AM +0200, Marek Vasut wrote: >>> + /* Set geometry info */ >>> + memset(buf, 0, bufsz); >>> + sprintf(buf, "%x", nand->writesize); >>> + setenv("nand_writesize", buf); >>> + >>> + memset(buf, 0, bufsz); >>> + sprintf(buf, "%x", nand->oobsize); >>> + setenv("nand_oobsize", buf); >>> + >>> + memset(buf, 0, bufsz); >>> + sprintf(buf, "%x", nand->erasesize); >>> + setenv("nand_erasesize", buf); >> >> Why the memsets? > > To clear the memory from previous usage ? What part of the previous usage will both survive the sprintf() and be looked at by setenv()? -Scott
On Tuesday, September 27, 2011 09:51:09 PM Scott Wood wrote: > On 09/27/2011 02:38 PM, Marek Vasut wrote: > > On Tuesday, September 27, 2011 09:01:53 PM Scott Wood wrote: > >> On Thu, Sep 22, 2011 at 03:57:26AM +0200, Marek Vasut wrote: > >>> + /* Set geometry info */ > >>> + memset(buf, 0, bufsz); > >>> + sprintf(buf, "%x", nand->writesize); > >>> + setenv("nand_writesize", buf); > >>> + > >>> + memset(buf, 0, bufsz); > >>> + sprintf(buf, "%x", nand->oobsize); > >>> + setenv("nand_oobsize", buf); > >>> + > >>> + memset(buf, 0, bufsz); > >>> + sprintf(buf, "%x", nand->erasesize); > >>> + setenv("nand_erasesize", buf); > >> > >> Why the memsets? > > > > To clear the memory from previous usage ? > > What part of the previous usage will both survive the sprintf() and be > looked at by setenv()? The part of data that are copied in _do_set_env() ? > > -Scott
On 09/27/2011 03:07 PM, Marek Vasut wrote: > On Tuesday, September 27, 2011 09:51:09 PM Scott Wood wrote: >> On 09/27/2011 02:38 PM, Marek Vasut wrote: >>> On Tuesday, September 27, 2011 09:01:53 PM Scott Wood wrote: >>>> On Thu, Sep 22, 2011 at 03:57:26AM +0200, Marek Vasut wrote: >>>>> + /* Set geometry info */ >>>>> + memset(buf, 0, bufsz); >>>>> + sprintf(buf, "%x", nand->writesize); >>>>> + setenv("nand_writesize", buf); >>>>> + >>>>> + memset(buf, 0, bufsz); >>>>> + sprintf(buf, "%x", nand->oobsize); >>>>> + setenv("nand_oobsize", buf); >>>>> + >>>>> + memset(buf, 0, bufsz); >>>>> + sprintf(buf, "%x", nand->erasesize); >>>>> + setenv("nand_erasesize", buf); >>>> >>>> Why the memsets? >>> >>> To clear the memory from previous usage ? >> >> What part of the previous usage will both survive the sprintf() and be >> looked at by setenv()? > > The part of data that are copied in _do_set_env() ? I don't see _do_set_env anywhere -- what tree are you looking at? In any case, sprintf() produces a zero-terminated string. setenv() consumes a zero-terminated string. setenv() doesn't even know that the buffer containing the string happens to be 32 bytes, much less have any business poking around in that area. -Scott
On Tuesday, September 27, 2011 10:53:56 PM Scott Wood wrote: > On 09/27/2011 03:07 PM, Marek Vasut wrote: > > On Tuesday, September 27, 2011 09:51:09 PM Scott Wood wrote: > >> On 09/27/2011 02:38 PM, Marek Vasut wrote: > >>> On Tuesday, September 27, 2011 09:01:53 PM Scott Wood wrote: > >>>> On Thu, Sep 22, 2011 at 03:57:26AM +0200, Marek Vasut wrote: > >>>>> + /* Set geometry info */ > >>>>> + memset(buf, 0, bufsz); > >>>>> + sprintf(buf, "%x", nand->writesize); > >>>>> + setenv("nand_writesize", buf); > >>>>> + > >>>>> + memset(buf, 0, bufsz); > >>>>> + sprintf(buf, "%x", nand->oobsize); > >>>>> + setenv("nand_oobsize", buf); > >>>>> + > >>>>> + memset(buf, 0, bufsz); > >>>>> + sprintf(buf, "%x", nand->erasesize); > >>>>> + setenv("nand_erasesize", buf); > >>>> > >>>> Why the memsets? > >>> > >>> To clear the memory from previous usage ? > >> > >> What part of the previous usage will both survive the sprintf() and be > >> looked at by setenv()? > > > > The part of data that are copied in _do_set_env() ? > > I don't see _do_set_env anywhere -- what tree are you looking at? > > In any case, sprintf() produces a zero-terminated string. setenv() > consumes a zero-terminated string. Correct > setenv() doesn't even know that the > buffer containing the string happens to be 32 bytes, much less have any > business poking around in that area. True ... but the stuff you call setenv() on is copied to environment. That's about it, it doesn't get lost anywhere.
On Thu, Sep 22, 2011 at 03:57:26AM +0200, Marek Vasut wrote: > The "nand info" and "nand device" now set shell/environment variables: > nand_writesize ... nand page size > nand_oobsize ..... nand oob area size > nand_erasesize ... nand erase block size > > Also, the "nand info" command now displays this info. > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com> > Cc: Scott Wood <scottwood@freescale.com> > Cc: Stefano Babic <sbabic@denx.de> > Cc: Wolfgang Denk <wd@denx.de> > Cc: Detlev Zundel <dzu@denx.de> > --- > common/cmd_nand.c | 25 ++++++++++++++++++++++--- > 1 files changed, 22 insertions(+), 3 deletions(-) > Applied to u-boot-nand-flash next > + /* Set geometry info */ > + memset(buf, 0, bufsz); > + sprintf(buf, "%x", nand->writesize); > + setenv("nand_writesize", buf); > + > + memset(buf, 0, bufsz); > + sprintf(buf, "%x", nand->oobsize); > + setenv("nand_oobsize", buf); > + > + memset(buf, 0, bufsz); > + sprintf(buf, "%x", nand->erasesize); > + setenv("nand_erasesize", buf); ...with memsets removed. -Scott
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index 72d418c..2f8723f 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -362,15 +362,34 @@ usage: #endif -static void nand_print_info(int idx) +static void nand_print_and_set_info(int idx) { nand_info_t *nand = &nand_info[idx]; struct nand_chip *chip = nand->priv; + const int bufsz = 32; + char buf[bufsz]; + printf("Device %d: ", idx); if (chip->numchips > 1) printf("%dx ", chip->numchips); printf("%s, sector size %u KiB\n", nand->name, nand->erasesize >> 10); + printf(" Page size %8d b\n", nand->writesize); + printf(" OOB size %8d b\n", nand->oobsize); + printf(" Erase size %8d b\n", nand->erasesize); + + /* Set geometry info */ + memset(buf, 0, bufsz); + sprintf(buf, "%x", nand->writesize); + setenv("nand_writesize", buf); + + memset(buf, 0, bufsz); + sprintf(buf, "%x", nand->oobsize); + setenv("nand_oobsize", buf); + + memset(buf, 0, bufsz); + sprintf(buf, "%x", nand->erasesize); + setenv("nand_erasesize", buf); } int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) @@ -407,7 +426,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) putc('\n'); for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) { if (nand_info[i].name) - nand_print_info(i); + nand_print_and_set_info(i); } return 0; } @@ -418,7 +437,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE) puts("no devices available\n"); else - nand_print_info(dev); + nand_print_and_set_info(dev); return 0; }
The "nand info" and "nand device" now set shell/environment variables: nand_writesize ... nand page size nand_oobsize ..... nand oob area size nand_erasesize ... nand erase block size Also, the "nand info" command now displays this info. Signed-off-by: Marek Vasut <marek.vasut@gmail.com> Cc: Scott Wood <scottwood@freescale.com> Cc: Stefano Babic <sbabic@denx.de> Cc: Wolfgang Denk <wd@denx.de> Cc: Detlev Zundel <dzu@denx.de> --- common/cmd_nand.c | 25 ++++++++++++++++++++++--- 1 files changed, 22 insertions(+), 3 deletions(-) V2: Remove setup of HUSH local vars.