Message ID | ce9ab5790912020630s160fee35r340d02ae9f41bcfb@mail.gmail.com |
---|---|
State | RFC |
Headers | show |
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.
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.
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.
--- 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); }
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> --- 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; } -