On Fri, Feb 13, 2009 at 08:29:32PM +0100, Sebastian Andrzej Siewior wrote:
>This is the result after converting mtd-utils' flash_eraseall to BB.
>The functionality given by this patch almost the same except that this
>one does not support long options.
>I needed this tool a system which does not have a lot of flash for RFS
>and merging this into BB as the only way out.

Please add the output of ./scripts/bloat-o-meter and size(1) of
miscutils/flash_eraseall.o

See below for a few comments.
>
>Signed-off-by: Benedigt Spranger <[email protected]>
>Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
>Index: busybox/include/applets.h
>===================================================================
>--- busybox.orig/include/applets.h
>+++ busybox/include/applets.h
>@@ -155,6 +155,7 @@ USE_FDISK(APPLET(fdisk, _BB_DIR_SBIN, _B
> USE_FEATURE_GREP_FGREP_ALIAS(APPLET_ODDNAME(fgrep, grep, _BB_DIR_BIN, 
> _BB_SUID_NEVER, fgrep))
> USE_FIND(APPLET_NOEXEC(find, find, _BB_DIR_USR_BIN, _BB_SUID_NEVER, find))
> USE_FINDFS(APPLET(findfs, _BB_DIR_SBIN, _BB_SUID_MAYBE))
>+USE_FLASH_ERASEALL(APPLET_ODDNAME(flash_eraseall, flash_eraseall, 
>_BB_DIR_USR_SBIN, _BB_SUID_NEVER, flash_eraseall))
> USE_FOLD(APPLET(fold, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
> USE_FREE(APPLET(free, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
> USE_FREERAMDISK(APPLET(freeramdisk, _BB_DIR_SBIN, _BB_SUID_NEVER))
>Index: busybox/include/usage.h
>===================================================================
>--- busybox.orig/include/usage.h
>+++ busybox/include/usage.h
>@@ -1217,6 +1217,12 @@
>        "$ find / -name passwd\n" \
>        "/etc/passwd\n"
> 
>+#define flash_eraseall_trivial_usage \
>+       "[-jq] MTD_DEVICE"
>+#define flash_eraseall_full_usage "\n\n" \
>+      "  -j     format the device for jffs2\n" \
>+      "  -q     don't display progress messages"

use a tab instead of '     ', twice.
>+
> #define fold_trivial_usage \
>        "[-bs] [-w WIDTH] [FILE]"
> #define fold_full_usage "\n\n" \
>Index: busybox/miscutils/flash_eraseall.c
>===================================================================
>--- /dev/null
>+++ busybox/miscutils/flash_eraseall.c
>@@ -0,0 +1,198 @@

>+static struct jffs2_unknown_node cleanmarker;
>+int target_endian = __BYTE_ORDER;
>+static uint32_t *crc32_table;
>+
>+static uint32_t crc32(uint32_t val, const void *ss, int len)
>+{
>+      const unsigned char *s = ss;
>+      while (--len >= 0)
>+              val = crc32_table[(val ^ *s++) & 0xff] ^ (val >> 8);
>+      return val;
>+}

Don't we have a crc impl in there already?
>+
>+static void show_progress (mtd_info_t *meminfo, erase_info_t *erase)
>+{
>+      printf("\rErasing %d Kibyte @ %x -- %2llu %% complete.",
>+              meminfo->erasesize / 1024, erase->start,
>+              (unsigned long long) erase->start * 100 / meminfo->size);
>+      fflush(stdout);
>+}

I think that we already have a progress-meter in there, perhaps you could
use that instead.
>+
>+int flash_eraseall_main(int argc, char **argv);

missing MAIN_EXTERNALLY_VISIBLE above.
See any other applet in the tree, taskset.c is in the same directory, for
example.

>+int flash_eraseall_main(int argc, char **argv)
>+{
>+      int quiet;
>+      int jffs2;

wow, perhaps just #define quiet (flags & OPTION_Q) and the same for jffs2.

>+      mtd_info_t meminfo;
>+      int fd, clmpos = 0, clmlen = 8;
>+      erase_info_t erase;
>+      struct stat st;
>+      int isNAND, bbtest = 1;

isNAND can be a simple bool and could just be ORed into e.g. flags.

>+      unsigned int flags;
>+      char *mtd_name;
>+
>+      if (argc < 2) {
>+              bb_show_usage();
>+      }
>+
>+      flags = getopt32(argv, "jq");
>+      jffs2 = flags & OPTION_J;
>+      quiet = flags & OPTION_Q;
>+
>+      mtd_name = *(argv + optind);
>+
>+      xstat(mtd_name, &st);
>+      if (!S_ISCHR(st.st_mode)) {
>+              bb_error_msg_and_die("%s: not a char device", mtd_name);
>+      }
>+
>+      fd = xopen(mtd_name, O_RDWR);
>+
>+      xioctl(fd, MEMGETINFO, &meminfo);
>+
>+      erase.length = meminfo.erasesize;
>+      isNAND = meminfo.type == MTD_NANDFLASH ? 1 : 0;
>+
>+      if (jffs2) {
>+              crc32_table = crc32_filltable(NULL, 0);
>+
>+              cleanmarker.magic = cpu_to_je16 (JFFS2_MAGIC_BITMASK);
>+              cleanmarker.nodetype = cpu_to_je16 (JFFS2_NODETYPE_CLEANMARKER);
>+              if (!isNAND)
>+                      cleanmarker.totlen = cpu_to_je32 (sizeof (struct 
>jffs2_unknown_node));
>+              else {
>+                      struct nand_oobinfo oobinfo;
>+
>+                      xioctl(fd, MEMGETOOBSEL, &oobinfo);
>+
>+                      /* Check for autoplacement */
>+                      if (oobinfo.useecc == MTD_NANDECC_AUTOPLACE) {
>+                              /* Get the position of the free bytes */
>+                              if (!oobinfo.oobfree[0][1]) {
>+                                      fprintf (stderr, " Eeep. Autoplacement 
>selected and no empty space in oob\n");
>+                                      return 1;
bb_error_msg_and_die()
>+                              }
>+                              clmpos = oobinfo.oobfree[0][0];
>+                              clmlen = oobinfo.oobfree[0][1];
>+                              if (clmlen > 8)
>+                                      clmlen = 8;
>+                      } else {
>+                              /* Legacy mode */
>+                              switch (meminfo.oobsize) {
>+                                      case 8:
>+                                              clmpos = 6;
>+                                              clmlen = 2;
>+                                              break;
>+                                      case 16:
>+                                              clmpos = 8;
>+                                              clmlen = 8;
>+                                              break;
>+                                      case 64:
>+                                              clmpos = 16;
>+                                              clmlen = 8;
>+                                              break;
>+                              }

I don't think that there is a pass that can optimize that properly, so better
spell that out explicitely with an if-else chain.

>+                      }
>+                      cleanmarker.totlen = cpu_to_je32(8);
>+              }
>+
>+              cleanmarker.hdr_crc =  cpu_to_je32 (crc32 (0, &cleanmarker,  
>sizeof (struct jffs2_unknown_node) - 4));
>+      }
>+
>+      for (erase.start = 0; erase.start < meminfo.size;
>+           erase.start += meminfo.erasesize) {
>+              if (bbtest) {
>+                      int ret;
>+
>+                      loff_t offset = erase.start;
>+                      ret = ioctl(fd, MEMGETBADBLOCK, &offset);
>+                      if (ret > 0) {
>+                              if (!quiet)
>+                                      printf("\nSkipping bad block at 
>0x%08x\n", erase.start);

bb_info_msg, perhaps?
>+                              continue;
>+                      } else if (ret < 0) {
>+                              /* Black block table is not available on 
>certain flash
>+                               * types e.g. NOR
>+                               */
>+                              if (errno == EOPNOTSUPP) {
>+                                      bbtest = 0;
>+                                      if (isNAND) {
>+                                              fprintf(stderr, "%s: %s: Bad 
>block check not available\n",
>+                                                              argv[0], 
>mtd_name);
>+                                              return 1;
bb_error_msg_and_die().
and return 1 is never adequate for a main, there is EXIT_{SUCCESS,FAILURE} in 
stdlib.h for that.
>+                                      }
>+                              } else {
>+                                      fprintf(stderr, "\n%s: %s: MTD get bad 
>block failed: %s\n",
>+                                                      argv[0], mtd_name, 
>strerror(errno));
>+                                      return 1;
bb_error_msg_and_die().
>+                              }
>+                      }
>+              }
>+
>+              if (!quiet)
>+                      show_progress(&meminfo, &erase);
>+
>+              xioctl(fd, MEMERASE, &erase);
>+
>+              /* format for JFFS2 ? */
>+              if (!jffs2)
>+                      continue;
>+
>+              /* write cleanmarker */
>+              if (isNAND) {
>+                      struct mtd_oob_buf oob;
>+                      oob.ptr = (unsigned char *) &cleanmarker;
>+                      oob.start = erase.start + clmpos;
>+                      oob.length = clmlen;
>+                      xioctl (fd, MEMWRITEOOB, &oob);
>+              } else {
>+                      if (lseek (fd, erase.start, SEEK_SET) < 0) {
>+                              fprintf(stderr, "\n%s: MTD lseek failure: 
>%s\n", mtd_name, strerror(errno));
>+                              continue;
>+                      }
>+                      if (write (fd , &cleanmarker, sizeof (cleanmarker)) != 
>sizeof (cleanmarker)) {
>+                              fprintf(stderr, "\n%s: MTD write failure: 
>%s\n", mtd_name, strerror(errno));
>+                              continue;
>+                      }
>+              }
>+              if (!quiet)
>+                      printf (" Cleanmarker written at %x.", erase.start);
>+      }
>+      if (!quiet) {
>+              show_progress(&meminfo, &erase);
>+              printf("\n");

putchar
>+      }
>+
>+      return 0;

EXIT_SUCCESS
>+}
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to