diff mbox

[U-Boot,v2,1/4] ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use

Message ID 1440266693-15664-2-git-send-email-hdegoede@redhat.com
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Hans de Goede Aug. 22, 2015, 6:04 p.m. UTC
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

Comments

Heiko Schocher Aug. 25, 2015, 11 a.m. UTC | #1
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)
>   {
>   	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__ */
>
Hans de Goede Aug. 25, 2015, 11:32 a.m. UTC | #2
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
Tom Rini Aug. 28, 2015, 2:52 p.m. UTC | #3
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 Schocher Aug. 28, 2015, 3:33 p.m. UTC | #4
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
Hans de Goede Aug. 31, 2015, 4:08 p.m. UTC | #5
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
Heiko Schocher Sept. 1, 2015, 3:46 a.m. UTC | #6
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
Heiko Schocher Sept. 1, 2015, 10:57 a.m. UTC | #7
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
diff mbox

Patch

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__ */