[RFC] [PATCH] [MTD-UTILS]: flash_unlock: enhancing for unlocking of specified number of blocks

[RFC] [PATCH] [MTD-UTILS]: flash_unlock: enhancing for unlocking of specified number of blocks

[Vimal Singh]

This patch enhances the flash_unlock utility to be able to do
unlocking for specified blocks range.
This patch also fixes calculation of 'length' as in previous patch.

Say there are 240 blocks present in the device. Then:
offset starts from: 0x0
and full size of device: 0x1E00000

doing: 240 * 0x20000 gives -> 0x1E00000
But last block address should be 0x1DE0000 (which spans for 0x20000
bytes, adding up to size of 0x1E00000)

Signed-off-by: Vimal Singh <vimalsingh@ti.com>
---

--- a/flash_unlock.c	2009-11-24 19:33:18.000000000 +0530
+++ b/flash_unlock.c	2009-11-24 19:36:18.000000000 +0530
@@ -21,13 +21,14 @@ int main(int argc, char *argv[])
 	int fd;
 	struct mtd_info_user mtdInfo;
 	struct erase_info_user mtdLockInfo;
+	int count;

 	/*
 	 * Parse command line options
 	 */
-	if(argc != 2)
+	if(argc < 2)
 	{
-		fprintf(stderr, "USAGE: %s <mtd device>\n", argv[0]);
+		fprintf(stderr, "USAGE: %s <mtd device> <offset in hex> <block
count in decimal number>\n", argv[0]);
 		exit(1);
 	}
 	else if(strncmp(argv[1], "/dev/mtd", 8) != 0)
@@ -50,8 +51,18 @@ int main(int argc, char *argv[])
 		exit(1);
 	}

-	mtdLockInfo.start = 0;
-	mtdLockInfo.length = mtdInfo.size;
+	if (argc > 2)
+		mtdLockInfo.start = strtol(argv[2], NULL, 0);
+	else
+		mtdLockInfo.start = 0;
+
+	if (argc > 3) {
+		count = strtol(argv[3], NULL, 0);
+		mtdLockInfo.length = mtdInfo.erasesize * count;
+	} else {
+		mtdLockInfo.length = mtdInfo.size - mtdInfo.erasesize;
+	}
+
 	if(ioctl(fd, MEMUNLOCK, &mtdLockInfo))
 	{
 		fprintf(stderr, "Could not unlock MTD device: %s\n", argv[1]);
@@ -61,4 +72,3 @@ int main(int argc, char *argv[])

 	return 0;
 }
-

Re: [RFC] [PATCH] [MTD-UTILS]: flash_unlock: enhancing for unlocking of specified number of blocks

[Artem Bityutskiy]

