distro_bootcmd: Add support for booting from ubifs
Subsystems (2)
v2 cover distro_bootcmd: Add support for booting from ubifs
Hi Stephen & Scott,
As requested by Stephen here is a new version of my patch-set to add mtd with ubi on top with ubifs on top support to distro_bootcmd, this time using the generic filesystem related commands / code.
Scott, can you review the first three patches, and perhaps these should also be merged through your tree ?
Stephen, can you review the last patch ? This can go in more or less independetly (unless Scott nacks the entire generic fs approach), since it does not do anything unless enabled.
Regards,
Hans
PATCH v2 1/4 ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
Modify the ubifs u-boot wrapper function prototypes for generic fs use, and give them their own header file.
This is a preparation patch for adding ubifs support to the generic fs code from fs/fs.c.
--- common/cmd_ubifs.c | 14 +++-------- fs/ubifs/ubifs.c | 70 ++++++++++++++++++++++++++++++++++++++++----------- fs/ubifs/ubifs.h | 6 +---- include/ubifs_uboot.h | 29 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-) create mode 100644 include/ubifs_uboot.h
common/cmd_ubifs.c +3 -11
@@ -15,11 +15,10 @@#include <common.h> #include <config.h> #include <command.h> - -#include "../fs/ubifs/ubifs.h" +#include <ubifs_uboot.h> static int ubifs_initialized; -static int ubifs_mounted; +int ubifs_mounted;static int do_ubifs_mount (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[])
@@ -54,14 +53,7 @@ int ubifs_is_mounted(void)void cmd_ubifs_umount (void ) { - -if (ubifs_sb) { -printf ("Unmounting UBIFS volume %s!\n" , - ((struct ubifs_info *)(ubifs_sb->s_fs_info))->vi.name); -ubifs_umount (ubifs_sb->s_fs_info); - } - - ubifs_sb =NULL ; +uboot_ubifs_umount (); ubifs_mounted =0 ; ubifs_initialized =0 ; }
fs/ubifs/ubifs.c +56 -14
122 lines not on this page · raw message
fs/ubifs/ubifs.h +1 -5
@@ -34,6 +34,7 @@#include <asm/atomic.h> #include <asm-generic/atomic-long.h> #include <ubi_uboot.h> +#include <ubifs_uboot.h> #include <linux/ctype.h> #include <linux/time.h>
@@ -2379,11 +2380,6 @@ int ubifs_decompress(const void *buf, int len, void *out, int *out_len,#include "key.h" #ifdef __UBOOT__ -/* these are used in cmd_ubifs.c */ -int ubifs_init (void ); -int uboot_ubifs_mount (char *vol_name);void ubifs_umount (struct ubifs_info *c); -int ubifs_ls (char *dir_name); -int ubifs_load (char *filename,u32 addr,u32 size);#endif #endif /* !__UBIFS_H__ */
include/ubifs_uboot.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* UBIFS u-boot wrapper functions header +* +* Copyright (C) 2006-2008 Nokia Corporation +* +* (C) Copyright 2008-2009 +* Stefan Roese, DENX Software Engineering, sr at denx.de. +* +* SPDX-License-Identifier: GPL-2.0+ +* +* Authors: Artem Bityutskiy (???????? ?????) +* Adrian Hunter +*/ + +#ifndef __UBIFS_UBOOT_H__ +#define __UBIFS_UBOOT_H__ + +extern int ubifs_mounted; + +int ubifs_init (void ); +int uboot_ubifs_mount (char *vol_name); +void uboot_ubifs_umount (void ); +int ubifs_load (char *filename,u32 addr,u32 size); + +int ubifs_ls (const char *dir_name); +int ubifs_read (const char *filename,void *buf,loff_t offset, +loff_t size,loff_t *actread); + +#endif /* __UBIFS_UBOOT_H__ */
Signature · 1 line
2.4.3
PATCH v2 1/4 ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 6 lines
Modify the ubifs u-boot wrapper function prototypes for generic fs use, and give them their own header file.
This is a preparation patch for adding ubifs support to the generic fs code from
fs/fs.c.
Quoted 8 lines
Signed-off-by Hans de Goede <[email protected]>--- common/cmd_ubifs.c | 14 +++-------- fs/ubifs/ubifs.c | 70 ++++++++++++++++++++++++++++++++++++++++----------- fs/ubifs/ubifs.h | 6 +---- include/ubifs_uboot.h | 29 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-)create mode 100644
include/ubifs_uboot.h
Only one nitpick, beside of this, you can add my:
Quoted 5 lines
common/cmd_ubifs.c +0
-0
common/cmd_ubifs.c line 21
Quoted 10 lines
common/cmd_ubifs.c +2
-3@@ -15,11 +15,10 @@#include <common.h> #include <config.h> #include <command.h> - -#include "../fs/ubifs/ubifs.h" +#include <ubifs_uboot.h> static int ubifs_initialized; -static int ubifs_mounted; +int ubifs_mounted;
later you add "extern int ubifs_mounted" in include/ubifs_uboot.h
Maybe you want to add a function, which returns the state of this var and let the var static?
bye,
Heiko
Quoted 210 lines
common/cmd_ubifs.c +1
-8static int do_ubifs_mount (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[])@@ -54,14 +53,7 @@ int ubifs_is_mounted(void)void cmd_ubifs_umount (void ) { - -if (ubifs_sb) { -printf ("Unmounting UBIFS volume %s!\n" , - ((struct ubifs_info *)(ubifs_sb->s_fs_info))->vi.name); -ubifs_umount (ubifs_sb->s_fs_info); - } - - ubifs_sb =NULL ; +uboot_ubifs_umount (); ubifs_mounted =0 ; ubifs_initialized =0 ; }fs/ubifs/ubifs.c +56
-14@@ -568,7 +568,7 @@ static unsigned long ubifs_findfile(struct super_block *sb, char *filename)return 0 ; } -int ubifs_ls (char *filename) +int ubifs_ls (const char *filename) {struct ubifs_info *c = ubifs_sb->s_fs_info;struct file *file;@@ -579,7 +579,7 @@ int ubifs_ls(char *filename)int ret =0 ; c->ubi =ubi_open_volume (c->vi.ubi_num, c->vi.vol_id,UBI_READONLY ); - inum =ubifs_findfile (ubifs_sb, filename); + inum =ubifs_findfile (ubifs_sb, (char *)filename);if (!inum) { ret =-1 ; goto out;@@ -785,7 +785,8 @@ error:return err; } -int ubifs_load (char *filename,u32 addr,u32 size) +int ubifs_read (const char *filename,void *buf,loff_t offset, +loff_t size,loff_t *actread) {struct ubifs_info *c = ubifs_sb->s_fs_info;unsigned long inum;@@ -796,10 +797,18 @@ int ubifs_load(char *filename, u32 addr, u32 size)int count;int last_block_size =0 ; + *actread =0 ; + +if (offset & (PAGE_SIZE -1 )) { +printf ("ubifs: Error offset must be a multple of %d\n" , +PAGE_SIZE ); +return -1 ; + } + c->ubi =ubi_open_volume (c->vi.ubi_num, c->vi.vol_id,UBI_READONLY );/* ubifs_findfile will resolve symlinks, so we know that we get * the real file here */ - inum =ubifs_findfile (ubifs_sb, filename); + inum =ubifs_findfile (ubifs_sb, (char *)filename);if (!inum) { err =-1 ; goto out;@@ -815,19 +824,24 @@ int ubifs_load(char *filename, u32 addr, u32 size) goto out; } +if (offset > inode->i_size) { +printf ("ubifs: Error offset (%lld) > file-size (%lld)\n" , + offset, size); + err =-1 ; + goto put_inode; + } +/* * If no size was specified or if size bigger than filesize * set size to filesize */ -if ((size ==0 ) || (size > inode->i_size)) - size = inode->i_size; +if ((size ==0 ) || (size > (inode->i_size - offset))) + size = inode->i_size - offset; count = (size +UBIFS_BLOCK_SIZE -1 ) >>UBIFS_BLOCK_SHIFT ; -printf ("Loading file '%s' to addr 0x%08x with size %d (0x%08x)...\n" , - filename, addr, size, size); - page.addr = (void *)addr; - page.index =0 ; + page.addr = buf; + page.index = offset /PAGE_SIZE ; page.inode = inode;for (i =0 ; i < count; i++) {/* @@ -844,16 +858,44 @@ int ubifs_load(char *filename, u32 addr, u32 size) page.index++; } -if (err) +if (err) {printf ("Error reading file '%s'\n" , filename); -else { -setenv_hex ("filesize" , size); -printf ("Done\n" ); + *actread = i *PAGE_SIZE ; + }else { + *actread = size; } +put_inode:ubifs_iput (inode); out:ubi_close_volume (c->ubi);return err; } + +/* Compat wrappers for common/cmd_ubifs.c */ +int ubifs_load (char *filename,u32 addr,u32 size) +{ +loff_t actread; +int err; + +printf ("Loading file '%s' to addr 0x%08x...\n" , filename, addr); + + err =ubifs_read (filename, (void *)addr,0 , size, &actread); +if (err ==0 ) { +setenv_hex ("filesize" , actread); +printf ("Done\n" ); + } + +return err; +} + +void uboot_ubifs_umount (void ) +{ +if (ubifs_sb) { +printf ("Unmounting UBIFS volume %s!\n" , + ((struct ubifs_info *)(ubifs_sb->s_fs_info))->vi.name); +ubifs_umount (ubifs_sb->s_fs_info); + ubifs_sb =NULL ; + } +}fs/ubifs/ubifs.h +1
-5@@ -34,6 +34,7 @@#include <asm/atomic.h> #include <asm-generic/atomic-long.h> #include <ubi_uboot.h> +#include <ubifs_uboot.h> #include <linux/ctype.h> #include <linux/time.h> @@ -2379,11 +2380,6 @@ int ubifs_decompress(const void *buf, int len, void *out, int *out_len,#include "key.h" #ifdef __UBOOT__ -/* these are used in cmd_ubifs.c */ -int ubifs_init (void ); -int uboot_ubifs_mount (char *vol_name);void ubifs_umount (struct ubifs_info *c); -int ubifs_ls (char *dir_name); -int ubifs_load (char *filename,u32 addr,u32 size);#endif #endif /* !__UBIFS_H__ */ include/ubifs_uboot.h new file +29
-0@@ -0,0 +1,29 @@ +/* +* UBIFS u-boot wrapper functions header +* +* Copyright (C) 2006-2008 Nokia Corporation +* +* (C) Copyright 2008-2009 +* Stefan Roese, DENX Software Engineering, sr at denx.de. +* +* SPDX-License-Identifier: GPL-2.0+ +* +* Authors: Artem Bityutskiy (???????? ?????) +* Adrian Hunter +*/ + +#ifndef __UBIFS_UBOOT_H__ +#define __UBIFS_UBOOT_H__ + +extern int ubifs_mounted; + +int ubifs_init (void ); +int uboot_ubifs_mount (char *vol_name); +void uboot_ubifs_umount (void ); +int ubifs_load (char *filename,u32 addr,u32 size); + +int ubifs_ls (const char *dir_name); +int ubifs_read (const char *filename,void *buf,loff_t offset, +loff_t size,loff_t *actread); + +#endif /* __UBIFS_UBOOT_H__ */
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v2 1/4 ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
Hi,
Quoted 34 lines · Heiko Schocher
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 6 lines
Modify the ubifs u-boot wrapper function prototypes for generic fs use, and give them their own header file.
This is a preparation patch for adding ubifs support to the generic fs code from
fs/fs.c.Quoted 8 lines
Signed-off-by Hans de Goede <[email protected]>--- common/cmd_ubifs.c | 14 +++-------- fs/ubifs/ubifs.c | 70 ++++++++++++++++++++++++++++++++++++++++----------- fs/ubifs/ubifs.h | 6 +---- include/ubifs_uboot.h | 29 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-)create mode 100644
include/ubifs_uboot.hOnly one nitpick, beside of this, you can add my:
Reviewed-by Heiko Schocher <[email protected]>Quoted 12 lines
common/cmd_ubifs.c +1
-2@@ -15,11 +15,10 @@#include <common.h> #include <config.h> #include <command.h> - -#include "../fs/ubifs/ubifs.h" +#include <ubifs_uboot.h>
Quoted 8 lines
common/cmd_ubifs.c line 21
Quoted 3 lines
common/cmd_ubifs.c +1
-1static int ubifs_initialized; -static int ubifs_mounted; +int ubifs_mounted;later you add "extern int
ubifs_mounted" ininclude/ubifs_uboot.hMaybe you want to add a function, which returns the state of this var and let the var static?
Yes that would be cleaner, I'll fix the patch-set to do things that way.
Thanks for the reviews.
So when the time come comes (when v2015.10 is out), how do we merge these 3, shall I take them upstream through the linux-sunxi tree, or do you want me to send a v2 with this fixed, and then you take them upstream ?
Regards,
Hans
PATCH v2 1/4 ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
Quoted 49 lines · Hans de Goede
Hi,
Quoted 34 lines · Heiko Schocher
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 6 lines
Modify the ubifs u-boot wrapper function prototypes for generic fs use, and give them their own header file.
This is a preparation patch for adding ubifs support to the generic fs code from
fs/fs.c.Quoted 8 lines
Signed-off-by Hans de Goede <[email protected]>--- common/cmd_ubifs.c | 14 +++-------- fs/ubifs/ubifs.c | 70 ++++++++++++++++++++++++++++++++++++++++----------- fs/ubifs/ubifs.h | 6 +---- include/ubifs_uboot.h | 29 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-) create mode 100644 include/ubifs_uboot.hOnly one nitpick, beside of this, you can add my:
Reviewed-by Heiko Schocher <[email protected]>Quoted 12 lines
common/cmd_ubifs.c +1
-2@@ -15,11 +15,10 @@#include <common.h> #include <config.h> #include <command.h> - -#include "../fs/ubifs/ubifs.h" +#include <ubifs_uboot.h> Quoted 8 lines
common/cmd_ubifs.c line 21
Quoted 3 lines
common/cmd_ubifs.c +1
-1static int ubifs_initialized; -static int ubifs_mounted; +int ubifs_mounted;later you add "extern int
ubifs_mounted" ininclude/ubifs_uboot.hMaybe you want to add a function, which returns the state of this var and let the var static?
Yes that would be cleaner, I'll fix the patch-set to do things that way.
Quoted 6 lines
Thanks for the reviews.
So when the time come comes (when v2015.10 is out), how do we merge these 3, shall I take them upstream through the linux-sunxi tree, or do you want me to send a v2 with this fixed, and then you take them upstream ?
I can just take 'em all into master :)
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150828/dabc5046/attachment.sig>
PATCH v2 1/4 ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
Hello tom,
Am 28.08.2015 um 16:52 schrieb Tom Rini:
Quoted 50 lines · Hans de Goede
Quoted 49 lines
Hi,
Quoted 34 lines · Heiko Schocher
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 6 lines
Modify the ubifs u-boot wrapper function prototypes for generic fs use, and give them their own header file.
This is a preparation patch for adding ubifs support to the generic fs code from
fs/fs.c.Quoted 8 lines
Signed-off-by Hans de Goede <[email protected]>--- common/cmd_ubifs.c | 14 +++-------- fs/ubifs/ubifs.c | 70 ++++++++++++++++++++++++++++++++++++++++----------- fs/ubifs/ubifs.h | 6 +---- include/ubifs_uboot.h | 29 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-)create mode 100644
include/ubifs_uboot.hOnly one nitpick, beside of this, you can add my:
Reviewed-by Heiko Schocher <[email protected]>Quoted 12 lines
common/cmd_ubifs.c +1
-2@@ -15,11 +15,10 @@#include <common.h> #include <config.h> #include <command.h> - -#include "../fs/ubifs/ubifs.h" +#include <ubifs_uboot.h> Quoted 8 lines
common/cmd_ubifs.c line 21
Quoted 3 lines
common/cmd_ubifs.c +1
-1static int ubifs_initialized; -static int ubifs_mounted; +int ubifs_mounted;later you add "extern int
ubifs_mounted" ininclude/ubifs_uboot.hMaybe you want to add a function, which returns the state of this var and let the var static?
Yes that would be cleaner, I'll fix the patch-set to do things that way.
Quoted 8 lines
Quoted 6 lines
Thanks for the reviews.
So when the time come comes (when v2015.10 is out), how do we merge these 3, shall I take them upstream through the linux-sunxi tree, or do you want me to send a v2 with this fixed, and then you take them upstream ?
I can just take 'em all into master :)
Ok, from my side ...
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v2 1/4 ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
In reply to Tom RiniHi,
Quoted 50 lines · Tom Rini
Quoted 49 lines · Hans de Goede
Hi,
Quoted 34 lines · Heiko Schocher
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 6 lines
Modify the ubifs u-boot wrapper function prototypes for generic fs use, and give them their own header file.
This is a preparation patch for adding ubifs support to the generic fs code from
fs/fs.c.Quoted 8 lines
Signed-off-by Hans de Goede <[email protected]>--- common/cmd_ubifs.c | 14 +++-------- fs/ubifs/ubifs.c | 70 ++++++++++++++++++++++++++++++++++++++++----------- fs/ubifs/ubifs.h | 6 +---- include/ubifs_uboot.h | 29 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-)create mode 100644
include/ubifs_uboot.hOnly one nitpick, beside of this, you can add my:
Reviewed-by Heiko Schocher <[email protected]>Quoted 12 lines
common/cmd_ubifs.c +1
-2@@ -15,11 +15,10 @@#include <common.h> #include <config.h> #include <command.h> - -#include "../fs/ubifs/ubifs.h" +#include <ubifs_uboot.h> Quoted 8 lines
common/cmd_ubifs.c line 21
Quoted 3 lines
common/cmd_ubifs.c +1
-1static int ubifs_initialized; -static int ubifs_mounted; +int ubifs_mounted;later you add "extern int
ubifs_mounted" ininclude/ubifs_uboot.hMaybe you want to add a function, which returns the state of this var and let the var static?
Yes that would be cleaner, I'll fix the patch-set to do things that way.
Quoted 8 lines
Quoted 6 lines
Thanks for the reviews.
So when the time come comes (when v2015.10 is out), how do we merge these 3, shall I take them upstream through the linux-sunxi tree, or do you want me to send a v2 with this fixed, and then you take them upstream ?
I can just take 'em all into master :)
Heiko and I both thought we were too far in the cycle for that, but I must admit I do not see that much chance of these changes causing regressions and they are a nice improvement.
So I'll send a v2 for you to merge, unless Heiko objects.
Regards,
Hans
PATCH v2 1/4 ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
Hello Hans,
Am 31.08.2015 um 18:08 schrieb Hans de Goede:
Quoted 60 lines
Hi,
Quoted 57 lines · Tom Rini
Quoted 55 lines · Hans de Goede
Hi,
Quoted 34 lines · Heiko Schocher
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 6 lines
Modify the ubifs u-boot wrapper function prototypes for generic fs use, and give them their own header file.
This is a preparation patch for adding ubifs support to the generic fs code from
fs/fs.c.Quoted 8 lines
Signed-off-by Hans de Goede <[email protected]>--- common/cmd_ubifs.c | 14 +++-------- fs/ubifs/ubifs.c | 70 ++++++++++++++++++++++++++++++++++++++++----------- fs/ubifs/ubifs.h | 6 +---- include/ubifs_uboot.h | 29 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-)create mode 100644
include/ubifs_uboot.hOnly one nitpick, beside of this, you can add my:
Reviewed-by Heiko Schocher <[email protected]>Quoted 12 lines
common/cmd_ubifs.c +1
-2@@ -15,11 +15,10 @@#include <common.h> #include <config.h> #include <command.h> - -#include "../fs/ubifs/ubifs.h" +#include <ubifs_uboot.h> Quoted 8 lines
common/cmd_ubifs.c line 21
Quoted 3 lines
common/cmd_ubifs.c +1
-1static int ubifs_initialized; -static int ubifs_mounted; +int ubifs_mounted;later you add "extern int
ubifs_mounted" ininclude/ubifs_uboot.hMaybe you want to add a function, which returns the state of this var and let the var static?
Yes that would be cleaner, I'll fix the patch-set to do things that way.
Thanks for the reviews.
So when the time come comes (when v2015.10 is out), how do we merge these 3, shall I take them upstream through the linux-sunxi tree, or do you want me to send a v2 with this fixed, and then you take them upstream ?
Quoted 7 lines
Quoted 1 line
I can just take 'em all into master :)
Heiko and I both thought we were too far in the cycle for that, but I must admit I do not see that much chance of these changes causing regressions and they are a nice improvement.
So I'll send a v2 for you to merge, unless Heiko objects.
Let me do with your v2 some tests today, if they are ok its Toms decision.
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v2 1/4 ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
Hello Hans,
Am 01.09.2015 um 05:46 schrieb Heiko Schocher:
Quoted 65 lines
Hello Hans,
Am 31.08.2015 um 18:08 schrieb Hans de Goede:
Quoted 62 lines
Hi,
Quoted 58 lines · Tom Rini
Quoted 49 lines · Hans de Goede
Hi,
Quoted 34 lines · Heiko Schocher
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 6 lines
Modify the ubifs u-boot wrapper function prototypes for generic fs use, and give them their own header file.
This is a preparation patch for adding ubifs support to the generic fs code from
fs/fs.c.Quoted 8 lines
Signed-off-by Hans de Goede <[email protected]>--- common/cmd_ubifs.c | 14 +++-------- fs/ubifs/ubifs.c | 70 ++++++++++++++++++++++++++++++++++++++++----------- fs/ubifs/ubifs.h | 6 +---- include/ubifs_uboot.h | 29 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-)create mode 100644
include/ubifs_uboot.hOnly one nitpick, beside of this, you can add my:
Reviewed-by Heiko Schocher <[email protected]>Quoted 12 lines
common/cmd_ubifs.c +1
-2@@ -15,11 +15,10 @@#include <common.h> #include <config.h> #include <command.h> - -#include "../fs/ubifs/ubifs.h" +#include <ubifs_uboot.h> Quoted 8 lines
common/cmd_ubifs.c line 21
Quoted 3 lines
common/cmd_ubifs.c +1
-1static int ubifs_initialized; -static int ubifs_mounted; +int ubifs_mounted;later you add "extern int
ubifs_mounted" ininclude/ubifs_uboot.hMaybe you want to add a function, which returns the state of this var and let the var static?
Yes that would be cleaner, I'll fix the patch-set to do things that way.
Quoted 6 lines
Thanks for the reviews.
So when the time come comes (when v2015.10 is out), how do we merge these 3, shall I take them upstream through the linux-sunxi tree, or do you want me to send a v2 with this fixed, and then you take them upstream ?
I can just take 'em all into master :)
Quoted 8 lines
Quoted 5 lines
Heiko and I both thought we were too far in the cycle for that, but I must admit I do not see that much chance of these changes causing regressions and they are a nice improvement.
So I'll send a v2 for you to merge, unless Heiko objects.
Let me do with your v2 some tests today, if they are ok its Toms decision.
Tested the patches:
Patchwork [U-Boot,v2,1/3] ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use
http://patchwork.ozlabs.org/patch/512543/
Patchwork [U-Boot,v2,2/3] ubifs: Add functions for generic fs use
http://patchwork.ozlabs.org/patch/512545/
Patchwork [U-Boot,v2,3/3] ubifs: Add generic fs support
http://patchwork.ozlabs.org/patch/512544/
on the aristainetos2 board (with an ubifs on SPI NOR and NAND) without seeing errors ... Are the above patches your current set? You have in the subject "[PATCH v2 1/4]" ...
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v2 2/4 ubifs: Add functions for generic fs use
In reply to Hans de GoedeImplement the necessary functions for implementing generic fs support for ubifs.
--- fs/ubifs/ubifs.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/ubifs_uboot.h | 4 ++++ 2 files changed, 66 insertions(+)
fs/ubifs/ubifs.c +62 -0
@@ -568,6 +568,22 @@ static unsigned long ubifs_findfile(struct super_block *sb, char *filename)return 0 ; } +int ubifs_set_blk_dev (block_dev_desc_t *rbdd,disk_partition_t *info) +{ +/* Check that ubifs is mounted and that we are not being a blkdev */ +if (!ubifs_mounted) { +printf ("UBIFS not mounted, use ubifsmount to mount volume first!\n" ); +return -1 ; + } + +if (rbdd) { +printf ("UBIFS cannot be used with normal block devices\n" ); +return -1 ; + } + +return 0 ; +} +int ubifs_ls (const char *filename) {struct ubifs_info *c = ubifs_sb->s_fs_info;
@@ -616,6 +632,48 @@ out:return ret; } +int ubifs_exists (const char *filename) +{ +struct ubifs_info *c = ubifs_sb->s_fs_info; +unsigned long inum; + + c->ubi =ubi_open_volume (c->vi.ubi_num, c->vi.vol_id,UBI_READONLY ); + inum =ubifs_findfile (ubifs_sb, (char *)filename); +ubi_close_volume (c->ubi); + +return inum !=0 ; +} + +int ubifs_size (const char *filename,loff_t *size) +{ +struct ubifs_info *c = ubifs_sb->s_fs_info; +unsigned long inum; +struct inode *inode; +int err =0 ; + + c->ubi =ubi_open_volume (c->vi.ubi_num, c->vi.vol_id,UBI_READONLY ); + + inum =ubifs_findfile (ubifs_sb, (char *)filename); +if (!inum) { + err =-1 ; + goto out; + } + + inode =ubifs_iget (ubifs_sb, inum); +if (IS_ERR (inode)) { +printf ("%s: Error reading inode %ld!\n" , __func__, inum); + err =PTR_ERR (inode); + goto out; + } + + *size = inode->i_size; + +ubifs_iput (inode); +out: +ubi_close_volume (c->ubi); +return err; +} +/* * ubifsload... */
@@ -873,6 +931,10 @@ out:return err; } +void ubifs_close (void ) +{ +} +/* Compat wrappers for common/cmd_ubifs.c */ int ubifs_load (char *filename,u32 addr,u32 size) {
include/ubifs_uboot.h +4 -0
@@ -22,8 +22,12 @@ int uboot_ubifs_mount(char *vol_name);void uboot_ubifs_umount (void );int ubifs_load (char *filename,u32 addr,u32 size); +int ubifs_set_blk_dev (block_dev_desc_t *rbdd,disk_partition_t *info);int ubifs_ls (const char *dir_name); +int ubifs_exists (const char *filename); +int ubifs_size (const char *filename,loff_t *size);int ubifs_read (const char *filename,void *buf,loff_t offset,loff_t size,loff_t *actread); +void ubifs_close (void );#endif /* __UBIFS_UBOOT_H__ */
Signature · 1 line
2.4.3
PATCH v2 2/4 ubifs: Add functions for generic fs use
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 8 lines
Implement the necessary functions for implementing generic fs support for ubifs.
Signed-off-by Hans de Goede <[email protected]>--- fs/ubifs/ubifs.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/ubifs_uboot.h | 4 ++++ 2 files changed, 66 insertions(+)
bye,
Heiko
Quoted 106 lines
fs/ubifs/ubifs.c +62
-0@@ -568,6 +568,22 @@ static unsigned long ubifs_findfile(struct super_block *sb, char *filename)return 0 ; } +int ubifs_set_blk_dev (block_dev_desc_t *rbdd,disk_partition_t *info) +{ +/* Check that ubifs is mounted and that we are not being a blkdev */ +if (!ubifs_mounted) { +printf ("UBIFS not mounted, use ubifsmount to mount volume first!\n" ); +return -1 ; + } + +if (rbdd) { +printf ("UBIFS cannot be used with normal block devices\n" ); +return -1 ; + } + +return 0 ; +} +int ubifs_ls (const char *filename) {struct ubifs_info *c = ubifs_sb->s_fs_info;@@ -616,6 +632,48 @@ out:return ret; } +int ubifs_exists (const char *filename) +{ +struct ubifs_info *c = ubifs_sb->s_fs_info; +unsigned long inum; + + c->ubi =ubi_open_volume (c->vi.ubi_num, c->vi.vol_id,UBI_READONLY ); + inum =ubifs_findfile (ubifs_sb, (char *)filename); +ubi_close_volume (c->ubi); + +return inum !=0 ; +} + +int ubifs_size (const char *filename,loff_t *size) +{ +struct ubifs_info *c = ubifs_sb->s_fs_info; +unsigned long inum; +struct inode *inode; +int err =0 ; + + c->ubi =ubi_open_volume (c->vi.ubi_num, c->vi.vol_id,UBI_READONLY ); + + inum =ubifs_findfile (ubifs_sb, (char *)filename); +if (!inum) { + err =-1 ; + goto out; + } + + inode =ubifs_iget (ubifs_sb, inum); +if (IS_ERR (inode)) { +printf ("%s: Error reading inode %ld!\n" , __func__, inum); + err =PTR_ERR (inode); + goto out; + } + + *size = inode->i_size; + +ubifs_iput (inode); +out: +ubi_close_volume (c->ubi); +return err; +} +/* * ubifsload... */ @@ -873,6 +931,10 @@ out:return err; } +void ubifs_close (void ) +{ +} +/* Compat wrappers for common/cmd_ubifs.c */ int ubifs_load (char *filename,u32 addr,u32 size) {include/ubifs_uboot.h +4
-0@@ -22,8 +22,12 @@ int uboot_ubifs_mount(char *vol_name);void uboot_ubifs_umount (void );int ubifs_load (char *filename,u32 addr,u32 size); +int ubifs_set_blk_dev (block_dev_desc_t *rbdd,disk_partition_t *info);int ubifs_ls (const char *dir_name); +int ubifs_exists (const char *filename); +int ubifs_size (const char *filename,loff_t *size);int ubifs_read (const char *filename,void *buf,loff_t offset,loff_t size,loff_t *actread); +void ubifs_close (void );#endif /* __UBIFS_UBOOT_H__ */
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v2 2/4 ubifs: Add functions for generic fs use
In reply to Hans de GoedeQuoted 10 lines · Hans de Goede
Implement the necessary functions for implementing generic fs support for ubifs.
fs/ubifs/ubifs.c +5
-0+int ubifs_set_blk_dev (block_dev_desc_t *rbdd,disk_partition_t *info) +{ +/* Check that ubifs is mounted and that we are not being a blkdev */ +if (!ubifs_mounted) { +printf ("UBIFS not mounted, use ubifsmount to mount volume first!\n" );
fs/ubifs/ubifs.c static unsigned long ubifs_findfile(struct super_block *sb, char *filename) · line 585
Quoted 10 lines
fs/ubifs/ubifs.c +10
-0+return -1 ; + } + +if (rbdd) { +printf ("UBIFS cannot be used with normal block devices\n" ); +return -1 ; + } + +return 0 ; +}
I think those printf() should be debug(). Otherwise, if (a) someone attempts to run generic filesystem commands on a device with no filesystem or (b) we add new filesystems into fstypes[] after ubifs, those prints are going to happen even though a user didn't do something to explicitly cause a ubifs-related issue.
PATCH v2 2/4 ubifs: Add functions for generic fs use
Hi
Quoted 10 lines · Stephen Warren
fs/ubifs/ubifs.c static unsigned long ubifs_findfile(struct super_block *sb, char *filename)
Quoted 8 lines · Hans de Goede
Implement the necessary functions for implementing generic fs support for ubifs.
fs/ubifs/ubifs.c +3
-0+int ubifs_set_blk_dev (block_dev_desc_t *rbdd,disk_partition_t *info) +{ +/* Check that ubifs is mounted and that we are not being a blkdev
*/
Quoted 2 lines
fs/ubifs/ubifs.c
Quoted 2 lines
fs/ubifs/ubifs.c +2
-0+if (!ubifs_mounted) { + printf("UBIFS not mounted, use ubifsmount to mount volume
first!\n");
Quoted 5 lines
fs/ubifs/ubifs.c static unsigned long ubifs_findfile(struct super_block *sb, char *filename)
Quoted 5 lines
fs/ubifs/ubifs.c +5
-0+return -1 ; + } + +if (rbdd) { + printf("UBIFS cannot be used with normal block
devices\n");
Quoted 12 lines
fs/ubifs/ubifs.c static unsigned long ubifs_findfile(struct super_block *sb, char *filename) · line 585
Quoted 5 lines
fs/ubifs/ubifs.c +5
-0+return -1 ; + } + +return 0 ; +}I think those
printf()should bedebug(). Otherwise, if (a) someone attempts to run generic filesystem commands on a device with no filesystem or (b) we add new filesystems into fstypes[] after ubifs, those prints are going to happen even though a user didn't do something to explicitly cause a ubifs-related issue.
Personally I don't like return code like -1
Michael
_______________________________________________
Quoted 3 lines
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH v2 2/4 ubifs: Add functions for generic fs use
In reply to Stephen WarrenHi,
Quoted 19 lines · Stephen Warren
Quoted 18 lines · Hans de Goede
Implement the necessary functions for implementing generic fs support for ubifs.
fs/ubifs/ubifs.c +13
-0+int ubifs_set_blk_dev (block_dev_desc_t *rbdd,disk_partition_t *info) +{ +/* Check that ubifs is mounted and that we are not being a blkdev */ +if (!ubifs_mounted) { +printf ("UBIFS not mounted, use ubifsmount to mount volume first!\n" ); +return -1 ; + } + +if (rbdd) { +printf ("UBIFS cannot be used with normal block devices\n" ); +return -1 ; + } +
Quoted 8 lines
fs/ubifs/ubifs.c static unsigned long ubifs_findfile(struct super_block *sb, char *filename) · line 585
Quoted 2 lines
fs/ubifs/ubifs.c +2
-0+return 0 ; +}I think those
printf()should bedebug(). Otherwise, if (a) someone attempts to run generic filesystem commands on a device with no filesystem or (b) we add new filesystems into fstypes[] after ubifs, those prints are going to happen even though a user didn't do something to explicitly cause a ubifs-related issue.
Ack, will fix.
Regards,
Hans
PATCH v2 3/4 ubifs: Add generic fs support
In reply to Hans de GoedeAdd generic fs support, so that commands like ls, load and test -e can be used on ubifs.
--- disk/part.c | 23 +++++++++++++++++++++++ fs/fs.c | 16 ++++++++++++++++ include/fs.h | 1 + 3 files changed, 40 insertions(+)
disk/part.c +23 -0
@@ -10,6 +10,7 @@#include <ide.h> #include <malloc.h> #include <part.h> +#include <ubifs_uboot.h> #undef PART_DEBUG
@@ -530,6 +531,28 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str,return 0 ; } +#ifdef CONFIG_CMD_UBIFS +/* +* Special-case ubi, ubi goes through a mtd, rathen then through +* a regular block device. +*/ +if (0 ==strcmp (ifname,"ubi" )) { +if (!ubifs_mounted) { +printf ("UBIFS not mounted, use ubifsmount to mount volume first!\n" ); +return -1 ; + } + + *dev_desc =NULL ; +memset (info,0 ,sizeof (*info)); +strcpy ((char *)info->type,BOOT_PART_TYPE ); +strcpy ((char *)info->name,"UBI" ); +#ifdef CONFIG_PARTITION_UUIDS + info->uuid[0 ] =0 ; +#endif +return 0 ; + } +#endif +/* If no dev_part_str, use bootdevice environment variable */ if (!dev_part_str || !strlen (dev_part_str) || !strcmp (dev_part_str,"-" ))
fs/fs.c +16 -0
@@ -23,6 +23,7 @@#include <fat.h> #include <fs.h> #include <sandboxfs.h> +#include <ubifs_uboot.h> #include <asm/io.h> #include <div64.h> #include <linux/math64.h>
@@ -157,6 +158,21 @@ static struct fstype_info fstypes[] = { .uuid = fs_uuid_unsupported, },#endif +#ifdef CONFIG_CMD_UBIFS + { + .fstype = FS_TYPE_UBIFS, + .name = "ubifs", + .null_dev_desc_ok = true, + .probe = ubifs_set_blk_dev, + .close = ubifs_close, + .ls = ubifs_ls, + .exists = ubifs_exists, + .size = ubifs_size, + .read = ubifs_read, + .write = fs_write_unsupported, + .uuid = fs_uuid_unsupported, + }, +#endif { .fstype = FS_TYPE_ANY, .name = "unsupported",
include/fs.h +1 -0
@@ -22,6 +22,7 @@#define FS_TYPE_FAT 1#define FS_TYPE_EXT 2#define FS_TYPE_SANDBOX 3 +#define FS_TYPE_UBIFS 4/* * Tell the fs layer which block device an partition to use for future
Signature · 1 line
2.4.3
PATCH v2 3/4 ubifs: Add generic fs support
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Quoted 9 lines
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
Signed-off-by Hans de Goede <[email protected]>--- disk/part.c | 23 +++++++++++++++++++++++ fs/fs.c | 16 ++++++++++++++++ include/fs.h | 1 + 3 files changed, 40 insertions(+)
bye,
Heiko
Quoted 89 lines
disk/part.c +23
-0@@ -10,6 +10,7 @@#include <ide.h> #include <malloc.h> #include <part.h> +#include <ubifs_uboot.h> #undef PART_DEBUG@@ -530,6 +531,28 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str,return 0 ; } +#ifdef CONFIG_CMD_UBIFS +/* +* Special-case ubi, ubi goes through a mtd, rathen then through +* a regular block device. +*/ +if (0 ==strcmp (ifname,"ubi" )) { +if (!ubifs_mounted) { +printf ("UBIFS not mounted, use ubifsmount to mount volume first!\n" ); +return -1 ; + } + + *dev_desc =NULL ; +memset (info,0 ,sizeof (*info)); +strcpy ((char *)info->type,BOOT_PART_TYPE ); +strcpy ((char *)info->name,"UBI" ); +#ifdef CONFIG_PARTITION_UUIDS + info->uuid[0 ] =0 ; +#endif +return 0 ; + } +#endif +/* If no dev_part_str, use bootdevice environment variable */ if (!dev_part_str || !strlen (dev_part_str) || !strcmp (dev_part_str,"-" ))fs/fs.c +16
-0@@ -23,6 +23,7 @@#include <fat.h> #include <fs.h> #include <sandboxfs.h> +#include <ubifs_uboot.h> #include <asm/io.h> #include <div64.h> #include <linux/math64.h> @@ -157,6 +158,21 @@ static struct fstype_info fstypes[] = { .uuid = fs_uuid_unsupported, },#endif +#ifdef CONFIG_CMD_UBIFS + { + .fstype = FS_TYPE_UBIFS, + .name = "ubifs", + .null_dev_desc_ok = true, + .probe = ubifs_set_blk_dev, + .close = ubifs_close, + .ls = ubifs_ls, + .exists = ubifs_exists, + .size = ubifs_size, + .read = ubifs_read, + .write = fs_write_unsupported, + .uuid = fs_uuid_unsupported, + }, +#endif { .fstype = FS_TYPE_ANY, .name = "unsupported",include/fs.h +1
-0@@ -22,6 +22,7 @@#define FS_TYPE_FAT 1#define FS_TYPE_EXT 2#define FS_TYPE_SANDBOX 3 +#define FS_TYPE_UBIFS 4/* * Tell the fs layer which block device an partition to use for future
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v2 3/4 ubifs: Add generic fs support
In reply to Hans de GoedeQuoted 18 lines · Hans de Goede
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
disk/part.c +11
-0@@ -530,6 +531,28 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str,return 0 ; } +#ifdef CONFIG_CMD_UBIFS +/* +* Special-case ubi, ubi goes through a mtd, rathen then through +* a regular block device. +*/ +if (0 ==strcmp (ifname,"ubi" )) { +if (!ubifs_mounted) { +printf ("UBIFS not mounted, use ubifsmount to mount volume first!\n" ); +return -1 ; + } +
disk/part.c int get_device_and_partition(const char *ifname, const char *dev_part_str, · line 554
Quoted 10 lines
disk/part.c +10
-0+ *dev_desc =NULL ; +memset (info,0 ,sizeof (*info)); +strcpy ((char *)info->type,BOOT_PART_TYPE ); +strcpy ((char *)info->name,"UBI" ); +#ifdef CONFIG_PARTITION_UUIDS + info->uuid[0 ] =0 ; +#endif + return 0; + } +#endif
We now have two paths through this function that can "Return" a NULL dev_desc. This makes it impossible for sandbox and ubifs to successfully co-exist in the same U-Boot binary, since the sandbox and ubifs fs probe functions won't be able to tell if "hostfs" or "ubifs" was passed to get_device_and_partition(). Perhaps there's no ubifs support in sandbox right now, so there's no issue?
If this is an issue that needs to be solved now, I think the best solution would be for the two special cases in get_device_and_partition() to "return" a real dev_desc rather than NULL. Since there's nothing meaningful to put there, how about returning a hard-coded value that can then be checked in the fs probe functions to make sure it matches:
get_device_and_partition():
if (hostfs) {
...
*dev_desc = &hostfs_fake_dev_desc;
...
return 0;
}
if (ubi) {
...
*dev_desc = &ubifs_fake_dev_desc;
...
return 0;
}ubifs_set_blk_dev():
if (rbdd != &ubifs_fake_dev_desc) return -1; ... return 0;
... that said, I wonder if the ubifs special case in get_device_and_partition() shouldn't actually perform the ubifs_mount() call itself, based on the user-supplied parameters?
PATCH v2 3/4 ubifs: Add generic fs support
Hi,
Quoted 28 lines · Stephen Warren
Quoted 27 lines · Hans de Goede
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
disk/part.c +20
-0@@ -530,6 +531,28 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str,return 0 ; } +#ifdef CONFIG_CMD_UBIFS +/* +* Special-case ubi, ubi goes through a mtd, rathen then through +* a regular block device. +*/ +if (0 ==strcmp (ifname,"ubi" )) { +if (!ubifs_mounted) { +printf ("UBIFS not mounted, use ubifsmount to mount volume first!\n" ); +return -1 ; + } + + *dev_desc =NULL ; +memset (info,0 ,sizeof (*info)); +strcpy ((char *)info->type,BOOT_PART_TYPE ); +strcpy ((char *)info->name,"UBI" ); +#ifdef CONFIG_PARTITION_UUIDS + info->uuid[0 ] =0 ; +#endif +return 0 ; + }
Quoted 8 lines
Review int get_device_and_partition(const char *ifname, const char *dev_part_str, · line 554
Quoted 1 line
(quoted diff) +1
-0+#endifWe now have two paths through this function that can "Return" a NULL
dev_desc. This makes it impossible for sandbox and ubifs to successfully co-exist in the same U-Boot binary, since the sandbox and ubifs fs probe functions won't be able to tell if "hostfs" or "ubifs" was passed toget_device_and_partition(). Perhaps there's no ubifs support in sandbox right now, so there's no issue?
Right, ubifs is for raw nand, sandbox is for access to a host filesystem in a sandbox build. I basically never expect both CONFIG_CMD_UBIFS and CONFIG_SANDBOX to be set at the same time. I'll add a pre-processor check + #error to enforce this in the next version.
Quoted 24 lines
If this is an issue that needs to be solved now, I think the best solution would be for the two special cases in
get_device_and_partition()to "return" a realdev_descrather than NULL. Since there's nothing meaningful to put there, how about returning a hard-coded value that can then be checked in the fs probe functions to make sure it matches:
get_device_and_partition():if (hostfs) { ... *dev_desc = &hostfs_fake_dev_desc; ... return 0; } if (ubi) { ... *dev_desc = &ubifs_fake_dev_desc; ... return 0; }
ubifs_set_blk_dev():
Quoted 8 lines
if (rbdd != &ubifs_fake_dev_desc) return -1; ... return 0;... that said, I wonder if the ubifs special case in
get_device_and_partition()shouldn't actually perform theubifs_mount()call itself, based on the user-supplied parameters?
That is not possible as the supplied parameter for a generic fs call is a device index + partition number, where as ubi volumes use names (strings).
Regards,
Hans
PATCH v2 3/4 ubifs: Add generic fs support
Quoted 77 lines · Hans de Goede
Hi,
Quoted 31 lines · Stephen Warren
Quoted 30 lines · Hans de Goede
Add generic fs support, so that commands like ls, load and test -e can be
used on ubifs.disk/part.c +20
-0@@ -530,6 +531,28 @@ int get_device_and_partition(const char *ifname,const char *dev_part_str, return 0; } +#ifdef CONFIG_CMD_UBIFS +/* +* Special-case ubi, ubi goes through a mtd, rathen then through +* a regular block device. +*/ +if (0 ==strcmp (ifname,"ubi" )) { +if (!ubifs_mounted) { + printf("UBIFS not mounted, use ubifsmount to mount volume first!\n"); + return -1; + } + + *dev_desc =NULL ; +memset (info,0 ,sizeof (*info)); +strcpy ((char *)info->type,BOOT_PART_TYPE ); +strcpy ((char *)info->name,"UBI" ); +#ifdef CONFIG_PARTITION_UUIDS + info->uuid[0 ] =0 ; +#endif +return 0 ; + }Quoted 8 lines
Review int get_device_and_partition(const char *ifname, · line 556
Quoted 1 line
(quoted diff) +1
-0+#endifWe now have two paths through this function that can "Return" a NULL
dev_desc. This makes it impossible for sandbox and ubifs to successfully co-exist in the same U-Boot binary, since the sandbox and ubifs fs probe functions won't be able to tell if "hostfs" or "ubifs" was passed toget_device_and_partition(). Perhaps there's no ubifs support in sandbox right now, so there's no issue?Right, ubifs is for raw nand, sandbox is for access to a host filesystem in a sandbox build. I basically never expect both
CONFIG_CMD_UBIFSandCONFIG_SANDBOXto be set at the same time. I'll add a pre-processor check + #error to enforce this in the next version.Quoted 29 lines
If this is an issue that needs to be solved now, I think the best solution would be for the two special cases in
get_device_and_partition()to "return" a realdev_descrather than NULL. Since there's nothing meaningful to put there, how about returning a hard-coded value that can then be checked in the fs probe functions to make sure it matches:
get_device_and_partition():if (hostfs) { ... *dev_desc = &hostfs_fake_dev_desc; ... return 0; } if (ubi) { ... *dev_desc = &ubifs_fake_dev_desc; ... return 0; }
ubifs_set_blk_dev():if (rbdd != &ubifs_fake_dev_desc) return -1; ... return 0;
Quoted 7 lines
Quoted 3 lines
... that said, I wonder if the ubifs special case in
get_device_and_partition()shouldn't actually perform theubifs_mount()call itself, based on the user-supplied parameters?That is not possible as the supplied parameter for a generic fs call is a device index + partition number, where as ubi volumes use names (strings).
I don't think that matters.
The "generic fs" plumbing added in this series doesn't require a volume name to be passed from the "generic fs" layer to the ubifs layer; it assumes that a ubifs volume is already mounted and so no volume identity is required. I wasn't implying that should be changed. Given that, the disparity between parameters doesn't matter, since there's no need to translate the ubifs_fake_dev_dec to a ubifs volume name at all; the only thing it'd be used for is as an identity check to dispatch from the generic fs layer to the right underlying filesystem.
Still, the current solution you have should be fine for now. We only need to fix this if someone implements a raw NAND emulator for sandbox.
PATCH v2 4/4 distro_bootcmd: Add support for booting from ubifs
In reply to Hans de GoedeFrom: Roy Spliet <[email protected]>
Under the assumptions of having a UBI volume called boot, containing a ubifs filesystem.
--- include/config_distro_bootcmd.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
include/config_distro_bootcmd.h +19 -0
@@ -72,6 +72,24 @@ BOOT_TARGET_DEVICES_references_MMC_without_CONFIG_CMD_MMC#endif +#ifdef CONFIG_CMD_UBIFS +#define BOOTENV_SHARED_UBIFS \ + "ubifs_boot=" \ + "if ubi part UBI && ubifsmount ubi${devnum}:boot; then " \ + "setenv devtype ubi; " \ + "setenv bootpart 0; " \ + "run scan_dev_for_boot; " \ + "fi\0" +#define BOOTENV_DEV_UBIFS BOOTENV_DEV_BLKDEV +#define BOOTENV_DEV_NAME_UBIFS BOOTENV_DEV_NAME_BLKDEV +#else +#define BOOTENV_SHARED_UBIFS +#define BOOTENV_DEV_UBIFS \ + BOOT_TARGET_DEVICES_references_UBIFS_without_CONFIG_CMD_UBIFS +#define BOOTENV_DEV_NAME_UBIFS \ + BOOT_TARGET_DEVICES_references_UBIFS_without_CONFIG_CMD_UBIFS +#endif +#ifdef CONFIG_CMD_SATA #define BOOTENV_SHARED_SATA BOOTENV_SHARED_BLKDEV(sata)#define BOOTENV_DEV_SATA BOOTENV_DEV_BLKDEV
@@ -185,6 +203,7 @@BOOTENV_SHARED_SATA \ BOOTENV_SHARED_SCSI \BOOTENV_SHARED_IDE \ + BOOTENV_SHARED_UBIFS \"boot_prefixes=/ /boot/\0" \"boot_scripts=boot.scr.uimg boot.scr\0" \"boot_script_dhcp=boot.scr.uimg\0" \
Signature · 1 line
2.4.3
PATCH v2 4/4 distro_bootcmd: Add support for booting from ubifs
Quoted 6 lines · Hans de Goede
From: Roy Spliet <[email protected]>
Under the assumptions of having a UBI volume called boot, containing a ubifs filesystem.
Signed-off-by Hans de Goede <[email protected]>
I'd expect the person in the "From:" line above to have an s-o-b line too. If the patch has changed so much that isn't appropriate, perhaps change the patch's git author to you and just mentioned "Based on work
by: Roy ..."?
include/config_distro_bootcmd.h line 78
Quoted 6 lines
include/config_distro_bootcmd.h +4
-0+#ifdef CONFIG_CMD_UBIFS +#define BOOTENV_SHARED_UBIFS \ + "ubifs_boot=" \ + "if ubi part UBI && ubifsmount ubi${devnum}:boot; then " \
I don't know ubifs well enough to know the implications of that particular set of ubi commands, but as far as the impact-on/integration-into the bootcmd.h file, this patch looks fine.
PATCH v2 4/4 distro_bootcmd: Add support for booting from ubifs
Hi,
Quoted 6 lines · Stephen Warren
Quoted 5 lines · Hans de Goede
From: Roy Spliet <[email protected]>
Under the assumptions of having a UBI volume called boot, containing a ubifs filesystem.
Quoted 6 lines
Quoted 1 line
Signed-off-by Hans de Goede <[email protected]>I'd expect the person in the "From:" line above to have an s-o-b line too. If the patch has changed so much that isn't appropriate, perhaps change the patch's git author to you and just mentioned "Based on work
by: Roy ..."?
The code has changed significantly, I've send Roy a mail asking him which solution he prefers, and I'll fix things up in my tree when I've an answer.
Regards,
Hans
v2 cover distro_bootcmd: Add support for booting from ubifs
In reply to Hans de GoedeQuoted 8 lines · Hans de Goede
Hi Stephen & Scott,
As requested by Stephen here is a new version of my patch-set to add mtd with ubi on top with ubifs on top support to
distro_bootcmd, this time using the generic filesystem related commands / code.Scott, can you review the first three patches, and perhaps these should also be merged through your tree ?
I don't see anything NAND-specific about them, nor am I particularly familiar with ubifs internals or U-Boot filesystem support...
Kyungmin Park and Heiko Schocher are listed as the UBI custodians.
-Scott
v2 cover distro_bootcmd: Add support for booting from ubifs
Hi,
Quoted 7 lines · Scott Wood
Quoted 6 lines · Hans de Goede
Hi Stephen & Scott,
As requested by Stephen here is a new version of my patch-set to add mtd with ubi on top with ubifs on top support to
distro_bootcmd, this time using the generic filesystem related commands / code.
Quoted 7 lines
Quoted 2 lines
Scott, can you review the first three patches, and perhaps these should also be merged through your tree ?
I don't see anything NAND-specific about them, nor am I particularly familiar with ubifs internals or U-Boot filesystem support...
Kyungmin Park and Heiko Schocher are listed as the UBI custodians.
My bad, since you are the mtd custodian, I assumed that you would be maintaining ubi[fs] too, my mistake.
Heiko, Kyungmin (added to the Cc), can one of you please review the first 3 patches of this set. I think this is post v2015.10 material, once reviewed I can merge it through the sunxi tree, or you can pick it up.
Regards,
Hans
v2 cover distro_bootcmd: Add support for booting from ubifs
Hello Hans,
Am 25.08.2015 um 09:15 schrieb Hans de Goede:
Quoted 16 lines
Hi,
Quoted 13 lines · Scott Wood
Quoted 8 lines · Hans de Goede
Hi Stephen & Scott,
As requested by Stephen here is a new version of my patch-set to add mtd with ubi on top with ubifs on top support to
distro_bootcmd, this time using the generic filesystem related commands / code.Scott, can you review the first three patches, and perhaps these should also be merged through your tree ?
I don't see anything NAND-specific about them, nor am I particularly familiar with ubifs internals or U-Boot filesystem support...
Quoted 8 lines
Quoted 1 line
Kyungmin Park and Heiko Schocher are listed as the UBI custodians.
My bad, since you are the mtd custodian, I assumed that you would be maintaining ubi[fs] too, my mistake.
Heiko, Kyungmin (added to the Cc), can one of you please review the first 3 patches of this set. I think this is post v2015.10 material, once reviewed I can merge it through the sunxi tree, or you can pick it up.
I had it on my list, but as I thought (as you ;-) it is 2015.10 material I looked not deeper into it ... Sorry, I look into it ASAP.
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany