Patchwork [U-Boot] ubifs: Allow ubifsmount volume reference by number

login
register
mail settings
Submitter Joe Hershberger
Date Nov. 2, 2012, 2:54 a.m.
Message ID <1351824858-10103-1-git-send-email-joe.hershberger@ni.com>
Download mbox | patch
Permalink /patch/196465/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Joe Hershberger - Nov. 2, 2012, 2:54 a.m.
UBI can mount volumes by name or number  The current code forces you
to name the volume by prepending every name with "ubi:".

From fs/ubifs/super.c
 * There are several ways to specify UBI volumes when mounting UBIFS:
 * o ubiX_Y    - UBI device number X, volume Y;
 * o ubiY      - UBI device number 0, volume Y;
 * o ubiX:NAME - mount UBI device X, volume with name NAME;
 * o ubi:NAME  - mount UBI device 0, volume with name NAME.

Now any name passed in any of the above forms are allowed.

Also update the configs that referenced ubifsmount.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 fs/ubifs/super.c                | 4 +---
 include/configs/apx4devkit.h    | 2 +-
 include/configs/cam_enc_4xx.h   | 2 +-
 include/configs/dockstar.h      | 2 +-
 include/configs/ea20.h          | 6 +++---
 include/configs/ib62x0.h        | 2 +-
 include/configs/iconnect.h      | 2 +-
 include/configs/omap3_pandora.h | 3 ++-
 include/configs/tricorder.h     | 2 +-
 include/configs/x600.h          | 2 +-
 10 files changed, 13 insertions(+), 14 deletions(-)
Vikram Narayanan - Nov. 5, 2012, 4:43 p.m.
On 11/2/2012 8:24 AM, Joe Hershberger wrote:
> UBI can mount volumes by name or number  The current code forces you
> to name the volume by prepending every name with "ubi:".
>
>  From fs/ubifs/super.c
>   * There are several ways to specify UBI volumes when mounting UBIFS:
>   * o ubiX_Y    - UBI device number X, volume Y;
>   * o ubiY      - UBI device number 0, volume Y;
>   * o ubiX:NAME - mount UBI device X, volume with name NAME;
>   * o ubi:NAME  - mount UBI device 0, volume with name NAME.
>
> Now any name passed in any of the above forms are allowed.

What exactly are we gaining from this naming compared to the default?
In what way the old naming affects the end user?

Regards,
Vikram
Joe Hershberger - Nov. 5, 2012, 4:49 p.m.
Hi Vikram,

On Mon, Nov 5, 2012 at 10:43 AM, Vikram Narayanan <vikram186@gmail.com> wrote:
>
> On 11/2/2012 8:24 AM, Joe Hershberger wrote:
>>
>> UBI can mount volumes by name or number  The current code forces you
>> to name the volume by prepending every name with "ubi:".
>>
>>  From fs/ubifs/super.c
>>   * There are several ways to specify UBI volumes when mounting UBIFS:
>>   * o ubiX_Y    - UBI device number X, volume Y;
>>   * o ubiY      - UBI device number 0, volume Y;
>>   * o ubiX:NAME - mount UBI device X, volume with name NAME;
>>   * o ubi:NAME  - mount UBI device 0, volume with name NAME.
>>
>> Now any name passed in any of the above forms are allowed.
>
>
> What exactly are we gaining from this naming compared to the default?
> In what way the old naming affects the end user?

The reason I needed this change is so that I could specify the volume
by number in a script instead of being forced to know the name.  In
Linux you can use the volume number using the format above (it's the
same code).

-Joe
Vikram Narayanan - Nov. 5, 2012, 5:36 p.m.
Hi Joe,

