| Message ID | 1276248975-1822-3-git-send-email-stanley.miao@windriver.com |
|---|---|
| State | New, archived |
| Headers | show |
> > The "struct nand_oobinfo" is able to record only 32 ECC code positions,which > is not enough for many big NAND chips. Therefore, this structure is replaced > by "struct nand_ecclayout" in linux kernel from the version 2.6.17. > Consequently, the ioctl command changed from MEMGETOOBSEL to ECCGETLAYOUT. > > Now update flash_eraseall to use the new ioctl command ECCGETLAYOUT. In order > to keep compatible with the old linux kernel, a linux version detection > function is added. > > Signed-off-by: Stanley.Miao <stanley.miao@windriver.com> > --- > flash_eraseall.c | 71 ++++++++++++++++++++++++++++++++---------------------- > 1 files changed, 42 insertions(+), 29 deletions(-) > > diff --git a/flash_eraseall.c b/flash_eraseall.c > index a22fc49..4e2108b 100644 > --- a/flash_eraseall.c > +++ b/flash_eraseall.c > @@ -43,6 +43,8 @@ > #define PROGRAM "flash_eraseall" > #define VERSION "$Revision: 1.22 $" > > +#define LINUX_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c)) > + > static const char *exe_name; > static const char *mtd_device; > static int quiet; /* true -- don't output progress */ > @@ -55,6 +57,21 @@ static void display_version (void); > static struct jffs2_unknown_node cleanmarker; > int target_endian = __BYTE_ORDER; > > +static int get_linux_version(void) > +{ > + FILE *fd; > + int a = 0, b = 0, c = 0; > + > + fd = fopen("/proc/version", "r"); > + if (fd) { > + fscanf(fd, "Linux version %d.%d.%d", &a, &b, &c); > + fclose(fd); > + } > + > + return LINUX_VERSION(a, b, c); > +} > + > + The fopen could fail and if it does, I think you should default to "assume latest kernel" Same goes for fscanf, you should at least test the return value. Jocke
Joakim Tjernlund/Transmode wrote on 2010/06/11 11:44:27: > > > > > The "struct nand_oobinfo" is able to record only 32 ECC code positions,which > > is not enough for many big NAND chips. Therefore, this structure is replaced > > by "struct nand_ecclayout" in linux kernel from the version 2.6.17. > > Consequently, the ioctl command changed from MEMGETOOBSEL to ECCGETLAYOUT. > > > > Now update flash_eraseall to use the new ioctl command ECCGETLAYOUT. In order > > to keep compatible with the old linux kernel, a linux version detection > > function is added. > > > > Signed-off-by: Stanley.Miao <stanley.miao@windriver.com> > > --- > > flash_eraseall.c | 71 ++++++++++++++++++++++++++++++++---------------------- > > 1 files changed, 42 insertions(+), 29 deletions(-) > > > > diff --git a/flash_eraseall.c b/flash_eraseall.c > > index a22fc49..4e2108b 100644 > > --- a/flash_eraseall.c > > +++ b/flash_eraseall.c > > @@ -43,6 +43,8 @@ > > #define PROGRAM "flash_eraseall" > > #define VERSION "$Revision: 1.22 $" > > > > +#define LINUX_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c)) > > + > > static const char *exe_name; > > static const char *mtd_device; > > static int quiet; /* true -- don't output progress */ > > @@ -55,6 +57,21 @@ static void display_version (void); > > static struct jffs2_unknown_node cleanmarker; > > int target_endian = __BYTE_ORDER; > > > > +static int get_linux_version(void) > > +{ > > + FILE *fd; > > + int a = 0, b = 0, c = 0; > > + > > + fd = fopen("/proc/version", "r"); > > + if (fd) { > > + fscanf(fd, "Linux version %d.%d.%d", &a, &b, &c); > > + fclose(fd); > > + } > > + > > + return LINUX_VERSION(a, b, c); > > +} > > + > > + > The fopen could fail and if it does, I think you should default to "assume > latest kernel" > Same goes for fscanf, you should at least test the return value. Forgot, use uname(2) instead of /proc/version Does the above scanf work on "2.6.31-gentoo-r6"? Jocke
Joakim Tjernlund wrote: > Joakim Tjernlund/Transmode wrote on 2010/06/11 11:44:27: > >>> The "struct nand_oobinfo" is able to record only 32 ECC code positions,which >>> is not enough for many big NAND chips. Therefore, this structure is replaced >>> by "struct nand_ecclayout" in linux kernel from the version 2.6.17. >>> Consequently, the ioctl command changed from MEMGETOOBSEL to ECCGETLAYOUT. >>> >>> Now update flash_eraseall to use the new ioctl command ECCGETLAYOUT. In order >>> to keep compatible with the old linux kernel, a linux version detection >>> function is added. >>> >>> Signed-off-by: Stanley.Miao <stanley.miao@windriver.com> >>> --- >>> flash_eraseall.c | 71 ++++++++++++++++++++++++++++++++---------------------- >>> 1 files changed, 42 insertions(+), 29 deletions(-) >>> >>> diff --git a/flash_eraseall.c b/flash_eraseall.c >>> index a22fc49..4e2108b 100644 >>> --- a/flash_eraseall.c >>> +++ b/flash_eraseall.c >>> @@ -43,6 +43,8 @@ >>> #define PROGRAM "flash_eraseall" >>> #define VERSION "$Revision: 1.22 $" >>> >>> +#define LINUX_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c)) >>> + >>> static const char *exe_name; >>> static const char *mtd_device; >>> static int quiet; /* true -- don't output progress */ >>> @@ -55,6 +57,21 @@ static void display_version (void); >>> static struct jffs2_unknown_node cleanmarker; >>> int target_endian = __BYTE_ORDER; >>> >>> +static int get_linux_version(void) >>> +{ >>> + FILE *fd; >>> + int a = 0, b = 0, c = 0; >>> + >>> + fd = fopen("/proc/version", "r"); >>> + if (fd) { >>> + fscanf(fd, "Linux version %d.%d.%d", &a, &b, &c); >>> + fclose(fd); >>> + } >>> + >>> + return LINUX_VERSION(a, b, c); >>> +} >>> + >>> + >>> >> The fopen could fail and if it does, I think you should default to "assume >> latest kernel" >> Same goes for fscanf, you should at least test the return value. >> I am not sure what kernel version the proc file "version" began to exist. If the fopen failed, it must be a old kernel, so I set the default to the old kernel. It is the same with the current mtd-utils. > > Forgot, use uname(2) instead of /proc/version > Why ? > Does the above scanf work on "2.6.31-gentoo-r6"? > I didn't do the test on all the platforms. Is there any reason that cause it not to work ? Thanks Stanley. > Jocke > > >
"stanley.miao" <stanley.miao@windriver.com> wrote on 2010/06/11 12:36:08: > > Joakim Tjernlund wrote: > > Joakim Tjernlund/Transmode wrote on 2010/06/11 11:44:27: > > > >>> The "struct nand_oobinfo" is able to record only 32 ECC code positions,which > >>> is not enough for many big NAND chips. Therefore, this structure is replaced > >>> by "struct nand_ecclayout" in linux kernel from the version 2.6.17. > >>> Consequently, the ioctl command changed from MEMGETOOBSEL to ECCGETLAYOUT. > >>> > >>> Now update flash_eraseall to use the new ioctl command ECCGETLAYOUT. In order > >>> to keep compatible with the old linux kernel, a linux version detection > >>> function is added. > >>> > >>> Signed-off-by: Stanley.Miao <stanley.miao@windriver.com> > >>> --- > >>> flash_eraseall.c | 71 ++++++++++++++++++++++++++++++++---------------------- > >>> 1 files changed, 42 insertions(+), 29 deletions(-) > >>> > >>> diff --git a/flash_eraseall.c b/flash_eraseall.c > >>> index a22fc49..4e2108b 100644 > >>> --- a/flash_eraseall.c > >>> +++ b/flash_eraseall.c > >>> @@ -43,6 +43,8 @@ > >>> #define PROGRAM "flash_eraseall" > >>> #define VERSION "$Revision: 1.22 $" > >>> > >>> +#define LINUX_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c)) > >>> + > >>> static const char *exe_name; > >>> static const char *mtd_device; > >>> static int quiet; /* true -- don't output progress */ > >>> @@ -55,6 +57,21 @@ static void display_version (void); > >>> static struct jffs2_unknown_node cleanmarker; > >>> int target_endian = __BYTE_ORDER; > >>> > >>> +static int get_linux_version(void) > >>> +{ > >>> + FILE *fd; > >>> + int a = 0, b = 0, c = 0; > >>> + > >>> + fd = fopen("/proc/version", "r"); > >>> + if (fd) { > >>> + fscanf(fd, "Linux version %d.%d.%d", &a, &b, &c); > >>> + fclose(fd); > >>> + } > >>> + > >>> + return LINUX_VERSION(a, b, c); > >>> +} > >>> + > >>> + > >>> > >> The fopen could fail and if it does, I think you should default to "assume > >> latest kernel" > >> Same goes for fscanf, you should at least test the return value. > >> > Forgot to say: Hi :) > I am not sure what kernel version the proc file "version" began to exist. But the proc FS might not be mounted. > If the fopen failed, it must be a old kernel, so I set the default to > the old kernel. > It is the same with the current mtd-utils. I think that behaviour is wrong, one should assume current kernel should one fail to retrive the exact version since it is much more likly that the user is running someting new enough. > > > > > Forgot, use uname(2) instead of /proc/version > > > > Why ? Cleaner, faster, shorter, more portable, less likely to fail. > > > Does the above scanf work on "2.6.31-gentoo-r6"? > > > > I didn't do the test on all the platforms. Is there any reason that > cause it not to work ? I don't remeber how picky scanf is, will it parse the numbers correctly when you have non WS trailing chars? In any case you need to check the return value so you know if it failed. Jocke
> "stanley.miao" <stanley.miao@windriver.com> wrote on 2010/06/11 12:36:08: > > > > Joakim Tjernlund wrote: > > > Joakim Tjernlund/Transmode wrote on 2010/06/11 11:44:27: > > > > > > > > Forgot, use uname(2) instead of /proc/version > > > > > > > Why ? > > Cleaner, faster, shorter, more portable, less likely to fail. Here is a quick example of uname(2): #include <stdio.h> #include <sys/utsname.h> int main() { int a, b, c , ret; struct utsname buf; ret = uname (&buf); if (ret < 0) return -1; ret = sscanf(buf.release, "%d.%d.%d", &a, &b, &c); if (ret != 3) return -1; printf("a:%d, b:%d, c:%d, ret:%d\n", a, b, c, ret); return 0; }
diff --git a/flash_eraseall.c b/flash_eraseall.c index a22fc49..4e2108b 100644 --- a/flash_eraseall.c +++ b/flash_eraseall.c @@ -43,6 +43,8 @@ #define PROGRAM "flash_eraseall" #define VERSION "$Revision: 1.22 $" +#define LINUX_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c)) + static const char *exe_name; static const char *mtd_device; static int quiet; /* true -- don't output progress */ @@ -55,6 +57,21 @@ static void display_version (void); static struct jffs2_unknown_node cleanmarker; int target_endian = __BYTE_ORDER; +static int get_linux_version(void) +{ + FILE *fd; + int a = 0, b = 0, c = 0; + + fd = fopen("/proc/version", "r"); + if (fd) { + fscanf(fd, "Linux version %d.%d.%d", &a, &b, &c); + fclose(fd); + } + + return LINUX_VERSION(a, b, c); +} + + int main (int argc, char *argv[]) { mtd_info_t meminfo; @@ -84,41 +101,37 @@ int main (int argc, char *argv[]) if (!isNAND) cleanmarker.totlen = cpu_to_je32 (sizeof (struct jffs2_unknown_node)); else { - struct nand_oobinfo oobinfo; - - if (ioctl(fd, MEMGETOOBSEL, &oobinfo) != 0) { - fprintf(stderr, "%s: %s: unable to get NAND oobinfo\n", exe_name, mtd_device); - return 1; - } + struct nand_ecclayout ecclayout; - /* 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"); + memset(&ecclayout, 0, sizeof(ecclayout)); + if (get_linux_version() > LINUX_VERSION(2, 6, 17)) { + if (ioctl(fd, ECCGETLAYOUT, &ecclayout) != 0) { + fprintf(stderr, "%s: %s: unable to get NAND oob layout\n", \ + exe_name, mtd_device); return 1; } - 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; + struct nand_oobinfo oi; + + if (ioctl(fd, MEMGETOOBSEL, &oi) != 0) { + fprintf(stderr, "%s: %s: unable to get NAND oobinfo\n", \ + exe_name, mtd_device); + return 1; } + memcpy(&ecclayout.eccpos, &oi.eccpos, sizeof(oi.eccpos)); + memcpy(&ecclayout.oobfree, &oi.oobfree, sizeof(oi.oobfree)); + ecclayout.eccbytes = oi.eccbytes; + } + + /* Get the position of the free bytes */ + if (!ecclayout.oobfree[0].length) { + fprintf(stderr, " Eeep. Autoplacement selected and no empty space in oob\n"); + return 1; } + clmpos = ecclayout.oobfree[0].offset; + clmlen = ecclayout.oobfree[0].length; + if (clmlen > 8) + clmlen = 8; cleanmarker.totlen = cpu_to_je32(8); } cleanmarker.hdr_crc = cpu_to_je32 (crc32 (0, &cleanmarker, sizeof (struct jffs2_unknown_node) - 4));
The "struct nand_oobinfo" is able to record only 32 ECC code positions,which is not enough for many big NAND chips. Therefore, this structure is replaced by "struct nand_ecclayout" in linux kernel from the version 2.6.17. Consequently, the ioctl command changed from MEMGETOOBSEL to ECCGETLAYOUT. Now update flash_eraseall to use the new ioctl command ECCGETLAYOUT. In order to keep compatible with the old linux kernel, a linux version detection function is added. Signed-off-by: Stanley.Miao <stanley.miao@windriver.com> --- flash_eraseall.c | 71 ++++++++++++++++++++++++++++++++---------------------- 1 files changed, 42 insertions(+), 29 deletions(-)