Re: [PATCH 5/6] mtdinfo: add regioninfo/sectormap display

[Mike Frysinger <vapier@gentoo.org>]

On Tue, Jun 7, 2011 at 03:41, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
>> @@ -58,14 +60,15 @@ static const char optionsstr[] =
>>  "-m, --mtdn=<MTD device number>  MTD device number to get information about\n"
>>  "-u, --ubi-info                  print what would UBI layout be if it was put\n"
>>  "                                on this MTD device\n"
>> +"-s, --sector-map                print sector map\n"
>>  "-a, --all                       print information about all MTD devices\n"
>>  "-h, --help                      print help message\n"
>>  "-V, --version                   print program version";
>>
>>  static const char usage[] =
>> -"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-h] [-V] [--mtdn <MTD device number>]\n"
>> +"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-s] [-h] [-V] [--mtdn <MTD device number>]\n"
>>  "\t\t[--ubi-info] [--help] [--version]\n"
>> -"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-h] [-V] [--ubi-info] [--help]\n"
>> +"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-s] [-h] [-V] [--ubi-info] [--help]\n"
>>  "\t\t[--version]\n"
>>  "Example 1: " PROGRAM_NAME " - (no arguments) print general MTD information\n"
>>  "Example 2: " PROGRAM_NAME " -m 1 - print information about MTD device number 1\n"
>> @@ -78,6 +81,7 @@ static const char usage[] =
>>  static const struct option long_options[] = {
>>       { .name = "mtdn",      .has_arg = 1, .flag = NULL, .val = 'm' },
>>       { .name = "ubi-info",  .has_arg = 0, .flag = NULL, .val = 'u' },
>> +     { .name = "sector-map",.has_arg = 0, .flag = NULL, .val = 's' },
>
> Is sector-map a good name?

it's what i'm used to calling it ;)

> May be we should call this feature "lock
> info" or something which suggests this is about detailed lock
> information? In this case you should also print an error message if the
> ISLOCKED ioctl is not supported. Probably adding a "islocked_supported"
> flag to 'struct mtd_info' (similarly to sysfs_supported) makes sense?
> Then you could just check this flag and refuse -s option?

honestly, i was more interested in the sector map than the ISLOCKED
info.  that was just a bonus for me.

> Also, could we avoid using term sector because it is overloaded and
> confusing. Could you please use term "eraseblock" instead (in prints and
> in internal names)?

ok

> Or you meant that this is like general per-sector info? So if we have
> more information to print, we add it there? In this case, could you
> please print "bad" or something for bad eraseblocks as well?

if MEMGETBADBLOCK provides that info, should be easy to add
-mike