splash_source: add support for ubifs formatted nand
PATCH 1/1 splash_source: add support for ubifs formatted nand
Add support for loading splash image from NAND Flash formatted with a (UBI) filesystem.
I'm a little ambivalent about this patch, as it adds support for a specific filesystem rather than a device, due to the nature of NAND and UBI, but it does make some sense to me to add a NAND filesystem option here. It would be nice to get your input about this.
--- common/splash_source.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- include/splash.h | 6 ++++-- 2 files changed, 50 insertions(+), 6 deletions(-)
common/splash_source.c +46 -4
@@ -120,6 +120,9 @@ static int splash_select_fs_dev(struct splash_location *location)case SPLASH_STORAGE_SATA : res =fs_set_blk_dev ("sata" , location->devpart,FS_TYPE_ANY );break ; +case SPLASH_STORAGE_NAND : + res =fs_set_blk_dev ("ubi" ,NULL ,FS_TYPE_ANY ); +break ;default :printf ("Error: unsupported location storage.\n" );return -ENODEV ;
@@ -163,6 +166,35 @@ static inline int splash_init_sata(void) }#endif +#ifdef CONFIG_CMD_UBIFS +static int splash_mount_ubifs (struct splash_location *location) +{ +int res; +char cmd[32 ]; + +sprintf (cmd,"ubi part %s" , location->mtdpart); + res =run_command (cmd,0 ); +if (res) +return res; + +sprintf (cmd,"ubifsmount %s" , location->ubi_volume); + res =run_command (cmd,0 ); + +return res; +} +#else +static inline int splash_mount_ubifs (struct splash_location *location) +{ +printf ("Cannot load splash image: no UBIFS support\n" ); +return -ENOSYS ; +} +#endif + +static inline int splash_umount_ubifs (void ) +{ +return run_command ("ubifsumount" ,0 ); +} +#define SPLASH_SOURCE_DEFAULT_FILE_NAME "splash.bmp"static int splash_load_fs (struct splash_location *location,u32 bmp_load_addr)
@@ -181,26 +213,36 @@ static int splash_load_fs(struct splash_location *location, u32 bmp_load_addr)if (location->storage ==SPLASH_STORAGE_SATA ) res =splash_init_sata (); +if (location->storage ==SPLASH_STORAGE_NAND ) + res =splash_mount_ubifs (location); +if (res)return res; res =splash_select_fs_dev (location);if (res) -return res; + goto out; res =fs_size (splash_file, &bmp_size);if (res) {printf ("Error (%d): cannot determine file size\n" , res); -return res; + goto out; }if (bmp_load_addr + bmp_size >= gd->start_addr_sp) {printf ("Error: splashimage address too high. Data overwrites U-Boot and/or placed beyond DRAM boundaries.\n" ); -return -EFAULT ; + res = -EFAULT ; + goto out; }splash_select_fs_dev (location); -return fs_read (splash_file, bmp_load_addr,0 ,0 ,NULL ); + res =fs_read (splash_file, bmp_load_addr,0 ,0 ,NULL ); + +out: +if (location->storage ==SPLASH_STORAGE_NAND ) +splash_umount_ubifs (); + +return res; }/**
include/splash.h +4 -2
@@ -41,8 +41,10 @@ struct splash_location {char *name;enum splash_storage storage;enum splash_flags flags; -u32 offset;/* offset from start of storage */ -char *devpart;/* Use the load command dev:part conventions */ +u32 offset;/* Offset from start of storage */ +char *devpart;/* Use the load command dev:part conventions */ +char *mtdpart;/* MTD partition for ubi part */ +char *ubi_volume;/* UBI volume-name for ubifsmount */ };int splash_source_load (struct splash_location *locations,uint size);
Signature · 1 line
1.9.1
PATCH 1/1 splash_source: add support for ubifs formatted nand
Hi Eran,
This mostly looks good to me, but I think there's a more generic way to address the UBIFS conundrum. See my comments below:
Quoted 21 lines · Eran Matityahu
Add support for loading splash image from NAND Flash formatted with a (UBI) filesystem.
I'm a little ambivalent about this patch, as it adds support for a specific filesystem rather than a device, due to the nature of NAND and UBI, but it does make some sense to me to add a NAND filesystem option here. It would be nice to get your input about this.
Signed-off-by Eran Matityahu <[email protected]>Cc Igor Grinberg <[email protected]>Cc Tom Rini <[email protected]>Cc Nikita Kiryanov <[email protected]>--- common/splash_source.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- include/splash.h | 6 ++++-- 2 files changed, 50 insertions(+), 6 deletions(-)common/splash_source.c +0
-0
common/splash_source.c static int splash_select_fs_dev(struct splash_location *location) · line 125
Quoted 6 lines
common/splash_source.c +3
-0@@ -120,6 +120,9 @@ static int splash_select_fs_dev(struct splash_location *location)case SPLASH_STORAGE_SATA : res =fs_set_blk_dev ("sata" , location->devpart,FS_TYPE_ANY );break ; +case SPLASH_STORAGE_NAND : + res =fs_set_blk_dev ("ubi" ,NULL ,FS_TYPE_ANY ); +break ;a
It seems to me like this should be FS_TYPE_UBIFS?
Quoted 25 lines
common/splash_source.c +18
-0default :printf ("Error: unsupported location storage.\n" );return -ENODEV ;@@ -163,6 +166,35 @@ static inline int splash_init_sata(void) }#endif +#ifdef CONFIG_CMD_UBIFS +static int splash_mount_ubifs (struct splash_location *location) +{ +int res; +char cmd[32 ]; + +sprintf (cmd,"ubi part %s" , location->mtdpart); + res =run_command (cmd,0 ); +if (res) +return res; + +sprintf (cmd,"ubifsmount %s" , location->ubi_volume); + res =run_command (cmd,0 ); + +return res; +} +#else +static inline int splash_mount_ubifs (struct splash_location *location)
common/splash_source.c static inline int splash_init_sata(void) · line 196
Quoted 10 lines
common/splash_source.c +10
-0+{ +printf ("Cannot load splash image: no UBIFS support\n" ); +return -ENOSYS ; +} +#endif + +static inline int splash_umount_ubifs (void ) +{ +return run_command ("ubifsumount" ,0 ); +}
I know you prevent this from being called ifndef CONFIG_CMD_UBIFS, but I still think it should have two definitions (ifdef and ifndef) so that it will not depend on the program control flow.
common/splash_source.c static int splash_load_fs(struct splash_location *location, u32 bmp_load_addr) · line 217
Quoted 10 lines
common/splash_source.c +3
-0+#define SPLASH_SOURCE_DEFAULT_FILE_NAME "splash.bmp"static int splash_load_fs (struct splash_location *location,u32 bmp_load_addr)@@ -181,26 +213,36 @@ static int splash_load_fs(struct splash_location *location, u32 bmp_load_addr)if (location->storage ==SPLASH_STORAGE_SATA ) res =splash_init_sata (); +if (location->storage ==SPLASH_STORAGE_NAND ) + res =splash_mount_ubifs (location);
I think a more generic way of selecting ubifs would be to look at the ubi_volume field. If it's not NULL, then we are dealing with UBIFS. That way we will not have to update this check when NOR flash support is added, and we do not tie NAND fs case to UBIFS (even though there's no good alternative for UBIFS at the moment).
splash_select_fs_dev() would have to check ubi_volume in the SPLASH_STORAGE_NAND case as well.
Quoted 55 lines
common/splash_source.c +12
-4+if (res)return res; res =splash_select_fs_dev (location);if (res) -return res; + goto out; res =fs_size (splash_file, &bmp_size);if (res) {printf ("Error (%d): cannot determine file size\n" , res); -return res; + goto out; }if (bmp_load_addr + bmp_size >= gd->start_addr_sp) {printf ("Error: splashimage address too high. Data overwrites U-Boot and/or placed beyond DRAM boundaries.\n" ); -return -EFAULT ; + res = -EFAULT ; + goto out; }splash_select_fs_dev (location); -return fs_read (splash_file, bmp_load_addr,0 ,0 ,NULL ); + res =fs_read (splash_file, bmp_load_addr,0 ,0 ,NULL ); + +out: +if (location->storage ==SPLASH_STORAGE_NAND ) +splash_umount_ubifs (); + +return res; }/** include/splash.h +4
-2@@ -41,8 +41,10 @@ struct splash_location {char *name;enum splash_storage storage;enum splash_flags flags; -u32 offset;/* offset from start of storage */ -char *devpart;/* Use the load command dev:part conventions */ +u32 offset;/* Offset from start of storage */ +char *devpart;/* Use the load command dev:part conventions */ +char *mtdpart;/* MTD partition for ubi part */ +char *ubi_volume;/* UBI volume-name for ubifsmount */ };int splash_source_load (struct splash_location *locations,uint size);Signature · 1 line
1.9.1
Regards,
Nikita Kiryanov