On 11/5/2012 10:19 PM, Joe Hershberger wrote:
> Hi Vikram,
>
> On Mon, Nov 5, 2012 at 10:43 AM, Vikram Narayanan<vikram186@gmail.com>  wrote:
>>
>> On 11/2/2012 8:24 AM, Joe Hershberger wrote:
>>>
>>> UBI can mount volumes by name or number  The current code forces you
>>> to name the volume by prepending every name with "ubi:".
>>>
>>>   From fs/ubifs/super.c
>>>    * There are several ways to specify UBI volumes when mounting UBIFS:
>>>    * o ubiX_Y    - UBI device number X, volume Y;
>>>    * o ubiY      - UBI device number 0, volume Y;
>>>    * o ubiX:NAME - mount UBI device X, volume with name NAME;
>>>    * o ubi:NAME  - mount UBI device 0, volume with name NAME.
>>>
>>> Now any name passed in any of the above forms are allowed.
>>
>>
>> What exactly are we gaining from this naming compared to the default?
>> In what way the old naming affects the end user?
>
> The reason I needed this change is so that I could specify the volume
> by number in a script instead of being forced to know the name.  In
> Linux you can use the volume number using the format above (it's the
> same code).

If that is the case, then the one can create the partition using the 
below command,

 > ubi createvol <vol name>

<vol name> can be any of the 4 methods listed above.

and the prefix "ubi:" will be appended in the environment which is 
expected by kernel for mounting. Right?

~Vikram
Joe Hershberger - Nov. 5, 2012, 6:18 p.m.
Hi Vikram,

On Mon, Nov 5, 2012 at 11:36 AM, Vikram Narayanan <vikram186@gmail.com> wrote:
> Hi Joe,
>
>
> On 11/5/2012 10:19 PM, Joe Hershberger wrote:
>>
>> Hi Vikram,
>>
>> On Mon, Nov 5, 2012 at 10:43 AM, Vikram Narayanan<vikram186@gmail.com>
>> wrote:
>>>
>>>
>>> On 11/2/2012 8:24 AM, Joe Hershberger wrote:
>>>>
>>>>
>>>> UBI can mount volumes by name or number  The current code forces you
>>>> to name the volume by prepending every name with "ubi:".
>>>>
>>>>   From fs/ubifs/super.c
>>>>    * There are several ways to specify UBI volumes when mounting UBIFS:
>>>>    * o ubiX_Y    - UBI device number X, volume Y;
>>>>    * o ubiY      - UBI device number 0, volume Y;
>>>>    * o ubiX:NAME - mount UBI device X, volume with name NAME;
>>>>    * o ubi:NAME  - mount UBI device 0, volume with name NAME.
>>>>
>>>> Now any name passed in any of the above forms are allowed.
>>>
>>>
>>>
>>> What exactly are we gaining from this naming compared to the default?
>>> In what way the old naming affects the end user?
>>
>>
>> The reason I needed this change is so that I could specify the volume
>> by number in a script instead of being forced to know the name.  In
>> Linux you can use the volume number using the format above (it's the
>> same code).
>
>
> If that is the case, then the one can create the partition using the below
> command,
>
>> ubi createvol <vol name>
>
> <vol name> can be any of the 4 methods listed above.

I believe that is incorrect.  <vol name> is literally the name that
can be used in the form "ubiX:<vol name>".  It doesn't make sense to
use the above forms when creating the volume.  The ubi index is
already set by calling ubi part <mtd partition>.  The volume index is
simply the next volume in that ubi.

> and the prefix "ubi:" will be appended in the environment which is expected
> by kernel for mounting. Right?

I assume you meant prepended.  Please expand on what you are saying
here.  What environment variable?  Prepended by what / who?  Yes, the
kernel command line expects a full name in one of the 4 forms above.

