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

[Michal Simek]

>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.

Yes it is not good approach. The whole filesystem handling needs to redesign.
I think that we can start with discuss about this.

>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.

OK. I think that for now I create macro for CONFIG_ROMFS and I add it to cmd_jffs.c file

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)

OK


> +
>  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.

I will add CONFIG_ROMFS macro and it is enough for now.

M