Patchwork [U-Boot] fs/fs.c: fix fs_set_blk_dev() for manual relocation

login
register
mail settings
Submitter Andreas Bießmann
Date Oct. 30, 2012, 6:29 p.m.
Message ID <1351621763-26635-1-git-send-email-andreas.devel@googlemail.com>
Download mbox | patch
Permalink /patch/195581/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Andreas Bießmann - Oct. 30, 2012, 6:29 p.m.
Commit 045fa1e1142552799ad3203e9e0bc22a11e866ea introduce an array with
filesystem accessors. On arches which need manual reloc this is broken cause the
function pointers still pointing to the privious location, fix it.

Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com>
Cc: swarren@wwwdotorg.org
Cc: trini@ti.com
---
 fs/fs.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
Stephen Warren - Oct. 30, 2012, 6:41 p.m.
On 10/30/2012 12:29 PM, Andreas Bießmann wrote:
> Commit 045fa1e1142552799ad3203e9e0bc22a11e866ea introduce an array with
> filesystem accessors. On arches which need manual reloc this is broken cause the
> function pointers still pointing to the privious location, fix it.

We found the same code to copy:-)

> diff --git a/fs/fs.c b/fs/fs.c

>  int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)

> +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> +	static int relocated = 0;

checkpatch bitches about the "= 0" there. I assume BSS initialization
isn't also broken when CONFIG_NEEDS_MANUAL_RELOC is enabled?
Tom Rini - Oct. 30, 2012, 10:19 p.m.
On Tue, Oct 30, 2012 at 12:41:02PM -0600, Stephen Warren wrote:
> On 10/30/2012 12:29 PM, Andreas Bie??mann wrote:
> > Commit 045fa1e1142552799ad3203e9e0bc22a11e866ea introduce an array with
> > filesystem accessors. On arches which need manual reloc this is broken cause the
> > function pointers still pointing to the privious location, fix it.
> 
> We found the same code to copy:-)
> 
> > diff --git a/fs/fs.c b/fs/fs.c
> 
> >  int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
> 
> > +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> > +	static int relocated = 0;
> 
> checkpatch bitches about the "= 0" there. I assume BSS initialization
> isn't also broken when CONFIG_NEEDS_MANUAL_RELOC is enabled?

The only other example of relocation this way also does an inital
assignment to 0.  Andreas, can you run-time test?  Thanks.
Andreas Bießmann - Oct. 31, 2012, 9:42 a.m.
Dear Stephen Warren,

On 30.10.2012 19:41, Stephen Warren wrote:
> On 10/30/2012 12:29 PM, Andreas Bießmann wrote:
>> Commit 045fa1e1142552799ad3203e9e0bc22a11e866ea introduce an array with
>> filesystem accessors. On arches which need manual reloc this is broken cause the
>> function pointers still pointing to the privious location, fix it.
> 
> We found the same code to copy:-)

we did ;)

>> diff --git a/fs/fs.c b/fs/fs.c
> 
>>  int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
> 
>> +#ifdef CONFIG_NEEDS_MANUAL_RELOC
>> +	static int relocated = 0;
> 
> checkpatch bitches about the "= 0" there. I assume BSS initialization
> isn't also broken when CONFIG_NEEDS_MANUAL_RELOC is enabled?

For AVR32 BSS initialization is working correctly, I do not know how the
other arches behave, but I think it is Ok to remove the '0' here.
Also the check for fstypes[i].probe != NULL in my patch seems to be not
necessary. So I would favor using your patch then.

Best regards

Andreas Bießmann

Patch

diff --git a/fs/fs.c b/fs/fs.c
index 23ffa25..66feb30 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -21,6 +21,8 @@ 
 #include <fat.h>
 #include <fs.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static block_dev_desc_t *fs_dev_desc;
 static disk_partition_t fs_partition;
 static int fs_type = FS_TYPE_ANY;
@@ -141,7 +143,7 @@  static inline void fs_close_ext(void)
 #define fs_read_ext fs_read_unsupported
 #endif
 
-static const struct {
+static struct {
 	int fstype;
 	int (*probe)(void);
 } fstypes[] = {
@@ -158,6 +160,18 @@  static const struct {
 int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
 {
 	int part, i;
+#ifdef CONFIG_NEEDS_MANUAL_RELOC
+	static int relocated = 0;
+
+	/* relocate fstypes[].probe */
+	if (!relocated) {
+		int i;
+		for (i = 0; i < ARRAY_SIZE(fstypes); i++)
+			if (fstypes[i].probe != NULL)
+				fstypes[i].probe += gd->reloc_off;
+		relocated = 1;
+	}
+#endif
 
 	part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc,
 					&fs_partition, 1);