-Joe
Vikram Narayanan - Nov. 5, 2012, 6:41 p.m.
On 11/5/2012 11:48 PM, Joe Hershberger wrote:
> Hi Vikram,
>
> On Mon, Nov 5, 2012 at 11:36 AM, Vikram Narayanan<vikram186@gmail.com>  wrote:
>> Hi Joe,
>>
>>
>> On 11/5/2012 10:19 PM, Joe Hershberger wrote:
>>>
>>> Hi Vikram,
>>>
>>> On Mon, Nov 5, 2012 at 10:43 AM, Vikram Narayanan<vikram186@gmail.com>
>>> wrote:
>>>>
>>>>
>>>> On 11/2/2012 8:24 AM, Joe Hershberger wrote:
>>>>>
>>>>>
>>>>> UBI can mount volumes by name or number  The current code forces you
>>>>> to name the volume by prepending every name with "ubi:".
>>>>>
>>>>>    From fs/ubifs/super.c
>>>>>     * There are several ways to specify UBI volumes when mounting UBIFS:
>>>>>     * o ubiX_Y    - UBI device number X, volume Y;
>>>>>     * o ubiY      - UBI device number 0, volume Y;
>>>>>     * o ubiX:NAME - mount UBI device X, volume with name NAME;
>>>>>     * o ubi:NAME  - mount UBI device 0, volume with name NAME.
>>>>>
>>>>> Now any name passed in any of the above forms are allowed.
>>>>
>>>>
>>>>
>>>> What exactly are we gaining from this naming compared to the default?
>>>> In what way the old naming affects the end user?
>>>
>>>
>>> The reason I needed this change is so that I could specify the volume
>>> by number in a script instead of being forced to know the name.  In
>>> Linux you can use the volume number using the format above (it's the
>>> same code).
>>
>>
>> If that is the case, then the one can create the partition using the below
>> command,
>>
>>> ubi createvol<vol name>
>>
>> <vol name>  can be any of the 4 methods listed above.
>
> I believe that is incorrect.<vol name>  is literally the name that
> can be used in the form "ubiX:<vol name>".  It doesn't make sense to
> use the above forms when creating the volume.  The ubi index is
> already set by calling ubi part<mtd partition>.  The volume index is
> simply the next volume in that ubi.

say I've an mtdpartition by the name "filesystem"

ubi part filesystem
ubi createvol rootfs

If I invoke the above commands, I'll have a ubi volume with a name 
rootfs with the complete size of (filesystem - metadata).

When passing bootargs to the kernel I'd give the root as
root=ubi0:rootfs

Why should this be wrong?

>
>> and the prefix "ubi:" will be appended in the environment which is expected
>> by kernel for mounting. Right?
>
> I assume you meant prepended.  Please expand on what you are saying

Yes. I meant prepended. sorry.

> here.  What environment variable?  Prepended by what / who?  Yes, the

Prepended by the string "ubi:" as you've done in this patch for some 
configs.

~Vikram
Joe Hershberger - Nov. 5, 2012, 6:49 p.m.
Hi Vikram,

On Mon, Nov 5, 2012 at 12:41 PM, Vikram Narayanan <vikram186@gmail.com> wrote:
> On 11/5/2012 11:48 PM, Joe Hershberger wrote:
>>
>> Hi Vikram,
>>
>> On Mon, Nov 5, 2012 at 11:36 AM, Vikram Narayanan<vikram186@gmail.com>
>> wrote:
>>>
>>> Hi Joe,
>>>
>>>
>>> On 11/5/2012 10:19 PM, Joe Hershberger wrote:
>>>>
>>>>
>>>> Hi Vikram,
>>>>
>>>> On Mon, Nov 5, 2012 at 10:43 AM, Vikram Narayanan<vikram186@gmail.com>
>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 11/2/2012 8:24 AM, Joe Hershberger wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> UBI can mount volumes by name or number  The current code forces you
>>>>>> to name the volume by prepending every name with "ubi:".
>>>>>>
>>>>>>    From fs/ubifs/super.c
>>>>>>     * There are several ways to specify UBI volumes when mounting
>>>>>> UBIFS:
>>>>>>     * o ubiX_Y    - UBI device number X, volume Y;
>>>>>>     * o ubiY      - UBI device number 0, volume Y;
>>>>>>     * o ubiX:NAME - mount UBI device X, volume with name NAME;
>>>>>>     * o ubi:NAME  - mount UBI device 0, volume with name NAME.
>>>>>>
>>>>>> Now any name passed in any of the above forms are allowed.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> What exactly are we gaining from this naming compared to the default?
>>>>> In what way the old naming affects the end user?
>>>>
>>>>
>>>>
>>>> The reason I needed this change is so that I could specify the volume
>>>> by number in a script instead of being forced to know the name.  In
>>>> Linux you can use the volume number using the format above (it's the
>>>> same code).
>>>
>>>
>>>
>>> If that is the case, then the one can create the partition using the
>>> below
>>> command,
>>>
>>>> ubi createvol<vol name>
>>>
>>>
>>> <vol name>  can be any of the 4 methods listed above.

This is wrong.

>> I believe that is incorrect.<vol name>  is literally the name that
>> can be used in the form "ubiX:<vol name>".  It doesn't make sense to
>> use the above forms when creating the volume.  The ubi index is
>> already set by calling ubi part<mtd partition>.  The volume index is
>> simply the next volume in that ubi.
>
>
> say I've an mtdpartition by the name "filesystem"
>
> ubi part filesystem
> ubi createvol rootfs
>
> If I invoke the above commands, I'll have a ubi volume with a name rootfs
> with the complete size of (filesystem - metadata).
>
> When passing bootargs to the kernel I'd give the root as
> root=ubi0:rootfs
>
> Why should this be wrong?

This sequence of commands isn't wrong.  What you claimed above is
wrong: "<vol name>  can be any of the 4 methods listed above".

>>
>>> and the prefix "ubi:" will be appended in the environment which is
>>> expected
>>> by kernel for mounting. Right?
>>
>>
>> I assume you meant prepended.  Please expand on what you are saying
>
>
> Yes. I meant prepended. sorry.
>
>
>> here.  What environment variable?  Prepended by what / who?  Yes, the
>
>
> Prepended by the string "ubi:" as you've done in this patch for some
> configs.

Ok... the whole point of this patch is to change the behavior of the
ubifsmount command.  This makes the ubifsmount command take the same
form as the kernel command line does.  This patch has nothing to do
with the ubi createvol command.

-Joe
Tom Rini - March 4, 2013, 9:27 p.m.
On Thu, Nov 01, 2012 at 04:54:18PM -0000, Joe Hershberger wrote:

> UBI can mount volumes by name or number  The current code forces you
> to name the volume by prepending every name with "ubi:".
> 
> >From fs/ubifs/super.c
>  * There are several ways to specify UBI volumes when mounting UBIFS:
>  * o ubiX_Y    - UBI device number X, volume Y;
>  * o ubiY      - UBI device number 0, volume Y;
>  * o ubiX:NAME - mount UBI device X, volume with name NAME;
>  * o ubi:NAME  - mount UBI device 0, volume with name NAME.
> 
> Now any name passed in any of the above forms are allowed.
> 
> Also update the configs that referenced ubifsmount.
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot/master, thanks!
Tom Rini - March 4, 2013, 9:27 p.m.
On Thu, Nov 01, 2012 at 04:54:18PM -0000, Joe Hershberger wrote:

> UBI can mount volumes by name or number  The current code forces you
> to name the volume by prepending every name with "ubi:".
> 
> >From fs/ubifs/super.c
>  * There are several ways to specify UBI volumes when mounting UBIFS:
>  * o ubiX_Y    - UBI device number X, volume Y;
>  * o ubiY      - UBI device number 0, volume Y;
>  * o ubiX:NAME - mount UBI device X, volume with name NAME;
>  * o ubi:NAME  - mount UBI device 0, volume with name NAME.
> 
> Now any name passed in any of the above forms are allowed.
> 
> Also update the configs that referenced ubifsmount.
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot/master, thanks!

Patch

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 30ccd98..9acf243 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1164,10 +1164,9 @@  static struct file_system_type ubifs_fs_type = {
 	.get_sb  = ubifs_get_sb,
 };
 
