[U-Boot] [PATCH v2 0/4] 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
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.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- 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
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index 8e9a4e5..0a3dd24 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -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; } diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 6dd6174..5af861c 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -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; + } +} diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index a51b237..225965c 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -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__ */ diff --git a/include/ubifs_uboot.h b/include/ubifs_uboot.h new file mode 100644 index 0000000..d10a909 --- /dev/null +++ b/include/ubifs_uboot.h @@ -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@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__ */
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
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.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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:
Reviewed-by: Heiko Schocher hs@denx.de
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index 8e9a4e5..0a3dd24 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -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
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; }
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 6dd6174..5af861c 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -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)@@ -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;- }
+} diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index a51b237..225965c 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -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__ */ diff --git a/include/ubifs_uboot.h b/include/ubifs_uboot.h new file mode 100644 index 0000000..d10a909 --- /dev/null +++ b/include/ubifs_uboot.h @@ -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@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__ */
Hi,
On 25-08-15 13:00, Heiko Schocher wrote:
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
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.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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:
Reviewed-by: Heiko Schocher hs@denx.de
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index 8e9a4e5..0a3dd24 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -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?
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
On Tue, Aug 25, 2015 at 01:32:05PM +0200, Hans de Goede wrote:
Hi,
On 25-08-15 13:00, Heiko Schocher wrote:
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
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.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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:
Reviewed-by: Heiko Schocher hs@denx.de
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index 8e9a4e5..0a3dd24 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -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?
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 ?
I can just take 'em all into master :)
Hello tom,
Am 28.08.2015 um 16:52 schrieb Tom Rini:
On Tue, Aug 25, 2015 at 01:32:05PM +0200, Hans de Goede wrote:
Hi,
On 25-08-15 13:00, Heiko Schocher wrote:
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
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.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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:
Reviewed-by: Heiko Schocher hs@denx.de
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index 8e9a4e5..0a3dd24 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -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?
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 ?
I can just take 'em all into master :)
Ok, from my side ...
bye, Heiko
Hi,
On 28-08-15 16:52, Tom Rini wrote:
On Tue, Aug 25, 2015 at 01:32:05PM +0200, Hans de Goede wrote:
Hi,
On 25-08-15 13:00, Heiko Schocher wrote:
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
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.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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:
Reviewed-by: Heiko Schocher hs@denx.de
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index 8e9a4e5..0a3dd24 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -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?
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 ?
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
Hello Hans,
Am 31.08.2015 um 18:08 schrieb Hans de Goede:
Hi,
On 28-08-15 16:52, Tom Rini wrote:
On Tue, Aug 25, 2015 at 01:32:05PM +0200, Hans de Goede wrote:
Hi,
On 25-08-15 13:00, Heiko Schocher wrote:
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
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.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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:
Reviewed-by: Heiko Schocher hs@denx.de
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index 8e9a4e5..0a3dd24 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -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?
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 ?
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
Hello Hans,
Am 01.09.2015 um 05:46 schrieb Heiko Schocher:
Hello Hans,
Am 31.08.2015 um 18:08 schrieb Hans de Goede:
Hi,
On 28-08-15 16:52, Tom Rini wrote:
On Tue, Aug 25, 2015 at 01:32:05PM +0200, Hans de Goede wrote:
Hi,
On 25-08-15 13:00, Heiko Schocher wrote:
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
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.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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:
Reviewed-by: Heiko Schocher hs@denx.de
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index 8e9a4e5..0a3dd24 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -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?
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 ?
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.
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
Implement the necessary functions for implementing generic fs support for ubifs.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- fs/ubifs/ubifs.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/ubifs_uboot.h | 4 ++++ 2 files changed, 66 insertions(+)
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 5af861c..89f1f2a 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -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) { diff --git a/include/ubifs_uboot.h b/include/ubifs_uboot.h index d10a909..c600e38 100644 --- a/include/ubifs_uboot.h +++ b/include/ubifs_uboot.h @@ -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__ */
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Implement the necessary functions for implementing generic fs support for ubifs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
fs/ubifs/ubifs.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/ubifs_uboot.h | 4 ++++ 2 files changed, 66 insertions(+)
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 5af861c..89f1f2a 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -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) {
diff --git a/include/ubifs_uboot.h b/include/ubifs_uboot.h index d10a909..c600e38 100644 --- a/include/ubifs_uboot.h +++ b/include/ubifs_uboot.h @@ -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__ */
On 08/22/2015 11:04 AM, Hans de Goede wrote:
Implement the necessary functions for implementing generic fs support for ubifs.
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
+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;
+}
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.
Hi
On Sep 1, 2015 9:57 PM, "Stephen Warren" swarren@wwwdotorg.org wrote:
On 08/22/2015 11:04 AM, Hans de Goede wrote:
Implement the necessary functions for implementing generic fs support for ubifs.
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
+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;+}
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.
Personally I don't like return code like -1
Michael _______________________________________________
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi,
On 01-09-15 21:57, Stephen Warren wrote:
On 08/22/2015 11:04 AM, Hans de Goede wrote:
Implement the necessary functions for implementing generic fs support for ubifs.
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
+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;
+}
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.
Ack, will fix.
Regards,
Hans
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- disk/part.c | 23 +++++++++++++++++++++++ fs/fs.c | 16 ++++++++++++++++ include/fs.h | 1 + 3 files changed, 40 insertions(+)
diff --git a/disk/part.c b/disk/part.c index 43485c9..e1a8bde 100644 --- a/disk/part.c +++ b/disk/part.c @@ -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, "-")) diff --git a/fs/fs.c b/fs/fs.c index 827b143..b2d6a53 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -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", diff --git a/include/fs.h b/include/fs.h index fd1e4ab..059a395 100644 --- a/include/fs.h +++ b/include/fs.h @@ -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
Hello Hans,
Am 22.08.2015 um 20:04 schrieb Hans de Goede:
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
disk/part.c | 23 +++++++++++++++++++++++ fs/fs.c | 16 ++++++++++++++++ include/fs.h | 1 + 3 files changed, 40 insertions(+)
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko
diff --git a/disk/part.c b/disk/part.c index 43485c9..e1a8bde 100644 --- a/disk/part.c +++ b/disk/part.c @@ -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, "-"))
diff --git a/fs/fs.c b/fs/fs.c index 827b143..b2d6a53 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -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", diff --git a/include/fs.h b/include/fs.h index fd1e4ab..059a395 100644 --- a/include/fs.h +++ b/include/fs.h @@ -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
On 08/22/2015 11:04 AM, Hans de Goede wrote:
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
@@ -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
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?
Hi,
On 01-09-15 22:03, Stephen Warren wrote:
On 08/22/2015 11:04 AM, Hans de Goede wrote:
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
@@ -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
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?
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.
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?
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
On 09/14/2015 11:35 AM, Hans de Goede wrote:
Hi,
On 01-09-15 22:03, Stephen Warren wrote:
On 08/22/2015 11:04 AM, Hans de Goede wrote:
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
@@ -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 mountvolume 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
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?
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.
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?
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.
From: Roy Spliet r.spliet@ultimaker.com
Under the assumptions of having a UBI volume called boot, containing a ubifs filesystem.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- include/config_distro_bootcmd.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 3a360ca4..2b36d80 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -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" \
On 08/22/2015 11:04 AM, Hans de Goede wrote:
From: Roy Spliet r.spliet@ultimaker.com
Under the assumptions of having a UBI volume called boot, containing a ubifs filesystem.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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 ..."?
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
+#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.
Acked-by: Stephen Warren swarren@nvidia.com
Hi,
On 01-09-15 22:13, Stephen Warren wrote:
On 08/22/2015 11:04 AM, Hans de Goede wrote:
From: Roy Spliet r.spliet@ultimaker.com
Under the assumptions of having a UBI volume called boot, containing a ubifs filesystem.
Signed-off-by: Hans de Goede hdegoede@redhat.com
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
On Sat, 2015-08-22 at 20:04 +0200, Hans de Goede wrote:
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
Hi,
On 24-08-15 18:57, Scott Wood wrote:
On Sat, 2015-08-22 at 20:04 +0200, Hans de Goede wrote:
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.
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
Hello Hans,
Am 25.08.2015 um 09:15 schrieb Hans de Goede:
Hi,
On 24-08-15 18:57, Scott Wood wrote:
On Sat, 2015-08-22 at 20:04 +0200, Hans de Goede wrote:
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.
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
participants (6)
-
Hans de Goede -
Heiko Schocher -
Michael Trimarchi -
Scott Wood -
Stephen Warren -
Tom Rini