Skip to contentSkip to conversation

nios2: fix out of reach case for do_reset

PATCH In review Thomas Chou 4 participants 7 messages Aug 13, 2010 – Aug 20, 2010 NIOS2 architecture

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 nios2: fix out of reach case for do_reset

The "Nios II Processor Reference Handbook" said,
"call can transfer execution anywhere within the 256 MByte range determined by PC31..28. The Nios II GNU linker does not automatically handle cases in which the address is out of this range."

So we have to use registered "callr" instruction to do the job.

---
 arch/nios2/cpu/cpu.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)
arch/nios2/cpu/cpu.c +8 -6
index 6379534..d9c3003 100644
@@ -40,11 +40,13 @@ int checkcpu (void)
 	return (0);
 }
 
-
-int do_reset (void)
+int do_reset(void)
 {
-	void (*rst)(void) = (void(*)(void))CONFIG_SYS_RESET_ADDR;
-	disable_interrupts ();
-	rst();
-	return(0);
+	disable_interrupts();
+	__asm__ __volatile__ (
+	"callr	%0"
+	:
+	: "r" (CONFIG_SYS_RESET_ADDR)
+	);
+	return 0;
 }
Signature · 1 line
1.7.2.1
#1 lore raw

PATCH 1/1 nios2: fix out of reach case for do_reset

Quoted 6 lines · Thomas Chou

The "Nios II Processor Reference Handbook" said,
"call can transfer execution anywhere within the 256 MByte range determined by PC31..28. The Nios II GNU linker does not automatically handle cases in which the address is out of this range."

So we have to use registered "callr" instruction to do the job.

doesnt NIOS have an insn to call indirectly via a register ? typically that's how function pointers should be handled when gcc generates code.
-mike

#2 lore raw

PATCH 1/1 nios2: fix out of reach case for do_reset

Quoted 6 lines · Mike Frysinger
Quoted 5 lines · Thomas Chou

The "Nios II Processor Reference Handbook" said,
"call can transfer execution anywhere within the 256 MByte range determined by PC31..28. The Nios II GNU linker does not automatically handle cases in which the address is out of this range."

Quoted 8 lines
Quoted 1 line

So we have to use registered "callr" instruction to do the job.

doesnt NIOS have an insn to call indirectly via a register ? typically that's how function pointers should be handled when gcc generates code.
-mike

Hi Mike,

The nios2 does have "callr" instruction to do registered call. But the nios2 gcc didn't generate correct code when the reset vector is passed as a constant. It just generated a direct "call", which was wrong when the reset vector was not located in the same 256MB span as u-boot. The gcc does generate correct "callr" if the reset vector is passed as an argument. As Altera said, this is a limitation (or bug?) of nios2 toolchain.

So I'd take a similar inline asm approach as we do in linux kernel.

Best regards,
Thomas

#3 lore raw

[U-Boot] UNS: [Nios2-dev] [PATCH] nios2: fix out of reach case for do_reset

In reply to Thomas Chou

As a comment, that patch looks pretty ugly. I might suggest an inline function (called "callr" or "nios2_callr") which
encapsulates the __asm__. That would make the code much easier to read.

You could put the inline function in one of the asm headers.

-brad

Quoted 37 lines · Thomas Chou

The "Nios II Processor Reference Handbook" said,
"call can transfer execution anywhere within the 256 MByte range determined by PC31..28. The Nios II GNU linker does not automatically handle cases in which the address is out of this range."

So we have to use registered "callr" instruction to do the job.

---
 arch/nios2/cpu/cpu.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)
arch/nios2/cpu/cpu.c +8 -6
index 6379534..d9c3003 100644
@@ -40,11 +40,13 @@ int checkcpu (void)
 	return (0);
 }
 
-
-int do_reset (void)
+int do_reset(void)
 {
-	void (*rst)(void) = (void(*)(void))CONFIG_SYS_RESET_ADDR;
-	disable_interrupts ();
-	rst();
-	return(0);
+	disable_interrupts();
+	__asm__ __volatile__ (
+	"callr	%0"
+	:
+	: "r" (CONFIG_SYS_RESET_ADDR)
+	);
+	return 0;
 }
#4 lore raw

[U-Boot] UNS: [Nios2-dev] [PATCH] nios2: fix out of reach case for do_reset

Quoted 7 lines · Brad Parker

