[U-Boot] [PATCH v2 0/2] ext2: Cleanup and simplify sector access code

[U-Boot] [PATCH v2 0/2] ext2: Cleanup and simplify sector access code

[Anton Staaf]

This patch set first cleans up all of the checkpatch warnings
and errors in fs/ext2/dev.c and then cleans up the partial sector
access logic in the ext2fs_devread function.

I didn't see a file system or ext2 custodian so I've CC'ed Andy
the custodian for MMC as that seems closest.  Please let me know
if I should bring this to someone elses attention.

changes in v2
- Update commit message to include mention of printf and syle cleanup

Signed-off-by: Anton Staaf <robotboy@chromium.org>
Cc: Andy Fleming <afleming@freescale.com>
Cc: Detlev Zundel <dzu@denx.de>

Anton Staaf (2):
  ext2: Fix checkpatch violations
  ext2: Simplify partial sector access logic

 fs/ext2/dev.c |  112 ++++++++++++++++++++++++++------------------------------
 1 files changed, 52 insertions(+), 60 deletions(-)

-- 
1.7.3.1

[U-Boot] [PATCH v2 1/2] ext2: Fix checkpatch violations

[Anton Staaf]

Fix all checkpatch violations in the low level Ext2 block
device reading code.  This is done in preparation for cleaning
up the partial sector access code.

Also replace hard coded function names in printfs with __func__
macro, and correctly indent comments for style consistency
not captured by checkpatch.

Signed-off-by: Anton Staaf <robotboy@chromium.org>
Cc: Andy Fleming <afleming@freescale.com>
Cc: Detlev Zundel <dzu@denx.de>
---
 fs/ext2/dev.c |   82 ++++++++++++++++++++++++++++++---------------------------
 1 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/fs/ext2/dev.c b/fs/ext2/dev.c
index 3b49650..4365b3b 100644
--- a/fs/ext2/dev.c
+++ b/fs/ext2/dev.c
@@ -31,7 +31,7 @@
 static block_dev_desc_t *ext2fs_block_dev_desc;
 static disk_partition_t part_info;
 
-int ext2fs_set_blk_dev (block_dev_desc_t * rbdd, int part)
+int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part)
 {
 	ext2fs_block_dev_desc = rbdd;
 
@@ -46,51 +46,55 @@ int ext2fs_set_blk_dev (block_dev_desc_t * rbdd, int part)
 			return 0;
 		}
 	}
-	return (part_info.size);
+	return part_info.size;
 }
 
 
