diff mbox series

mtd-utils: nandtest: handle large nand devices

Message ID 7c089577-710f-3310-305e-f05f1d125652@brainaid.de
State Superseded
Headers show
Series mtd-utils: nandtest: handle large nand devices | expand

Commit Message

Christian Wendt he/him Aug. 10, 2023, 8:20 a.m. UTC 
[PATCH] mtd-utils: nandtest: handle large nand devices

Running nandtest on devices with sizes of 4Gb or more does not work, as
the size returned in mtd_info_user is reported as 0 for 4Gb devices for
example.

The patch uses sysfs to figure out the size of the nand device (as
recommended in a comment in include/mtd/mtd-abi.h) and changes sizes
and offsets to 64 bit values.

[From: Christian Wendt <cw@brainaid.de>]
   */
@@ -156,8 +177,9 @@ int main(int argc, char **argv)
         int nr_passes = 1;
         int nr_reads = 4;
         int keep_contents = 0;
-       uint32_t offset = 0;
-       uint32_t length = -1;
+       uint64_t offset = 0;
+       uint64_t length = -1;
+       uint64_t mem_size = 0;
         int error = 0;

         seed = time(NULL);
@@ -238,29 +260,31 @@ int main(int argc, char **argv)
                 exit(1);
         }

+       mem_size = get_mem_size(argv[optind]);
+
         if (length == -1)
