Patchwork [1/4] flash_info: convert to common.h

login
register
mail settings
Submitter Mike Frysinger
Date June 6, 2011, 6:22 p.m.
Message ID <1307384537-9607-1-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/98983/
State Accepted, archived
Headers show

Comments

Mike Frysinger - June 6, 2011, 6:22 p.m.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 flash_info.c |   27 +++++++++++++++++----------
 1 files changed, 17 insertions(+), 10 deletions(-)
Artem Bityutskiy - June 7, 2011, 4:31 a.m.
On Mon, 2011-06-06 at 14:22 -0400, Mike Frysinger wrote:
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  flash_info.c |   27 +++++++++++++++++----------
>  1 files changed, 17 insertions(+), 10 deletions(-)

How about killing this utility completely and use mtdinfo from ubi-utils
instead? Yes, before creating mtdinfo I should have noticed flash_info,
but I did not. Did you look at mtdinfo?
Mike Frysinger - June 7, 2011, 4:41 a.m.
On Tue, Jun 7, 2011 at 00:31, Artem Bityutskiy wrote:
> On Mon, 2011-06-06 at 14:22 -0400, Mike Frysinger wrote:
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>>  flash_info.c |   27 +++++++++++++++++----------
>>  1 files changed, 17 insertions(+), 10 deletions(-)
>
> How about killing this utility completely and use mtdinfo from ubi-utils
> instead? Yes, before creating mtdinfo I should have noticed flash_info,
> but I did not. Did you look at mtdinfo?

didnt even notice it :P

if mtdinfo provides all the details that my new flash_info does, then
i dont care about punting it.  be nice to merge my fixes before
deleting it though, otherwise i feel like i wasted quite a bit of time
for nothing ;).
-mike
Artem Bityutskiy - June 7, 2011, 4:42 a.m.
On Tue, 2011-06-07 at 00:41 -0400, Mike Frysinger wrote:
> On Tue, Jun 7, 2011 at 00:31, Artem Bityutskiy wrote:
> > On Mon, 2011-06-06 at 14:22 -0400, Mike Frysinger wrote:
> >> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> >> ---
> >>  flash_info.c |   27 +++++++++++++++++----------
> >>  1 files changed, 17 insertions(+), 10 deletions(-)
> >
> > How about killing this utility completely and use mtdinfo from ubi-utils
> > instead? Yes, before creating mtdinfo I should have noticed flash_info,
> > but I did not. Did you look at mtdinfo?
> 
> didnt even notice it :P
> 
> if mtdinfo provides all the details that my new flash_info does, then
> i dont care about punting it.  be nice to merge my fixes before
> deleting it though, otherwise i feel like i wasted quite a bit of time
> for nothing ;).

I think it provides everything except of region information, which
flash_info provides :)
Artem Bityutskiy - June 7, 2011, 4:45 a.m.
On Tue, 2011-06-07 at 07:42 +0300, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 00:41 -0400, Mike Frysinger wrote:
> > On Tue, Jun 7, 2011 at 00:31, Artem Bityutskiy wrote:
> > > On Mon, 2011-06-06 at 14:22 -0400, Mike Frysinger wrote:
> > >> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > >> ---
> > >>  flash_info.c |   27 +++++++++++++++++----------
> > >>  1 files changed, 17 insertions(+), 10 deletions(-)
> > >
> > > How about killing this utility completely and use mtdinfo from ubi-utils
> > > instead? Yes, before creating mtdinfo I should have noticed flash_info,
> > > but I did not. Did you look at mtdinfo?
> > 
> > didnt even notice it :P
> > 
> > if mtdinfo provides all the details that my new flash_info does, then
> > i dont care about punting it.  be nice to merge my fixes before
> > deleting it though, otherwise i feel like i wasted quite a bit of time
> > for nothing ;).
> 
> I think it provides everything except of region information, which
> flash_info provides :)