On Wed, 2009-12-02 at 20:00 +0530, Vimal Singh wrote:
> This patch enhances the flash_unlock utility to be able to do
> unlocking for specified blocks range.
> This patch also fixes calculation of 'length' as in previous patch.
> 
> Say there are 240 blocks present in the device. Then:
> offset starts from: 0x0
> and full size of device: 0x1E00000
> 
> doing: 240 * 0x20000 gives -> 0x1E00000
> But last block address should be 0x1DE0000 (which spans for 0x20000
> bytes, adding up to size of 0x1E00000)
> 
> Signed-off-by: Vimal Singh <vimalsingh@ti.com>
> ---
> 
> --- a/flash_unlock.c	2009-11-24 19:33:18.000000000 +0530
> +++ b/flash_unlock.c	2009-11-24 19:36:18.000000000 +0530
> @@ -21,13 +21,14 @@ int main(int argc, char *argv[])
>  	int fd;
>  	struct mtd_info_user mtdInfo;
>  	struct erase_info_user mtdLockInfo;
> +	int count;
> 
>  	/*
>  	 * Parse command line options
>  	 */
> -	if(argc != 2)
> +	if(argc < 2)
>  	{
> -		fprintf(stderr, "USAGE: %s <mtd device>\n", argv[0]);
> +		fprintf(stderr, "USAGE: %s <mtd device> <offset in hex> <block

The patch looks fine for me, except that you should instead make these
to be some command line options.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [RFC] [PATCH] [MTD-UTILS]: flash_unlock: enhancing for unlocking of specified number of blocks

[Vimal Singh]

On 12/8/09, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Wed, 2009-12-02 at 20:00 +0530, Vimal Singh wrote:
> > This patch enhances the flash_unlock utility to be able to do
> > unlocking for specified blocks range.
> > This patch also fixes calculation of 'length' as in previous patch.
> >
> > Say there are 240 blocks present in the device. Then:
> > offset starts from: 0x0
> > and full size of device: 0x1E00000
> >
> > doing: 240 * 0x20000 gives -> 0x1E00000
> > But last block address should be 0x1DE0000 (which spans for 0x20000
> > bytes, adding up to size of 0x1E00000)
> >
> > Signed-off-by: Vimal Singh <vimalsingh@ti.com>
> > ---
> >
> > --- a/flash_unlock.c  2009-11-24 19:33:18.000000000 +0530
> > +++ b/flash_unlock.c  2009-11-24 19:36:18.000000000 +0530
> > @@ -21,13 +21,14 @@ int main(int argc, char *argv[])
> >       int fd;
> >       struct mtd_info_user mtdInfo;
> >       struct erase_info_user mtdLockInfo;
> > +     int count;
> >
> >       /*
> >        * Parse command line options
> >        */
> > -     if(argc != 2)
> > +     if(argc < 2)
> >       {
> > -             fprintf(stderr, "USAGE: %s <mtd device>\n", argv[0]);
> > +             fprintf(stderr, "USAGE: %s <mtd device> <offset in hex> <block
>
> The patch looks fine for me, except that you should instead make these
> to be some command line options.

I guess you did not go through the patch fully. The same is done in this patch.

-- 
Regards,
Vimal Singh

Re: [RFC] [PATCH] [MTD-UTILS]: flash_unlock: enhancing for unlocking of specified number of blocks

[Artem Bityutskiy]

On Tue, 2009-12-08 at 20:33 -0800, Vimal Singh wrote:
> On 12/8/09, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > On Wed, 2009-12-02 at 20:00 +0530, Vimal Singh wrote:
> > > This patch enhances the flash_unlock utility to be able to do
> > > unlocking for specified blocks range.
> > > This patch also fixes calculation of 'length' as in previous patch.
> > >
> > > Say there are 240 blocks present in the device. Then:
> > > offset starts from: 0x0
> > > and full size of device: 0x1E00000
> > >
> > > doing: 240 * 0x20000 gives -> 0x1E00000
> > > But last block address should be 0x1DE0000 (which spans for 0x20000
> > > bytes, adding up to size of 0x1E00000)
> > >
> > > Signed-off-by: Vimal Singh <vimalsingh@ti.com>
> > > ---
> > >
> > > --- a/flash_unlock.c  2009-11-24 19:33:18.000000000 +0530
> > > +++ b/flash_unlock.c  2009-11-24 19:36:18.000000000 +0530
> > > @@ -21,13 +21,14 @@ int main(int argc, char *argv[])
> > >       int fd;
> > >       struct mtd_info_user mtdInfo;
> > >       struct erase_info_user mtdLockInfo;
> > > +     int count;
> > >
> > >       /*
> > >        * Parse command line options
> > >        */
> > > -     if(argc != 2)
> > > +     if(argc < 2)
> > >       {
> > > -             fprintf(stderr, "USAGE: %s <mtd device>\n", argv[0]);
> > > +             fprintf(stderr, "USAGE: %s <mtd device> <offset in hex> <block
> >
> > The patch looks fine for me, except that you should instead make these
> > to be some command line options.
> 
> I guess you did not go through the patch fully. The same is done in this patch.

No, I looked. What I meant was doing:

flash_unlock /dev/mtdZ --offset=X --block=Y

or

flash_unlock /dev/mtdZ -b Y -o X

instead of

flash_unlock /dev/mtdZ X Y

Obviously the first one is cleaner and is easier to extend, and you do
not have so strict dependency on the X and Y order, and this is more
readable, and less error-prone.

However, I glanced to flash_lock, and it uses similar bad style, so I
guess it is ok if flash_unlock is symmetric. Thus, I've pushed this
patch, thank you!

****** NB !!!! *******
Your patch was again line-wrapped. How many times I complained about
this??? I've fixed it up, but really, it not funny already.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)