-               length = meminfo.size;
+               length = mem_size;

         if (offset % meminfo.erasesize) {
-               fprintf(stderr, "Offset %x not multiple of erase size %x\n",
+               fprintf(stderr, "Offset %llx not multiple of erase size 
%x\n",
                         offset, meminfo.erasesize);
                 exit(1);
         }
         if (length % meminfo.erasesize) {
-               fprintf(stderr, "Length %x not multiple of erase size %x\n",
+               fprintf(stderr, "Length %llx not multiple of erase size 
%x\n",
                         length, meminfo.erasesize);
                 exit(1);
         }
-       if (length + offset > meminfo.size) {
-               fprintf(stderr, "Length %x + offset %x exceeds device 
size %x\n",
-                       length, offset, meminfo.size);
+       if (length + offset > mem_size) {
+               fprintf(stderr, "Length %llx + offset %llx exceeds 
device size %llx\n",
+                       length, offset, mem_size);
                 exit(1);
         }

         wbuf = malloc(meminfo.erasesize * 3);
         if (!wbuf) {
                 fprintf(stderr, "Could not allocate %d bytes for buffer\n",
-                       meminfo.erasesize * 2);
+                       meminfo.erasesize * 3);
                 exit(1);
         }
         rbuf = wbuf + meminfo.erasesize;

Comments

Zhihao Cheng Aug. 12, 2023, 2:53 a.m. UTC | #1 
在 2023/8/10 16:20, Christian Wendt he/him 写道:
> [PATCH] mtd-utils: nandtest: handle large nand devices
> 
> Running nandtest on devices with sizes of 4Gb or more does not work, as
> the size returned in mtd_info_user is reported as 0 for 4Gb devices for
> example.
> 
> The patch uses sysfs to figure out the size of the nand device (as
> recommended in a comment in include/mtd/mtd-abi.h) and changes sizes
> and offsets to 64 bit values.
> 
> [From: Christian Wendt <cw@brainaid.de>]
> diff --git a/nand-utils/nandtest.c b/nand-utils/nandtest.c
> index 06dec25..e60428f 100644
> --- a/nand-utils/nandtest.c
> +++ b/nand-utils/nandtest.c
> @@ -8,6 +8,7 @@
>   #include <string.h>
>   #include <time.h>
>   #include <unistd.h>
> +#include <limits.h>
>   #include <sys/stat.h>
>   #include <sys/ioctl.h>
>   #include <sys/types.h>
> @@ -145,6 +146,26 @@ static int erase_and_write(loff_t ofs, unsigned 
> char *data, unsigned char *rbuf,
>   }
> 
> 
> +static uint64_t get_mem_size(const char* device)
> +{
> +       const char* p = strrchr(device, '/');
> +       char path[PATH_MAX];
> +       int fd;
> +
> +       snprintf(path, sizeof(path), "/sys/class/mtd/%s/size", p);
> +       fd = open(path, O_RDONLY);
> +       if (fd) {
> +               char buffer[32];
> +               ssize_t n = read(fd, buffer, sizeof(buffer));
> +               close(fd);
> +               if (n > 0) {
> +                       return strtoull(buffer, NULL, 0);
> +               }
> +       }

It's better to add error handling for open/read failures, otherwise 
'length' will be 0 if user doesn't pass '-l' parameter when open/read 
'/sys/class/mtd/mtdX/size' failed.

> +
> +       return 0;
> +}
> +
>   /*
>    * Main program
>    */
> @@ -156,8 +177,9 @@ int main(int argc, char **argv)
>          int nr_passes = 1;
>          int nr_reads = 4;
>          int keep_contents = 0;
> -       uint32_t offset = 0;
> -       uint32_t length = -1;
> +       uint64_t offset = 0;
> +       uint64_t length = -1;
> +       uint64_t mem_size = 0;
>          int error = 0;
> 
>          seed = time(NULL);
> @@ -238,29 +260,31 @@ int main(int argc, char **argv)
>                  exit(1);
>          }
> 
> +       mem_size = get_mem_size(argv[optind]);
> +
>          if (length == -1)
> -               length = meminfo.size;
> +               length = mem_size;
> 
>          if (offset % meminfo.erasesize) {
> -               fprintf(stderr, "Offset %x not multiple of erase size 
> %x\n",
> +               fprintf(stderr, "Offset %llx not multiple of erase size 
> %x\n",
>                          offset, meminfo.erasesize);
>                  exit(1);
>          }
>          if (length % meminfo.erasesize) {
> -               fprintf(stderr, "Length %x not multiple of erase size 
> %x\n",
> +               fprintf(stderr, "Length %llx not multiple of erase size 
> %x\n",
>                          length, meminfo.erasesize);
>                  exit(1);
>          }
> -       if (length + offset > meminfo.size) {
> -               fprintf(stderr, "Length %x + offset %x exceeds device 
> size %x\n",
> -                       length, offset, meminfo.size);
> +       if (length + offset > mem_size) {
> +               fprintf(stderr, "Length %llx + offset %llx exceeds 
> device size %llx\n",
> +                       length, offset, mem_size);
>                  exit(1);
>          }
> 
>          wbuf = malloc(meminfo.erasesize * 3);
>          if (!wbuf) {
>                  fprintf(stderr, "Could not allocate %d bytes for 
> buffer\n",
> -                       meminfo.erasesize * 2);
> +                       meminfo.erasesize * 3);
>                  exit(1);
>          }
>          rbuf = wbuf + meminfo.erasesize;
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> .
Christian Wendt he/him Aug. 16, 2023, 2:07 p.m. UTC | #2 
[PATCH] mtd-utils: nandtest: handle large nand devices

Running nandtest on devices with sizes of 4Gb or more does not work, as
the size returned in mtd_info_user is reported as 0 for 4Gb devices for
example.

The patch uses sysfs to figure out the size of the nand device (as
recommended in a comment in include/mtd/mtd-abi.h) and changes sizes
and offsets to 64 bit values.

[From: Christian Wendt <cw@brainaid.de>]

diff --git a/nand-utils/nandtest.c b/nand-utils/nandtest.c
index 06dec25..98217ba 100644
--- a/nand-utils/nandtest.c
+++ b/nand-utils/nandtest.c
@@ -8,6 +8,7 @@
  #include <string.h>
  #include <time.h>
  #include <unistd.h>
+#include <limits.h>
  #include <sys/stat.h>
  #include <sys/ioctl.h>
  #include <sys/types.h>
@@ -145,6 +146,27 @@ static int erase_and_write(loff_t ofs, unsigned 
char *data, unsigned char *rbuf,
  }


+static uint64_t get_mem_size(const char* device)
+{
+       const char* p = strrchr(device, '/');
+       char path[PATH_MAX];
+       int fd;
+
+       snprintf(path, sizeof(path), "/sys/class/mtd/%s/size", p);
+       fd = open(path, O_RDONLY);
+       if (fd >= 0) {
+               char buffer[32];
+               ssize_t n = read(fd, buffer, sizeof(buffer));
+               close(fd);
+               if (n > 0) {
+                       return strtoull(buffer, NULL, 0);
+               }
+       }
+
+       fprintf(stderr, "Can't read size from %s\n", path);
+       exit(1);
+}
+
  /*
   * Main program
   */
@@ -156,8 +178,9 @@ int main(int argc, char **argv)
         int nr_passes = 1;
         int nr_reads = 4;
         int keep_contents = 0;
-       uint32_t offset = 0;
-       uint32_t length = -1;
+       uint64_t offset = 0;
+       uint64_t length = -1;
+       uint64_t mem_size = 0;
         int error = 0;

         seed = time(NULL);
@@ -212,11 +235,11 @@ int main(int argc, char **argv)
                         break;

                 case 'o':
-                       offset = simple_strtoul(optarg, &error);
+                       offset = simple_strtoull(optarg, &error);
                         break;

                 case 'l':
-                       length = simple_strtoul(optarg, &error);
+                       length = simple_strtoull(optarg, &error);
                         break;

                 }
@@ -238,29 +261,31 @@ int main(int argc, char **argv)
                 exit(1);
         }

+       mem_size = get_mem_size(argv[optind]);
+
         if (length == -1)
-               length = meminfo.size;
+               length = mem_size;

         if (offset % meminfo.erasesize) {
-               fprintf(stderr, "Offset %x not multiple of erase size %x\n",
+               fprintf(stderr, "Offset %llx not multiple of erase size 
%x\n",
                         offset, meminfo.erasesize);
                 exit(1);
         }
         if (length % meminfo.erasesize) {
-               fprintf(stderr, "Length %x not multiple of erase size %x\n",
+               fprintf(stderr, "Length %llx not multiple of erase size 
%x\n",
                         length, meminfo.erasesize);
                 exit(1);
         }
-       if (length + offset > meminfo.size) {
-               fprintf(stderr, "Length %x + offset %x exceeds device 
size %x\n",
-                       length, offset, meminfo.size);
+       if (length + offset > mem_size) {
+               fprintf(stderr, "Length %llx + offset %llx exceeds 
device size %llx\n",
+                       length, offset, mem_size);
                 exit(1);
         }

         wbuf = malloc(meminfo.erasesize * 3);
         if (!wbuf) {
                 fprintf(stderr, "Could not allocate %d bytes for buffer\n",
-                       meminfo.erasesize * 2);
+                       meminfo.erasesize * 3);
                 exit(1);
         }
         rbuf = wbuf + meminfo.erasesize;

On 12-08-2023 04:53, Zhihao Cheng wrote:
> 在 2023/8/10 16:20, Christian Wendt he/him 写道:
>> [PATCH] mtd-utils: nandtest: handle large nand devices
>>
>> Running nandtest on devices with sizes of 4Gb or more does not work, as
>> the size returned in mtd_info_user is reported as 0 for 4Gb devices for
>> example.
>>
>> The patch uses sysfs to figure out the size of the nand device (as
>> recommended in a comment in include/mtd/mtd-abi.h) and changes sizes
>> and offsets to 64 bit values.
>>
>> [From: Christian Wendt <cw@brainaid.de>]
>> diff --git a/nand-utils/nandtest.c b/nand-utils/nandtest.c
>> index 06dec25..e60428f 100644
>> --- a/nand-utils/nandtest.c
>> +++ b/nand-utils/nandtest.c
>> @@ -8,6 +8,7 @@
>>   #include <string.h>
>>   #include <time.h>
>>   #include <unistd.h>
>> +#include <limits.h>
>>   #include <sys/stat.h>
>>   #include <sys/ioctl.h>
>>   #include <sys/types.h>
>> @@ -145,6 +146,26 @@ static int erase_and_write(loff_t ofs, unsigned 
>> char *data, unsigned char *rbuf,
>>   }
>>
>>
>> +static uint64_t get_mem_size(const char* device)
>> +{
>> +       const char* p = strrchr(device, '/');
>> +       char path[PATH_MAX];
>> +       int fd;
>> +
>> +       snprintf(path, sizeof(path), "/sys/class/mtd/%s/size", p);
>> +       fd = open(path, O_RDONLY);
>> +       if (fd) {
>> +               char buffer[32];
>> +               ssize_t n = read(fd, buffer, sizeof(buffer));
>> +               close(fd);
>> +               if (n > 0) {
>> +                       return strtoull(buffer, NULL, 0);
>> +               }
>> +       }
> 
> It's better to add error handling for open/read failures, otherwise 
> 'length' will be 0 if user doesn't pass '-l' parameter when open/read 
> '/sys/class/mtd/mtdX/size' failed.
> 
>> +
>> +       return 0;
>> +}
>> +
>>   /*
>>    * Main program
>>    */
>> @@ -156,8 +177,9 @@ int main(int argc, char **argv)
>>          int nr_passes = 1;
>>          int nr_reads = 4;
>>          int keep_contents = 0;
>> -       uint32_t offset = 0;
>> -       uint32_t length = -1;
>> +       uint64_t offset = 0;
>> +       uint64_t length = -1;
>> +       uint64_t mem_size = 0;
>>          int error = 0;
>>
>>          seed = time(NULL);
>> @@ -238,29 +260,31 @@ int main(int argc, char **argv)
>>                  exit(1);
>>          }
>>
>> +       mem_size = get_mem_size(argv[optind]);
>> +
>>          if (length == -1)
>> -               length = meminfo.size;
>> +               length = mem_size;
>>
>>          if (offset % meminfo.erasesize) {
>> -               fprintf(stderr, "Offset %x not multiple of erase size 
>> %x\n",
>> +               fprintf(stderr, "Offset %llx not multiple of erase 
>> size %x\n",
>>                          offset, meminfo.erasesize);
>>                  exit(1);
>>          }
>>          if (length % meminfo.erasesize) {
>> -               fprintf(stderr, "Length %x not multiple of erase size 
>> %x\n",
>> +               fprintf(stderr, "Length %llx not multiple of erase 
>> size %x\n",
>>                          length, meminfo.erasesize);
>>                  exit(1);
>>          }
>> -       if (length + offset > meminfo.size) {
>> -               fprintf(stderr, "Length %x + offset %x exceeds device 
>> size %x\n",
>> -                       length, offset, meminfo.size);
>> +       if (length + offset > mem_size) {
>> +               fprintf(stderr, "Length %llx + offset %llx exceeds 
>> device size %llx\n",
>> +                       length, offset, mem_size);
>>                  exit(1);
>>          }
>>
>>          wbuf = malloc(meminfo.erasesize * 3);
>>          if (!wbuf) {
>>                  fprintf(stderr, "Could not allocate %d bytes for 
>> buffer\n",
>> -                       meminfo.erasesize * 2);
>> +                       meminfo.erasesize * 3);
>>                  exit(1);
>>          }
>>          rbuf = wbuf + meminfo.erasesize;
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>
>> .
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Zhihao Cheng Aug. 17, 2023, 1:17 a.m. UTC | #3 
在 2023/8/16 22:07, Christian Wendt he/him 写道:
> [PATCH] mtd-utils: nandtest: handle large nand devices
> 
> Running nandtest on devices with sizes of 4Gb or more does not work, as
> the size returned in mtd_info_user is reported as 0 for 4Gb devices for
> example.
> 
> The patch uses sysfs to figure out the size of the nand device (as
> recommended in a comment in include/mtd/mtd-abi.h) and changes sizes
> and offsets to 64 bit values.
> 
> [From: Christian Wendt <cw@brainaid.de>]
> 

It would be better to send it in a patch format, otherwise there is a 
indent problem. Anyway,

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/nand-utils/nandtest.c b/nand-utils/nandtest.c
> index 06dec25..98217ba 100644
> --- a/nand-utils/nandtest.c
> +++ b/nand-utils/nandtest.c
> @@ -8,6 +8,7 @@
>   #include <string.h>
>   #include <time.h>
>   #include <unistd.h>
> +#include <limits.h>
>   #include <sys/stat.h>
>   #include <sys/ioctl.h>
>   #include <sys/types.h>
> @@ -145,6 +146,27 @@ static int erase_and_write(loff_t ofs, unsigned 
> char *data, unsigned char *rbuf,
>   }
> 
> 
> +static uint64_t get_mem_size(const char* device)
> +{
> +       const char* p = strrchr(device, '/');
> +       char path[PATH_MAX];
> +       int fd;
> +
> +       snprintf(path, sizeof(path), "/sys/class/mtd/%s/size", p);
> +       fd = open(path, O_RDONLY);
> +       if (fd >= 0) {
> +               char buffer[32];
> +               ssize_t n = read(fd, buffer, sizeof(buffer));
> +               close(fd);
> +               if (n > 0) {
> +                       return strtoull(buffer, NULL, 0);
> +               }
> +       }
> +
> +       fprintf(stderr, "Can't read size from %s\n", path);
> +       exit(1);
> +}
> +
>   /*
>    * Main program
>    */
> @@ -156,8 +178,9 @@ int main(int argc, char **argv)
>          int nr_passes = 1;
>          int nr_reads = 4;
>          int keep_contents = 0;
> -       uint32_t offset = 0;
> -       uint32_t length = -1;
> +       uint64_t offset = 0;
> +       uint64_t length = -1;
> +       uint64_t mem_size = 0;
>          int error = 0;
> 
>          seed = time(NULL);
> @@ -212,11 +235,11 @@ int main(int argc, char **argv)
>                          break;
> 
>                  case 'o':
> -                       offset = simple_strtoul(optarg, &error);
> +                       offset = simple_strtoull(optarg, &error);
>                          break;
> 
>                  case 'l':
> -                       length = simple_strtoul(optarg, &error);
> +                       length = simple_strtoull(optarg, &error);
>                          break;
> 
>                  }
> @@ -238,29 +261,31 @@ int main(int argc, char **argv)
>                  exit(1);
>          }
> 
> +       mem_size = get_mem_size(argv[optind]);
> +
>          if (length == -1)
> -               length = meminfo.size;
> +               length = mem_size;
> 
>          if (offset % meminfo.erasesize) {
> -               fprintf(stderr, "Offset %x not multiple of erase size 
> %x\n",
> +               fprintf(stderr, "Offset %llx not multiple of erase size 
> %x\n",
>                          offset, meminfo.erasesize);
>                  exit(1);
>          }
>          if (length % meminfo.erasesize) {
> -               fprintf(stderr, "Length %x not multiple of erase size 
> %x\n",
> +               fprintf(stderr, "Length %llx not multiple of erase size 
> %x\n",
>                          length, meminfo.erasesize);
>                  exit(1);
>          }
> -       if (length + offset > meminfo.size) {
> -               fprintf(stderr, "Length %x + offset %x exceeds device 
> size %x\n",
> -                       length, offset, meminfo.size);
> +       if (length + offset > mem_size) {
> +               fprintf(stderr, "Length %llx + offset %llx exceeds 
> device size %llx\n",
> +                       length, offset, mem_size);
>                  exit(1);
>          }
> 
>          wbuf = malloc(meminfo.erasesize * 3);
>          if (!wbuf) {
>                  fprintf(stderr, "Could not allocate %d bytes for 
> buffer\n",
> -                       meminfo.erasesize * 2);
> +                       meminfo.erasesize * 3);
>                  exit(1);
>          }
>          rbuf = wbuf + meminfo.erasesize;
>
diff mbox series

Patch

diff --git a/nand-utils/nandtest.c b/nand-utils/nandtest.c
index 06dec25..e60428f 100644
--- a/nand-utils/nandtest.c
+++ b/nand-utils/nandtest.c
@@ -8,6 +8,7 @@ 
  #include <string.h>
  #include <time.h>
  #include <unistd.h>
+#include <limits.h>
  #include <sys/stat.h>
  #include <sys/ioctl.h>
  #include <sys/types.h>
@@ -145,6 +146,26 @@  static int erase_and_write(loff_t ofs, unsigned 
char *data, unsigned char *rbuf,
  }


+static uint64_t get_mem_size(const char* device)
+{
+       const char* p = strrchr(device, '/');
+       char path[PATH_MAX];
+       int fd;
+
+       snprintf(path, sizeof(path), "/sys/class/mtd/%s/size", p);
+       fd = open(path, O_RDONLY);
+       if (fd) {
+               char buffer[32];
+               ssize_t n = read(fd, buffer, sizeof(buffer));
+               close(fd);
+               if (n > 0) {
+                       return strtoull(buffer, NULL, 0);
+               }
+       }
+
+       return 0;
+}
+
  /*
   * Main program