[U-Boot-Users] [PATCH7] microblaze - ROMFS adapt files

[Grant Likely]

On 8/30/07, Michal Simek <Monstr at seznam.cz> wrote:
> DESC:  ROMFS support files and sorted names in Makefiles
>
> Signed-off-by: Michal Simek <monstr at monstr.eu>
>
> Note; I think that we can start process with redesigning all FS support

I'm still not really fond of this approach.  Repurposing CONFIG_JFFS2
to add more and more filesystems is not a good approach.  I don't
recommend merging this as is.

Ideally, the whole filesystem handling should be reworked into a
single API, but I understand that it's a non-trivial amount of work.
However, at the very least, ROMFS should be configured with a separate
CONFIG_xxx macro.

More comments below.

> diff --git a/common/cmd_jffs2.c b/common/cmd_jffs2.c
> index 513a226..3e6061a 100644
> --- a/common/cmd_jffs2.c
> +++ b/common/cmd_jffs2.c
> @@ -85,7 +85,7 @@
>   */
>
>  /*
> - * JFFS2/CRAMFS support
> + * JFFS2/CRAMFS/ROMFS support
>   */
>  #include <common.h>
>  #include <command.h>
> @@ -175,6 +175,11 @@ extern int cramfs_load (char *loadoffset, struct part_info *info, char *filename
>  extern 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);

Bad form and dangerous.  Use a header file.  (Yes I know this file
already uses a bad pattern, but don't follow the example)

> +
>  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";
> +               }

This will get unwieldy in a real hurry.  This should probably be
reworked into a table of filesystem handlers which binds all the
driver callbacks.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195