And it does not show a map of sectors. May be we can add this to mtdinfo
instead?
Mike Frysinger - June 7, 2011, 4:51 a.m.
On Tue, Jun 7, 2011 at 00:45, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 07:42 +0300, Artem Bityutskiy wrote:
>> On Tue, 2011-06-07 at 00:41 -0400, Mike Frysinger wrote:
>> > On Tue, Jun 7, 2011 at 00:31, Artem Bityutskiy wrote:
>> > > On Mon, 2011-06-06 at 14:22 -0400, Mike Frysinger wrote:
>> > >> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> > >> ---
>> > >>  flash_info.c |   27 +++++++++++++++++----------
>> > >>  1 files changed, 17 insertions(+), 10 deletions(-)
>> > >
>> > > How about killing this utility completely and use mtdinfo from ubi-utils
>> > > instead? Yes, before creating mtdinfo I should have noticed flash_info,
>> > > but I did not. Did you look at mtdinfo?
>> >
>> > didnt even notice it :P
>> >
>> > if mtdinfo provides all the details that my new flash_info does, then
>> > i dont care about punting it.  be nice to merge my fixes before
>> > deleting it though, otherwise i feel like i wasted quite a bit of time
>> > for nothing ;).
>>
>> I think it provides everything except of region information, which
>> flash_info provides :)
>
> And it does not show a map of sectors. May be we can add this to mtdinfo
> instead?

if you merge the patches i posted here, i'll see about extending
mtdinfo to include the new functionality and then i'll post another
patch to punt flash_info ;).
-mike
Artem Bityutskiy - June 7, 2011, 5:01 a.m.
On Tue, 2011-06-07 at 00:51 -0400, Mike Frysinger wrote:
> On Tue, Jun 7, 2011 at 00:45, Artem Bityutskiy wrote:
> > On Tue, 2011-06-07 at 07:42 +0300, Artem Bityutskiy wrote:
> >> On Tue, 2011-06-07 at 00:41 -0400, Mike Frysinger wrote:
> >> > On Tue, Jun 7, 2011 at 00:31, Artem Bityutskiy wrote:
> >> > > On Mon, 2011-06-06 at 14:22 -0400, Mike Frysinger wrote:
> >> > >> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> >> > >> ---
> >> > >>  flash_info.c |   27 +++++++++++++++++----------
> >> > >>  1 files changed, 17 insertions(+), 10 deletions(-)
> >> > >
> >> > > How about killing this utility completely and use mtdinfo from ubi-utils
> >> > > instead? Yes, before creating mtdinfo I should have noticed flash_info,
> >> > > but I did not. Did you look at mtdinfo?
> >> >
> >> > didnt even notice it :P
> >> >
> >> > if mtdinfo provides all the details that my new flash_info does, then
> >> > i dont care about punting it.  be nice to merge my fixes before
> >> > deleting it though, otherwise i feel like i wasted quite a bit of time
> >> > for nothing ;).
> >>
> >> I think it provides everything except of region information, which
> >> flash_info provides :)
> >
> > And it does not show a map of sectors. May be we can add this to mtdinfo
> > instead?
> 
> if you merge the patches i posted here, i'll see about extending
> mtdinfo to include the new functionality and then i'll post another
> patch to punt flash_info ;).

Sure, thanks, I've sent you some feed-back for the last patch, though.

Patch

diff --git a/flash_info.c b/flash_info.c
index f8b7efa..d4887da 100644
--- a/flash_info.c
+++ b/flash_info.c
@@ -12,24 +12,31 @@ 
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 
+#include "common.h"
 #include <mtd/mtd-user.h>
 
+static void usage(int status)
+{
+	fprintf(status ? stderr : stdout,
+		"Usage: %s <device>\n",
+		PROGRAM_NAME);
+	exit(status);
+}
+
 int main(int argc, char *argv[])
 {
 	int regcount;
 	int fd;
 
-	if (1 >= argc) {
-		fprintf(stderr, "Usage: %s device\n", PROGRAM_NAME);
-		return 16;
-	}
+	if (argc < 2)
+		usage(1);
+	if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
+		usage(0);
 
 	/* Open and size the device */
 	fd = open(argv[1], O_RDONLY);
-	if (fd < 0) {
-		fprintf(stderr, "File open error\n");
-		return 8;
-	}
+	if (fd < 0)
+		sys_errmsg_die("could not open: %s", argv[1]);
 
 	if (ioctl(fd, MEMGETREGIONCOUNT, &regcount) == 0) {
 		int i;
@@ -42,8 +49,8 @@  int main(int argc, char *argv[])
 						"has 0x%x blocks\n", i, reginfo.offset,
 						reginfo.erasesize, reginfo.numblocks);
 			} else {
-				printf("Strange can not read region %d from a %d region device\n",
-						i, regcount);
+				warnmsg("can not read region %d from a %d region device",
+					i, regcount);
 			}
 		}
 	}