-int ext2fs_devread (int sector, int byte_offset, int byte_len, char *buf) {
+int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf)
+{
 	char sec_buf[SECTOR_SIZE];
 	unsigned block_len;
 
-/*
- *  Check partition boundaries
- */
-	if ((sector < 0)
-	    || ((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS)) >=
+	/*
+	 *  Check partition boundaries
+	 */
+	if ((sector < 0) ||
+	    ((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS)) >=
 		part_info.size)) {
-	/*      errnum = ERR_OUTSIDE_PART; */
-		printf (" ** ext2fs_devread() read outside partition sector %d\n", sector);
-		return (0);
+		/* errnum = ERR_OUTSIDE_PART; */
+		printf(" ** %s read outside partition sector %d\n",
+		       __func__,
+		       sector);
+		return 0;
 	}
 
-/*
- *  Get the read to the beginning of a partition.
- */
+	/*
+	 *  Get the read to the beginning of a partition.
+	 */
 	sector += byte_offset >> SECTOR_BITS;
 	byte_offset &= SECTOR_SIZE - 1;
 
-	debug (" <%d, %d, %d>\n", sector, byte_offset, byte_len);
+	debug(" <%d, %d, %d>\n", sector, byte_offset, byte_len);
 
 	if (ext2fs_block_dev_desc == NULL) {
-		printf ("** Invalid Block Device Descriptor (NULL)\n");
-		return (0);
+		printf(" ** %s Invalid Block Device Descriptor (NULL)\n",
+		       __func__);
+		return 0;
 	}
 
 	if (byte_offset != 0) {
 		/* read first part which isn't aligned with start of sector */
 		if (ext2fs_block_dev_desc->
-		    block_read (ext2fs_block_dev_desc->dev,
-				part_info.start + sector, 1,
-				(unsigned long *) sec_buf) != 1) {
-			printf (" ** ext2fs_devread() read error **\n");
-			return (0);
+		    block_read(ext2fs_block_dev_desc->dev,
+			       part_info.start + sector, 1,
+			       (unsigned long *) sec_buf) != 1) {
+			printf(" ** %s read error **\n", __func__);
+			return 0;
 		}
-		memcpy (buf, sec_buf + byte_offset,
-			min (SECTOR_SIZE - byte_offset, byte_len));
-		buf += min (SECTOR_SIZE - byte_offset, byte_len);
-		byte_len -= min (SECTOR_SIZE - byte_offset, byte_len);
+		memcpy(buf, sec_buf + byte_offset,
+		       min(SECTOR_SIZE - byte_offset, byte_len));
+		buf += min(SECTOR_SIZE - byte_offset, byte_len);
+		byte_len -= min(SECTOR_SIZE - byte_offset, byte_len);
 		sector++;
 	}
 
@@ -111,13 +115,13 @@ int ext2fs_devread (int sector, int byte_offset, int byte_len, char *buf) {
 		return 1;
 	}
 
-	if (ext2fs_block_dev_desc->block_read (ext2fs_block_dev_desc->dev,
-					       part_info.start + sector,
-					       block_len / SECTOR_SIZE,
-					       (unsigned long *) buf) !=
+	if (ext2fs_block_dev_desc->block_read(ext2fs_block_dev_desc->dev,
+					      part_info.start + sector,
+					      block_len / SECTOR_SIZE,
+					      (unsigned long *) buf) !=
 	    block_len / SECTOR_SIZE) {
-		printf (" ** ext2fs_devread() read error - block\n");
-		return (0);
+		printf(" ** %s read error - block\n", __func__);
+		return 0;
 	}
 	block_len = byte_len & ~(SECTOR_SIZE - 1);
 	buf += block_len;
@@ -127,13 +131,13 @@ int ext2fs_devread (int sector, int byte_offset, int byte_len, char *buf) {
 	if (byte_len != 0) {
 		/* read rest of data which are not in whole sector */
 		if (ext2fs_block_dev_desc->
-		    block_read (ext2fs_block_dev_desc->dev,
-				part_info.start + sector, 1,
-				(unsigned long *) sec_buf) != 1) {
-			printf (" ** ext2fs_devread() read error - last part\n");
-			return (0);
+		    block_read(ext2fs_block_dev_desc->dev,
+			       part_info.start + sector, 1,
+			       (unsigned long *) sec_buf) != 1) {
+			printf(" ** %s read error - last part\n", __func__);
+			return 0;
 		}
-		memcpy (buf, sec_buf, byte_len);
+		memcpy(buf, sec_buf, byte_len);
 	}
-	return (1);
+	return 1;
 }
-- 
1.7.3.1

[U-Boot] [PATCH v2 2/2] ext2: Simplify partial sector access logic

[Anton Staaf]

Previously reading zero full sectors (reading the end of one
sector and the beginning of the next for example) was special
cased and involved stack allocating a second sector buffer.  This
change uses the same code path for this case as well as when there
are a non-zero number of full sectors to access.  The result is
easier to read and reduces the maximum stack used.

Signed-off-by: Anton Staaf <robotboy@chromium.org>
Cc: Andy Fleming <afleming@freescale.com>
Cc: Detlev Zundel <dzu@denx.de>
---
 fs/ext2/dev.c |   42 +++++++++++++++---------------------------
 1 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/fs/ext2/dev.c b/fs/ext2/dev.c
index 4365b3b..78851d0 100644
--- a/fs/ext2/dev.c
+++ b/fs/ext2/dev.c
@@ -53,7 +53,7 @@ int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part)
 int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 {
 	char sec_buf[SECTOR_SIZE];
-	unsigned block_len;
+	unsigned sectors;
 
 	/*
 	 *  Check partition boundaries
@@ -98,35 +98,23 @@ int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 		sector++;
 	}
 
-	if (byte_len == 0)
-		return 1;
-
 	/*  read sector aligned part */
-	block_len = byte_len & ~(SECTOR_SIZE - 1);
-
-	if (block_len == 0) {
-		u8 p[SECTOR_SIZE];
-
-		block_len = SECTOR_SIZE;
-		ext2fs_block_dev_desc->block_read(ext2fs_block_dev_desc->dev,
-						  part_info.start + sector,
-						  1, (unsigned long *)p);
-		memcpy(buf, p, byte_len);
-		return 1;
-	}
+	sectors = byte_len / SECTOR_SIZE;
+
+	if (sectors > 0) {
+		if (ext2fs_block_dev_desc->block_read(
+			ext2fs_block_dev_desc->dev,
+			part_info.start + sector,
+			sectors,
+			(unsigned long *) buf) != sectors) {
+			printf(" ** %s read error - block\n", __func__);
+			return 0;
+		}
 
-	if (ext2fs_block_dev_desc->block_read(ext2fs_block_dev_desc->dev,
-					      part_info.start + sector,
-					      block_len / SECTOR_SIZE,
-					      (unsigned long *) buf) !=
-	    block_len / SECTOR_SIZE) {
-		printf(" ** %s read error - block\n", __func__);
-		return 0;
+		buf += sectors * SECTOR_SIZE;
+		byte_len -= sectors * SECTOR_SIZE;
+		sector += sectors;
 	}
-	block_len = byte_len & ~(SECTOR_SIZE - 1);
-	buf += block_len;
-	byte_len -= block_len;
-	sector += block_len / SECTOR_SIZE;
 
 	if (byte_len != 0) {
 		/* read rest of data which are not in whole sector */
-- 
1.7.3.1

[U-Boot] [PATCH v2 1/2] ext2: Fix checkpatch violations

[Detlev Zundel]

Hi Anton,

> Fix all checkpatch violations in the low level Ext2 block
> device reading code.  This is done in preparation for cleaning
> up the partial sector access code.
>
> Also replace hard coded function names in printfs with __func__
> macro, and correctly indent comments for style consistency
> not captured by checkpatch.
>
> Signed-off-by: Anton Staaf <robotboy@chromium.org>
> Cc: Andy Fleming <afleming@freescale.com>
> Cc: Detlev Zundel <dzu@denx.de>

Thanks for the re-spin.

Acked-by: Detlev Zundel <dzu@denx.de>

-- 
While  the list of  different methods is not  endless, it is certainly
pretty long, with new ones being dreamed up all the time. Fortunately,
space limitations prevent us from looking at all of them.
                                            -- Andrew S. Tanenbaum
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 2/2] ext2: Simplify partial sector access logic

[Detlev Zundel]

Hi Anton,

> Previously reading zero full sectors (reading the end of one
> sector and the beginning of the next for example) was special
> cased and involved stack allocating a second sector buffer.  This
> change uses the same code path for this case as well as when there
> are a non-zero number of full sectors to access.  The result is
> easier to read and reduces the maximum stack used.
>
> Signed-off-by: Anton Staaf <robotboy@chromium.org>
> Cc: Andy Fleming <afleming@freescale.com>
> Cc: Detlev Zundel <dzu@denx.de>

Acked-by: Detlev Zundel <dzu@denx.de>

-- 
Of course my password is the same as my pet's name
My macaw's name was Q47pY!3 and I change it every 90 days
                        -- Trevor Linton
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 1/2] ext2: Fix checkpatch violations

[Wolfgang Denk]

Dear Anton Staaf,

In message <1309459537-312-2-git-send-email-robotboy@chromium.org> you wrote:
> Fix all checkpatch violations in the low level Ext2 block
> device reading code.  This is done in preparation for cleaning
> up the partial sector access code.
> 
> Also replace hard coded function names in printfs with __func__
> macro, and correctly indent comments for style consistency
> not captured by checkpatch.
> 
> Signed-off-by: Anton Staaf <robotboy@chromium.org>
> Cc: Andy Fleming <afleming@freescale.com>
> Cc: Detlev Zundel <dzu@denx.de>
> ---
>  fs/ext2/dev.c |   82 ++++++++++++++++++++++++++++++---------------------------
>  1 files changed, 43 insertions(+), 39 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Half of the people in the world are below average.

[U-Boot] [PATCH v2 2/2] ext2: Simplify partial sector access logic

[Wolfgang Denk]

Dear Anton Staaf,

In message <1309459537-312-3-git-send-email-robotboy@chromium.org> you wrote:
> Previously reading zero full sectors (reading the end of one
> sector and the beginning of the next for example) was special
> cased and involved stack allocating a second sector buffer.  This
> change uses the same code path for this case as well as when there
> are a non-zero number of full sectors to access.  The result is
> easier to read and reduces the maximum stack used.
> 
> Signed-off-by: Anton Staaf <robotboy@chromium.org>
> Cc: Andy Fleming <afleming@freescale.com>
> Cc: Detlev Zundel <dzu@denx.de>
> ---
>  fs/ext2/dev.c |   42 +++++++++++++++---------------------------
>  1 files changed, 15 insertions(+), 27 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I can type faster than I can move a  mouse,  so  I  find  menu-driven
drawing packages time consuming and frustrating.      - W. R. Stevens