ROMFS
ROMFS: Add support for ROMFS filesystem based on MTD system.
Patch is in attachement.
Best regards,
Michal Simek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: romfs.patch
Type: application/octet-stream
Size: 13132 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070620/441a2386/attachment.obj
Quoted 6 lines · In message <003901c7b366$e4c09190$0200a8c0@monstrone> you wrote:
ROMFS: Add support for ROMFS filesystem based on MTD system.
Signed-off-by Michal Simek <[email protected]>Patch is in attachement.
Please cleanup coding style violations (trailing white space, indentation not by TAB etc.) and resubmit.
Best regards,
Wolfgang Denk
Signature · 5 lines
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
Don't tell me how hard you work. Tell me how much you get done.
-- James J. LingQuoted 2 lines
Please cleanup coding style violations (trailing white space, indentation not by TAB etc.) and resubmit.
I hope you thought Use TAB characters for indentation, not spaces. (http://www.denx.de/wiki/UBoot/CodingStyle)
I resolve some coding style violations.
Best regards,
Michal Simek
Quoted 4 lines
Quoted 4 lines
ROMFS: Add support for ROMFS filesystem based on MTD system.
Signed-off-by Michal Simek <[email protected]>
Quoted 14 lines
Quoted 1 line
Patch is in attachement.
Please cleanup coding style violations (trailing white space, indentation not by TAB etc.) and resubmit.
Best regards,
Wolfgang Denk
Signature · 4 lines
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 Don't tell me how hard you work. Tell me how much you get done.
v -- James J. Ling
-------------- next part --------------
A non-text attachment was scrubbed...
Name: romfs2.patch
Type: text/x-patch
Size: 12765 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070621/287b5fb6/attachment.bin
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 3 lines
Please cleanup coding style violations (trailing white space, indentation not by TAB etc.) and resubmit.
I hope you thought Use TAB characters for indentation, not spaces. (http://www.denx.de/wiki/UBoot/CodingStyle)
I mean what I wrote: your patch contained coding style violations; one of these was that indentation was NOT done by TABs.
Quoted 1 line
I resolve some coding style violations.
Then please also remove the remaining ones now (trailing white space in common/cmd_jffs2.c) and resubmit again.
Best regards,
Wolfgang Denk
Signature · 7 lines
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
Brontosaurus Principle: Organizations can grow faster than their
brains can manage them in relation to their environment and to their
own physiology: when this occurs, they are an endangered species.
- Thomas K. ConnellanQuoted 2 lines
Then please also remove the remaining ones now (trailing white space in
common/cmd_jffs2.c) and resubmit again.
I checked all files on trailing white space. I don't see them.
Best regards
Michal Simek
[microblaze at monstr u-boot]$ patch -p1 < ../romfs3.patch patching file Makefile
patching file common/cmd_jffs2.c
patching file fs/Makefile
patching file fs/romfs/Makefile
patching file fs/romfs/romfs.c
[microblaze at monstr u-boot]$ cat Makefile | grep " $" [microblaze at monstr u-boot]$ cat common/cmd_jffs2.c | grep " $" [microblaze at monstr u-boot]$ cat fs/Makefile | grep " $" [microblaze at monstr u-boot]$ cat fs/romfs/Makefile | grep " $" [microblaze at monstr u-boot]$ cat fs/romfs/romfs.c | grep " $" [microblaze at monstr u-boot]$
ROMFS: Add support for ROMFS filesystem based on MTD system.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: romfs3.patch
Type: text/x-patch
Size: 14377 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070623/a7dfb3cf/attachment.bin
Hello,
Quoted 5 lines · in message <[email protected]> you wrote:
Quoted 3 lines
Then please also remove the remaining ones now (trailing white space in
common/cmd_jffs2.c) and resubmit again.I checked all files on trailing white space. I don't see them.
I know. That's the problem with white space - you don't see it. Printing characters are better visible ;-)
Quoted 11 lines
[microblaze at monstr u-boot]$ patch -p1 <
../romfs3.patchpatching fileMakefile
patching filecommon/cmd_jffs2.c
patching file fs/Makefile
patching filefs/romfs/Makefile
patching filefs/romfs/romfs.c
[microblaze at monstr u-boot]$ catMakefile| grep " $" [microblaze at monstr u-boot]$ catcommon/cmd_jffs2.c| grep " $" [microblaze at monstr u-boot]$ cat fs/Makefile | grep " $" [microblaze at monstr u-boot]$ catfs/romfs/Makefile| grep " $" [microblaze at monstr u-boot]$ catfs/romfs/romfs.c| grep " $"
This gives you 5 UUOCA in a row, whichis kind of a record, but it does not help much - for example, you don't find any trailing TABs that way.
Quoted 6 lines
...
common/cmd_jffs2.c +0
-0@@ -1961,13 +1975,18 @@ int do_jffs2_fsinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])if ((part =jffs2_part_info (current_dev, current_partnum))){
common/cmd_jffs2.c int do_jffs2_fsinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) · line 1983
Quoted 10 lines
common/cmd_jffs2.c +3
-2/* check partition type for cramfs */ - fsname = (cramfs_check (part) ?"CRAMFS" :"JFFS2" ); -printf ("### filesystem type is %s\n" , fsname); +puts ("### filesystem type is " );if (cramfs_check (part)) { +puts ("CRAMFS\n" ); ret =cramfs_info (part); +
vi's "l" command shows this line as "+^I^I^I$", i. e. 3 trailing TABs.
Best regards,
Wolfgang Denk
Signature · 4 lines
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 You cannot propel yourself forward by patting yourself on the back.
Quoted 2 lines
I know. That's the problem with white space - you don't see it. Printing characters are better visible ;-)
yes, you are right. Some characters are really better visible :-)
Quoted 4 lines
This gives you 5 UUOCA in a row, whichis kind of a record, but it does not help much - for example, you don't find any trailing TABs that way.
vi's "l" command shows this line as "+^I^I^I$", i. e. 3 trailing TABs.
I know this command (set list).
I hope that 3 TABS in one row was the last fault. :-)
Best regards
Michal Simek
ROMFS: Add support for ROMFS filesystem based on MTD system.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: romfs4.patch
Type: text/x-patch
Size: 14372 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070623/661aa822/attachment.bin
Quoted 1 line · Wolfgang Denk
vi's "l" command shows this line as "+^I^I^I$", i. e. 3 trailing TABs.
and vim's "let c_space_errors=1" will show all such spacing errors in red (beware, some u-boot's files will really burn your eyes ;-))
Best regards,
ladis
Quoted 8 lines · Wolfgang Denk
Quoted 1 line
vi's "l" command shows this line as "+^I^I^I$", i. e. 3 trailing TABs.
and vim's "let c_space_errors=1" will show all such spacing errors in red (beware, some u-boot's files will really burn your eyes ;-))
Thanks.
Vim can find all trailing white spaces with commands "/\t$\| $".
Best regards
Michal Simek
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 10 lines · Michal Simek
Quoted 6 lines · Wolfgang Denk
Quoted 1 line
vi's "l" command shows this line as "+^I^I^I$", i. e. 3 trailing TABs.
and vim's "let c_space_errors=1" will show all such spacing errors in red (beware, some u-boot's files will really burn your eyes ;-))
Thanks.
Vim can find all trailing white spaces with commands "/\t$\| $".
Or just "%s/\s\+$//" and be done with it...
- -Cory
- --
Cory T. Tusar
Embedded Systems Engineer
Videon Central, Inc.
2171 Sandy Drive
State College, PA 16801
(814) 235-1111 x316
(814) 235-1118 fax
"There are two ways of constructing a software design. One way is to
make it so simple that there are obviously no deficiencies, and the other way is to make it so complicated that there are no obvious deficiencies." --Sir Charles Anthony Richard Hoare
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
iD8DBQFGl47YHT1tsfGwHJ8RAgKjAJ4rGeWcvQfWDTcBo/W+2dx9vFJOzwCeP6nU i7/iqkgC9DbvbYS0qNiAx7Y=
=A4Kx
-----END PGP SIGNATURE-----
Quoted 4 lines · Monstr at seznam.cz
Quoted 2 lines
Then please also remove the remaining ones now (trailing white space in
common/cmd_jffs2.c) and resubmit again.I checked all files on trailing white space. I don't see them.
FWIW, I like to run all my code through scripts/Lindent (in the Linux kernel tree) before submitting. It takes care of all that fiddly stuff so I don't have to go searching for things.
Cheers,
g.
Signature · 4 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. grant.likely at secretlab.ca (403) 399-0195
Quoted 4 lines
FWIW, I like to run all my code through
scripts/Lindent(in the Linux kernel tree) before submitting. It takes care of all that fiddly stuff so I don't have to go searching for things.
Lindent skript is good but it is without checking long of comment and intend create lot of coding style violations
I use liuboot. (Lindent with -pcs - http://www.denx.de/wiki/UBoot/CodingStyle)
[microblaze at monstr common]$ cat /home/uboot/liuboot
#!/bin/sh
indent -npro -kr -i8 -ts8 -sob -l80 -ss -ncs -pcs "$@"
You can try it on common/cmd_jffs2.c [microblaze at monstr common]$ liuboot cmd_jffs2.c [microblaze at monstr common]$
And check.
Without checking long of comment- Is it a problem longer row than 80 character? lines 56, 64, 69, 557
Check the lines 477, 681, 721, 819, 1307, 1402, etc.- consist of some white space (Do not add more than 2 empty lines to source files - it is violation)
I think Lindent with -pcs (or liuboot) is great but brings up many problems.
Best regards
Michal Simek
Quoted 5 lines · Michal Simek
ROMFS: Add support for ROMFS filesystem based on MTD system.
Signed-off-by Michal Simek <[email protected]>Patch is in attachement.
Please send your patches inline. It is harder to comment on patches that are attachments because I cannot hit 'reply' and start typing. Patches that don't get reviewed are harder to get merged. (git-send-email is your friend here)
I needed to cut and paste to write this email, but here are some comments.
Also, it's looking more and more like there needs to be an abstraction between filesystems and block devices. (More of a general comment than a comment on your patch) Overriding cmd_jffs2 to do more and more filesystems just smells wrong.
Otherwise, it's a good looking driver.
Cheers,
g.
Quoted 9 lines
Best regards,
Michal Simekcommon/cmd_jffs2.c +0
-0
common/cmd_jffs2.c line 88
Quoted 5 lines
common/cmd_jffs2.c +1
-1@@ -85,7 +85,7 @@*/ /* -* JFFS2/CRAMFS support +* JFFS2/CRAMFS/ROMFS support
Hmmm, perhaps the filename cmd_jffs2.c needs to be changed.
Quoted 120 lines
common/cmd_jffs2.c +27
-8*/ #include <common.h> #include <command.h> @@ -175,6 +175,11 @@ extern int cramfs_load (char *loadoffset, struct part_info *info, char *filenameextern int cramfs_ls (struct part_info *info,char *filename);extern int cramfs_info (struct part_info *info); +extern int romfs_check (struct part_info *info); +extern int romfs_load (char *loadoffset,struct part_info *info,char *filename); +extern int romfs_ls (struct part_info *info,char *filename); +extern int romfs_info (struct part_info *info); +static struct part_info *jffs2_part_info (struct mtd_device *dev,unsigned int part_num);/* command line only routines */ @@ -1874,14 +1879,22 @@ int do_jffs2_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])if ((part =jffs2_part_info (current_dev, current_partnum))){ -/* check partition type for cramfs */ - fsname = (cramfs_check (part) ?"CRAMFS" :"JFFS2" ); +/* check partition type for JFFS2, cramfs, romfs */ +if (cramfs_check (part)) { + fsname ="CRAMFS" ; + }else if (romfs_check (part)) { + fsname ="ROMFS" ; + }else { + fsname ="JFFS2" ; + }printf ("### %s loading '%s' to 0x%lx\n" , fsname, filename, offset);if (cramfs_check (part)) { size =cramfs_load ((char *) offset, part, filename); + }else if (romfs_check (part)){ + size =romfs_load ((char *) offset, part, filename); }else { -/* if this is not cramfs assume jffs2 */ +/* if this is not cramfs or romfs assume jffs2 */ size =jffs2_1pass_load ((char *)offset, part, filename); }@@ -1928,8 +1941,10 @@ int do_jffs2_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])/* check partition type for cramfs */ if (cramfs_check (part)) { ret =cramfs_ls (part, filename); + }else if (romfs_check (part)) { + ret =romfs_ls (part, filename); }else { -/* if this is not cramfs assume jffs2 */ +/* if this is not cramfs or romfs assume jffs2 */ ret =jffs2_1pass_ls (part, filename); }@@ -1951,7 +1966,6 @@ int do_jffs2_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])int do_jffs2_fsinfo (cmd_tbl_t *cmdtp,int flag,int argc,char *argv[]) {struct part_info *part; -char *fsname;int ret;/* make sure we are in sync with env variables */ @@ -1961,13 +1975,18 @@ int do_jffs2_fsinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])if ((part =jffs2_part_info (current_dev, current_partnum))){/* check partition type for cramfs */ - fsname = (cramfs_check (part) ?"CRAMFS" :"JFFS2" ); -printf ("### filesystem type is %s\n" , fsname); +puts ("### filesystem type is " );if (cramfs_check (part)) { +puts ("CRAMFS\n" ); ret =cramfs_info (part); + + }else if (romfs_check (part)) { +puts ("ROMFS\n" ); + ret =romfs_info (part); }else { -/* if this is not cramfs assume jffs2 */ +/* if this is not cramfs or romfs assume jffs2 */ +puts ("JFFS2\n" ); ret =jffs2_1pass_info (part); }fs/Makefile +1
-1@@ -22,7 +22,7 @@ # # -SUBDIRS := jffs2 cramfs fdos fat reiserfs ext2 +SUBDIRS := romfs jffs2 cramfs fdos fat reiserfs ext2 $(obj).depend all: @for dir in $(SUBDIRS) ; do \fs/romfs/romfs.c +19
-0@@ -0,0 +1,270 @@ +/* +* (C) Copyright 2007 Michal Simek +* +* Michal SIMEK <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License
fs/romfs/romfs.c line 29
Quoted 10 lines
fs/romfs/romfs.c +10
-0+* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#include <common.h> +#include <malloc.h> +#include <command.h> + +#if (CONFIG_COMMANDS &CFG_CMD_JFFS2 )
I believe Jon's patches have caused CONFIG_COMMANDS to go away. Also, shouldn't it be gated by a new config value "CONFIG_CMD_ROMFS"?
Quoted 34 lines
fs/romfs/romfs.c +34
-0+static int romfs_list_inode (struct part_info *info,unsigned long offset) +{ +struct romfs_inode *inode = + (struct romfs_inode *)(PART_OFFSET (info) + offset); +struct romfs_inode *hardlink =NULL ; +char str[3 ], *data; + +/* mapping spec.info means +0 hard link link destination [file header] +1 directory first file's header +2 regular file unused, must be zero [MBZ] +3 symbolic link unused, MBZ (file data is the link content) +4 block device 16/16 bits major/minor number +5 char device - " - +6 socket unused, MBZ +7 fifo unused, MBZ */ +switch (inode->next &0x7 ) { +case 0 : + str[0 ] ='h' ; +break ; +case 1 : + str[0 ] ='d' ; +break ; +case 2 : + str[0 ] ='f' ; +break ; +case 3 : + str[0 ] ='l' ; +break ; +case 4 : + str[0 ] ='b' ; +break ; +case 5 : + str[0 ] ='c' ;
fs/romfs/romfs.c line 73
Quoted 10 lines
fs/romfs/romfs.c +10
-0+break ; +case 6 : + str[0 ] ='s' ; +break ; +case 7 : + str[0 ] ='p' ; +break ; +default : + str[0 ] ='?' ; + }
This can be done in a less verbose way by using an offset into a table or string.
fs/romfs/romfs.c line 79
Quoted 6 lines
fs/romfs/romfs.c +6
-0+ +if (inode->next &0x8 ) { + str[1 ] ='x' ; + }else { + str[1 ] ='-' ; + }
Maybe?
str[1] = inode->next & 0x8 ? 'x' : '-';
Grant,
Quoted 5 lines · Michal Simek
Quoted 4 lines
ROMFS: Add support for ROMFS filesystem based on MTD system.
Signed-off-by Michal Simek <[email protected]>
Quoted 8 lines
Quoted 1 line
Patch is in attachement.
Please send your patches inline. It is harder to comment on patches that are attachments because I cannot hit 'reply' and start typing. Patches that don't get reviewed are harder to get merged. (git-send-email is your friend here)
I needed to cut and paste to write this email, but here are some comments.
OK. Next driver.
Quoted 6 lines
Also, it's looking more and more like there needs to be an abstraction between filesystems and block devices. (More of a general comment than a comment on your patch) Overriding
cmd_jffs2to do more and more filesystems just smells wrong.Otherwise, it's a good looking driver.
Yes. I agree with you. System around block devices and filesystems needs rebuild.
Quoted 5 lines
Quoted 3 lines
/*
- * JFFS2/CRAMFS support
+ * JFFS2/CRAMFS/ROMFS supportHmmm, perhaps the filename
cmd_jffs2.cneeds to be changed.
It's the same design goal as CRAMFS.
Quoted 27 lines
Quoted 18 lines
+ str[0] = 'h';
+ break;
+ case 1:
+ str[0] = 'd';
+ break;
+ case 2:
+ str[0] = 'f';
+ break;
+ case 3:
+ str[0] = 'l';
+ break;
+ case 4:
+ str[0] = 'b';
+ break;
+ case 5:
+ str[0] = 'c';
+ break;
+ case 6:Quoted 8 lines
+ str[0] = 's';
+ break;
+ case 7:
+ str[0] = 'p';
+ break;
+ default:
+ str[0] = '?';
+ }
Quoted 2 lines
This can be done in a less verbose way by using an offset into a table or string.
Yes, it can be.
Quoted 9 lines
Quoted 6 lines
+
+ if (inode->next&0x8) {
+ str[1] = 'x';
+ } else {
+ str[1] = '-';
+ }Maybe?
str[1] =inode->next&0x8? 'x' : '-';
It can be accepted.
Michal Simek