Patchwork [U-Boot,5/7] JFFS2: Change DEFAULT_EMPTY_SCAN_SIZE to 256 Bytes

login
register
mail settings
Submitter Baidu Boy
Date April 24, 2011, 3:39 a.m.
Message ID <002001cc0231$44174310$6401a8c0@LENOVOE5CA6843>
Download mbox | patch
Permalink /patch/92634/
State New
Delegated to: Detlev Zundel
Headers show

Comments

Baidu Boy - April 24, 2011, 3:39 a.m.
1/ Syncs up with jffs2 in the linux kernel:
 If the first 256 Bytes is 0xff,we get the conclusion
 that the sector is empty.

Signed-off-by: Baidu Liu <liucai.lfn@gmail.com>
---
 fs/jffs2/jffs2_1pass.c      |   11 ++++++-----
 fs/jffs2/jffs2_nand_1pass.c |   13 ++++++-------
 include/jffs2/jffs2.h       |    2 ++
 3 files changed, 14 insertions(+), 12 deletions(-)
Detlev Zundel - April 29, 2011, 1:27 p.m.
Hi Baidu,

>  1/ Syncs up with jffs2 in the linux kernel:
>  If the first 256 Bytes is 0xff,we get the conclusion
>  that the sector is empty.
>
> Signed-off-by: Baidu Liu <liucai.lfn@gmail.com>
> ---
>  fs/jffs2/jffs2_1pass.c      |   11 ++++++-----
>  fs/jffs2/jffs2_nand_1pass.c |   13 ++++++-------
>  include/jffs2/jffs2.h       |    2 ++
>  3 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
> index b3d94af..62ba250 100644
> --- a/fs/jffs2/jffs2_1pass.c
> +++ b/fs/jffs2/jffs2_1pass.c
> @@ -801,7 +801,7 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
>  #endif
>  				default:
>  					/* unknown */
> -					putLabeledWord("UNKOWN COMPRESSION METHOD = ", jNode->compr);
> +					putLabeledWord("UNKNOWN COMPRESSION METHOD = ", jNode->compr);
>  					put_fl_mem(jNode, pL->readbuf);
>  					return -1;
>  					break;

This typo change is not mentioned in the change log and really does not
belong here.  Please put it into a separate changeset.

[...]

> diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c
> index 885fa3c..5afe779 100644
> --- a/fs/jffs2/jffs2_nand_1pass.c
> +++ b/fs/jffs2/jffs2_nand_1pass.c
> @@ -351,7 +351,7 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 ino, char *dest,
>  #endif
>  			default:
>  				/* unknown */
> -				putLabeledWord("UNKOWN COMPRESSION METHOD = ", inode->compr);
> +				putLabeledWord("UNKNOWN COMPRESSION METHOD = ", inode->compr);
>  				return -1;
>  			}
>  		}

Dito.

Cheers
  Detlev
Baidu Boy - April 29, 2011, 3:13 p.m.
Hi, Detlev :

>>                               default:
>>                                       /* unknown */
>> -                                     putLabeledWord("UNKOWN COMPRESSION METHOD = ", jNode->compr);
>> +                                     putLabeledWord("UNKNOWN COMPRESSION METHOD = ", jNode->compr);
>>                                       put_fl_mem(jNode, pL->readbuf);
>>                                       return -1;
>>                                       break;
>
> This typo change is not mentioned in the change log and really does not
> belong here.  Please put it into a separate changeset.
>
This is just the typo error correction. Do you think we really need
another patch?
Baidu Boy - April 30, 2011, 1:04 a.m.
Hi,Detlev