-int ubifs_mount(char *vol_name)
+int ubifs_mount(char *name)
 {
 	int flags;
-	char name[80] = "ubi:";
 	void *data;
 	struct vfsmount *mnt;
 	int ret;
@@ -1186,7 +1185,6 @@  int ubifs_mount(char *vol_name)
 	 * Mount in read-only mode
 	 */
 	flags = MS_RDONLY;
-	strcat(name, vol_name);
 	data = NULL;
 	mnt = NULL;
 	ret = ubifs_get_sb(&ubifs_fs_type, flags, name, data, mnt);
diff --git a/include/configs/apx4devkit.h b/include/configs/apx4devkit.h
index 6764b47..b809dfe 100644
--- a/include/configs/apx4devkit.h
+++ b/include/configs/apx4devkit.h
@@ -223,7 +223,7 @@ 
 		"root=ubi0:rootfs rootfstype=ubifs ${mtdparts} rw\0" \
 	"bootcmd_nand=" \
 		"run bootargs_nand && ubi part root 2048 && " \
-		"ubifsmount rootfs && ubifsload 41000000 boot/uImage && " \
+		"ubifsmount ubi:rootfs && ubifsload 41000000 boot/uImage && " \
 		"bootm 41000000\0" \
 	"bootargs_mmc=" \
 		"setenv bootargs ${kernelargs} " \
diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
index 56dc1cb..38a2c1b 100644
--- a/include/configs/cam_enc_4xx.h
+++ b/include/configs/cam_enc_4xx.h
@@ -448,7 +448,7 @@ 
 	"bootfile=" __stringify(CONFIG_HOSTNAME) "/uImage \0"		\
 	"kernel_addr_r=80600000\0"					\
 	"load_kernel=tftp ${kernel_addr_r} ${bootfile}\0"		\
-	"ubi_load_kernel=ubi part ubi 2048;ubifsmount ${img_volume};"	\
+	"ubi_load_kernel=ubi part ubi 2048;ubifsmount ubi:${img_volume};" \
 		"ubifsload ${kernel_addr_r} boot/uImage\0"		\
 	"fit_addr_r=" __stringify(CONFIG_BOARD_IMG_ADDR_R) "\0"		\
 	"img_addr_r=" __stringify(CONFIG_BOARD_IMG_ADDR_R) "\0"		\
diff --git a/include/configs/dockstar.h b/include/configs/dockstar.h
index 249f93b..63d5e35 100644
--- a/include/configs/dockstar.h
+++ b/include/configs/dockstar.h
@@ -85,7 +85,7 @@ 
 #define CONFIG_BOOTCOMMAND \
 	"setenv bootargs ${console} ${mtdparts} ${bootargs_root}; "	\
 	"ubi part root; " \
-	"ubifsmount root; " \
+	"ubifsmount ubi:root; " \
 	"ubifsload 0x800000 ${kernel}; " \
 	"ubifsload 0x1100000 ${initrd}; " \
 	"bootm 0x800000 0x1100000"
diff --git a/include/configs/ea20.h b/include/configs/ea20.h
index d3eb596..a8c08e8 100644
--- a/include/configs/ea20.h
+++ b/include/configs/ea20.h
@@ -293,12 +293,12 @@ 
 		"bootm ${kernel_addr_r}\0"				\
 	"net_self_load=tftp ${kernel_addr_r} ${bootfile};"		\
 		"tftp ${ramdisk_addr_r} ${ramdisk_file};\0"		\
-	"nand_nand=ubi part nand0,${as};ubifsmount rootfs;"		\
+	"nand_nand=ubi part nand0,${as};ubifsmount ubi:rootfs;"		\
 		"ubifsload ${kernel_addr_r} /boot/uImage;"		\
 		"ubifsumount; run nandargs addip addtty "		\
 		"addmtd addmisc addmem;clrlogo;"			\
 		"bootm ${kernel_addr_r}\0"				\
-	"nand_nandrw=ubi part nand0,${as};ubifsmount rootfs;"		\
+	"nand_nandrw=ubi part nand0,${as};ubifsmount ubi:rootfs;"	\
 		"ubifsload ${kernel_addr_r} /boot/uImage;"		\
 		"ubifsumount; run nandrwargs addip addtty "		\
 		"addmtd addmisc addmem;clrlogo;"			\
@@ -309,7 +309,7 @@ 
 	"u-boot=" __stringify(CONFIG_HOSTNAME) "/u-boot.bin\0"		\
 	"load_magic=if sf probe 0;then sf "				\
 		"read c0000000 0x10000 0x60000;fi\0"			\
-	"load_nand=ubi part nand0,${as};ubifsmount rootfs;"		\
+	"load_nand=ubi part nand0,${as};ubifsmount ubi:rootfs;"		\
 		"if ubifsload c0000014 /boot/u-boot.bin;"		\
 		"then mw c0000008 ${filesize};else echo Error reading"	\
 		" u-boot from nand!;fi\0"				\
diff --git a/include/configs/ib62x0.h b/include/configs/ib62x0.h
index 85856f2..440206f 100644
--- a/include/configs/ib62x0.h
+++ b/include/configs/ib62x0.h
@@ -88,7 +88,7 @@ 
 #define CONFIG_BOOTCOMMAND \
 	"setenv bootargs ${console} ${mtdparts} ${bootargs_root}; "	\
 	"ubi part root; "						\
-	"ubifsmount root; "						\
+	"ubifsmount ubi:root; "						\
 	"ubifsload 0x800000 ${kernel}; "				\
 	"ubifsload 0x1100000 ${initrd}; "				\
 	"bootm 0x800000 0x1100000"
diff --git a/include/configs/iconnect.h b/include/configs/iconnect.h
index 2b523c9..ef41a87 100644
--- a/include/configs/iconnect.h
+++ b/include/configs/iconnect.h
@@ -87,7 +87,7 @@ 
 #define CONFIG_BOOTCOMMAND \
 	"setenv bootargs ${console} ${mtdparts} ${bootargs_root}; "	\
 	"ubi part rootfs; "						\
-	"ubifsmount rootfs; "						\
+	"ubifsmount ubi:rootfs; "					\
 	"ubifsload 0x800000 ${kernel}; "				\
 	"bootm 0x800000"
 
diff --git a/include/configs/omap3_pandora.h b/include/configs/omap3_pandora.h
index 8a8a5d1..ecb0aa5 100644
--- a/include/configs/omap3_pandora.h
+++ b/include/configs/omap3_pandora.h
@@ -182,7 +182,8 @@ 
 			"ext2load mmc1 0 ${loadaddr} autoboot.scr; then " \
 		"source ${loadaddr}; " \
 	"fi; " \
-	"ubi part boot && ubifsmount boot && ubifsload ${loadaddr} uImage && bootm ${loadaddr}"
+	"ubi part boot && ubifsmount ubi:boot && " \
+		"ubifsload ${loadaddr} uImage && bootm ${loadaddr}"
 
 #define CONFIG_AUTO_COMPLETE	1
 /*
diff --git a/include/configs/tricorder.h b/include/configs/tricorder.h
index 5859a73..326ba90 100644
--- a/include/configs/tricorder.h
+++ b/include/configs/tricorder.h
@@ -195,7 +195,7 @@ 
 		"bootm ${loadaddr}\0" \
 	"loaduimage_ubi=mtd default; " \
 		"ubi part fs; " \
-		"ubifsmount root; " \
+		"ubifsmount ubi:root; " \
 		"ubifsload ${loadaddr} /boot/uImage\0" \
 	"nandboot=echo Booting from nand ...; " \
 		"run nandargs; " \
diff --git a/include/configs/x600.h b/include/configs/x600.h
index 3082aaa..bb495a1 100644
--- a/include/configs/x600.h
+++ b/include/configs/x600.h
@@ -262,7 +262,7 @@ 
 	"nand_ubifs=run ubifs_mount ubifs_load ubifsargs addip"		\
 		" addcon addmisc addmtd;"				\
 		"bootm ${kernel_addr} - ${dtb_addr}\0"			\
-	"ubifs_mount=ubi part ubi${boot_part};ubifsmount rootfs\0"	\
+	"ubifs_mount=ubi part ubi${boot_part};ubifsmount ubi:rootfs\0"	\
 	"ubifs_load=ubifsload ${kernel_addr} ${kernel_fs};"		\
 		"ubifsload ${dtb_addr} ${dtb_fs};\0"			\
 	"nand_ubifs=run ubifs_mount ubifs_load ubifsargs addip addcon "	\