As a comment, that patch looks pretty ugly. I might suggest an inline function (called "callr" or "nios2_callr") which
encapsulates the __asm__. That would make the code much easier to read.

You could put the inline function in one of the asm headers.

-brad

Hi Brad,

Thanks. I will add a macro for the inline asm.

- Thomas

#5 lore raw

PATCH v2 1/1 nios2: fix out of reach case for do_reset

In reply to Brad Parker

There is a limitation (or bug?) of nios2 toolchain. The nios2 gcc didn't generate correct code when the reset vector is passed as a constant. It just generated a direct "call", which was wrong when the reset vector was not located in the same 256MB span as u-boot.

The "Nios II Processor Reference Handbook" said,
"call can transfer execution anywhere within the 256 MByte range determined by PC31..28. The Nios II GNU linker does not automatically handle cases in which the address is out of this range."

So we have to use registered "callr" instruction to do the job.

---
v2, add a macro for the inlince asm nios2_callr.

 arch/nios2/cpu/cpu.c            |   11 +++++------
 arch/nios2/include/asm/system.h |    5 +++++
 2 files changed, 10 insertions(+), 6 deletions(-)
arch/nios2/cpu/cpu.c +5 -6
index 6379534..d9c6544 100644
@@ -40,11 +40,10 @@ int checkcpu (void)
 	return (0);
 }
 
-
-int do_reset (void)
+int do_reset(void)
 {
-	void (*rst)(void) = (void(*)(void))CONFIG_SYS_RESET_ADDR;
-	disable_interrupts ();
-	rst();
-	return(0);
+	disable_interrupts();
+	/* indirect call to go beyond 256MB limitation of toolchain */
+	nios2_callr(CONFIG_SYS_RESET_ADDR);
+	return 0;
 }
arch/nios2/include/asm/system.h +5 -0
index bb03ca5..086d92b 100644
@@ -56,4 +56,9 @@
 	((flags & NIOS2_STATUS_PIE_MSK) == 0x0);	\
 })
 
+/* indirect call to go beyond 256MB limitation of toolchain */
+#define nios2_callr(addr) __asm__ __volatile__ (	\
+	"callr	%0"					\
+	: : "r" (addr))
+
 #endif /* __ASM_NIOS2_SYSTEM_H */
Signature · 1 line
1.7.2.1
#6 lore raw

PATCH v2 1/1 nios2: fix out of reach case for do_reset

Thomas,

Applied to: git://git.denx.de/u-boot-nios.git next

Thanks,
--Scott

Quoted 55 lines · Thomas Chou

There is a limitation (or bug?) of nios2 toolchain. The nios2 gcc didn't generate correct code when the reset vector is passed as a constant. It just generated a direct "call", which was wrong when the reset vector was not located in the same 256MB span as u-boot.

The "Nios II Processor Reference Handbook" said,
"call can transfer execution anywhere within the 256 MByte range determined by PC31..28. The Nios II GNU linker does not automatically handle cases in which the address is out of this range."

So we have to use registered "callr" instruction to do the job.

---
v2, add a macro for the inlince asm nios2_callr.

 arch/nios2/cpu/cpu.c            |   11 +++++------
 arch/nios2/include/asm/system.h |    5 +++++
 2 files changed, 10 insertions(+), 6 deletions(-)
arch/nios2/cpu/cpu.c +5 -6
index 6379534..d9c6544 100644
@@ -40,11 +40,10 @@ int checkcpu (void)
 	return (0);
 }
 
-
-int do_reset (void)
+int do_reset(void)
 {
-	void (*rst)(void) = (void(*)(void))CONFIG_SYS_RESET_ADDR;
-	disable_interrupts ();
-	rst();
-	return(0);
+	disable_interrupts();
+	/* indirect call to go beyond 256MB limitation of toolchain */
+	nios2_callr(CONFIG_SYS_RESET_ADDR);
+	return 0;
 }
arch/nios2/include/asm/system.h +5 -0
index bb03ca5..086d92b 100644
@@ -56,4 +56,9 @@
 	((flags & NIOS2_STATUS_PIE_MSK) == 0x0);	\
 })
 
+/* indirect call to go beyond 256MB limitation of toolchain */
+#define nios2_callr(addr) __asm__ __volatile__ (	\
+	"callr	%0"					\
+	: : "r" (addr))
+
 #endif /* __ASM_NIOS2_SYSTEM_H */