2011/4/30 Detlev Zundel <dzu@denx.de>:
> Hi Baidu,
>
>> Hi,Detlev
>>
>>>>                               default:
>>>>                                       /* unknown */
>>>> -                                     putLabeledWord("UNKOWN
>>>> COMPRESSION METHOD = ", jNode->compr);
>>>> +                                     putLabeledWord("UNKNOWN
>>>> COMPRESSION METHOD = ", jNode->compr);
>>>>                                       put_fl_mem(jNode, pL->readbuf);
>>>>                                       return -1;
>>>>                                       break;
>>>
>>> This typo change is not mentioned in the change log and really does not
>>> belong here.  Please put it into a separate changeset.
>>
>> This is just the typo error correction. Do you think we really need
>> another patch?
>
> A patch description should include _all_ changes.  Either you also put
> that change into the patch description or you make a separate change.
> Doing changes not described in the commit log are a good sign to show
> that one does not care what one does.

Yes, you are right. Add all the description in the commit log.
But please firstly make this principle be applied for all guys
submiting patch. Not just to the people rarely summitint patch.
Be fair.
Wolfgang Denk - April 30, 2011, 7:34 a.m.
Dear Baidu Liu,

In message <BANLkTin7REQN=pv1yTHLFOWrqToj2pafaw@mail.gmail.com> you wrote:
> 
> > A patch description should include _all_ changes.  Either you also put
> > that change into the patch description or you make a separate change.
> > Doing changes not described in the commit log are a good sign to show
> > that one does not care what one does.
> 
> Yes, you are right. Add all the description in the commit log.
> But please firstly make this principle be applied for all guys
> submiting patch. Not just to the people rarely summitint patch.
> Be fair.

We try to be.  We realy strive hard to apply the same, consequent
policy to all submitters. You may be "lucky" and have a poor patch
slip though because nobody foundenough time for a thorough review, but
this is the exception. Or should be.  In any case, when a (new) problem
gets noted, it has to be fixed.

Best regards,

Wolfgang Denk

Patch

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index b3d94af..62ba250 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -801,7 +801,7 @@  jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 #endif
 				default:
 					/* unknown */
-					putLabeledWord("UNKOWN COMPRESSION METHOD = ", jNode->compr);
+					putLabeledWord("UNKNOWN COMPRESSION METHOD = ", jNode->compr);
 					put_fl_mem(jNode, pL->readbuf);
 					return -1;
 					break;
@@ -1442,8 +1442,6 @@  dump_dirents(struct b_lists *pL)
 }
 #endif
 
-#define DEFAULT_EMPTY_SCAN_SIZE	4096
-
 static inline uint32_t EMPTY_SCAN_SIZE(uint32_t sector_size)
 {
 	if (sector_size < DEFAULT_EMPTY_SCAN_SIZE)
@@ -1560,14 +1558,17 @@  jffs2_1pass_build_lists(struct part_info * part)
 		/* We temporarily use 'ofs' as a pointer into the buffer/jeb */
 		ofs = 0;
 
-		/* Scan only 4KiB of 0xFF before declaring it's empty */
+		/* Scan only DEFAULT_EMPTY_SCAN_SIZE of 0xFF before declaring it's empty */
 		while (ofs < EMPTY_SCAN_SIZE(part->sector_size) &&
 				*(uint32_t *)(&buf[ofs]) == 0xFFFFFFFF)
 			ofs += 4;
 
-		if (ofs == EMPTY_SCAN_SIZE(part->sector_size))
+		if (ofs == EMPTY_SCAN_SIZE(part->sector_size)) {
+			printf("Block at 0x%08x is empty (erased)\n", sector_ofs);
 			continue;
+		}
 
+		/* Now ofs is a complete physical flash offset as it always was... */
 		ofs += sector_ofs;
 		prevofs = ofs - 1;
 
diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c
index 885fa3c..5afe779 100644
--- a/fs/jffs2/jffs2_nand_1pass.c
+++ b/fs/jffs2/jffs2_nand_1pass.c
@@ -351,7 +351,7 @@  jffs2_1pass_read_inode(struct b_lists *pL, u32 ino, char *dest,
 #endif
 			default:
 				/* unknown */
-				putLabeledWord("UNKOWN COMPRESSION METHOD = ", inode->compr);
+				putLabeledWord("UNKNOWN COMPRESSION METHOD = ", inode->compr);
 				return -1;
 			}
 		}
@@ -789,7 +789,6 @@  jffs2_fill_scan_buf(nand_info_t *nand, unsigned char *buf,
 	return 0;
 }
 
-#define	EMPTY_SCAN_SIZE	1024
 static u32
 jffs2_1pass_build_lists(struct part_info * part)
 {
@@ -828,17 +827,17 @@  jffs2_1pass_build_lists(struct part_info * part)
 		if (nand_block_isbad(nand, offset))
 			continue;
 
-		if (jffs2_fill_scan_buf(nand, buf, offset, EMPTY_SCAN_SIZE))
+		if (jffs2_fill_scan_buf(nand, buf, offset, DEFAULT_EMPTY_SCAN_SIZE))
 			return 0;
 
 		ofs = 0;
-		/* Scan only 4KiB of 0xFF before declaring it's empty */
-		while (ofs < EMPTY_SCAN_SIZE && *(uint32_t *)(&buf[ofs]) == 0xFFFFFFFF)
+		/* Scan only DEFAULT_EMPTY_SCAN_SIZE of 0xFF before declaring it's empty */
+		while (ofs < DEFAULT_EMPTY_SCAN_SIZE && *(uint32_t *)(&buf[ofs]) == 0xFFFFFFFF)
 			ofs += 4;
-		if (ofs == EMPTY_SCAN_SIZE)
+		if (ofs == DEFAULT_EMPTY_SCAN_SIZE)
 			continue;
 
-		if (jffs2_fill_scan_buf(nand, buf + EMPTY_SCAN_SIZE, offset + EMPTY_SCAN_SIZE, sectorsize - EMPTY_SCAN_SIZE))
+		if (jffs2_fill_scan_buf(nand, buf + DEFAULT_EMPTY_SCAN_SIZE, offset + DEFAULT_EMPTY_SCAN_SIZE, sectorsize - DEFAULT_EMPTY_SCAN_SIZE))
 			return 0;
 		offset += ofs;
 
diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
index 5b006c0..9fb89b3 100644
--- a/include/jffs2/jffs2.h
+++ b/include/jffs2/jffs2.h
@@ -51,6 +51,8 @@  CONFIG_JFFS2_SUMMARY is enabled.
 #endif
 #endif
 
+#define DEFAULT_EMPTY_SCAN_SIZE	256
+
 #define JFFS2_SUPER_MAGIC 0x72b6
 
 /* Values we may expect to find in the 'magic' field */