Patchwork [U-Boot,v2,3/4] LaCie/common: Fix cloudbox ethernet leds

login
register
mail settings
Submitter Frederic Leroy
Date June 30, 2013, 10:12 a.m.
Message ID <1372587149-8220-4-git-send-email-fredo@starox.org>
Download mbox | patch
Permalink /patch/255873/
State Superseded
Headers show

Comments

Frederic Leroy - June 30, 2013, 10:12 a.m.
The cloudbox device have a different ethernet phy setup than other ns2
devices. We get initialization value from the GPL LaCie source

Signed-off-by: Frédéric Leroy <fredo@starox.org>
---
 board/LaCie/common/common.c | 13 ++++++++++---
 include/configs/lacie_kw.h  |  5 +++++
 2 files changed, 15 insertions(+), 3 deletions(-)
Simon Guinot - June 30, 2013, 11:01 p.m.
On Sun, Jun 30, 2013 at 12:12:28PM +0200, Frédéric Leroy wrote:
> The cloudbox device have a different ethernet phy setup than other ns2
> devices. We get initialization value from the GPL LaCie source
> 
> Signed-off-by: Frédéric Leroy <fredo@starox.org>
> ---
>  board/LaCie/common/common.c | 13 ++++++++++---
>  include/configs/lacie_kw.h  |  5 +++++
>  2 files changed, 15 insertions(+), 3 deletions(-)

Acked-by: Simon Guinot <simon.guinot@sequanux.org>

> 
> diff --git a/board/LaCie/common/common.c b/board/LaCie/common/common.c
> index a62bf9f..85480e7 100644
> --- a/board/LaCie/common/common.c
> +++ b/board/LaCie/common/common.c
> @@ -21,6 +21,12 @@
>  #define MV88E1116_RGMII_TXTM_CTRL	(1 << 4)
>  #define MV88E1116_RGMII_RXTM_CTRL	(1 << 5)
>  
> +#if !defined(MII_MARVELL_LED_REG)
> +# define MII_MARVELL_LED_REG 16
> +# define MII_MARVELL_LED_MASK 0xf0
> +# define MII_MARVELL_LED_VALUE 0x0f
> +#endif
> +
>  void mv_phy_88e1116_init(const char *name, u16 phyaddr)
>  {
>  	u16 reg;
> @@ -53,9 +59,10 @@ void mv_phy_88e1318_init(const char *name, u16 phyaddr)
>  	 * Set control mode 4 for LED[0].
>  	 */
>  	miiphy_write(name, phyaddr, MII_MARVELL_PHY_PAGE, 3);
> -	miiphy_read(name, phyaddr, 16, &reg);
> -	reg |= 0xf;
> -	miiphy_write(name, phyaddr, 16, reg);
> +	miiphy_read(name, phyaddr, MII_MARVELL_LED_REG, &reg);
> +	reg &= MII_MARVELL_LED_MASK;
> +	reg |= MII_MARVELL_LED_VALUE;
> +	miiphy_write(name, phyaddr, MII_MARVELL_LED_REG, reg);
>  
>  	/*
>  	 * Enable RGMII delay on Tx and Rx for CPU port
> diff --git a/include/configs/lacie_kw.h b/include/configs/lacie_kw.h
> index fe27bbe..02e0882 100644
> --- a/include/configs/lacie_kw.h
> +++ b/include/configs/lacie_kw.h
> @@ -138,6 +138,11 @@
>  #ifdef CONFIG_CMD_NET
>  #define CONFIG_MVGBE_PORTS		{1, 0} /* enable port 0 only */
>  #define CONFIG_NETCONSOLE
> +#if defined(CONFIG_CLOUDBOX)
> +# define MII_MARVELL_LED_REG 17
> +# define MII_MARVELL_LED_MASK 0xffc0
> +# define MII_MARVELL_LED_VALUE 0x15
> +#endif
>  #endif
>  
>  /*
> -- 
> 1.8.1.2
TigerLiu@viatech.com.cn - July 1, 2013, 9:51 a.m.
Hi, experts:
I tried to turn off gcc optimazition by changing config.mk file:
OPTFLAGS= -O0 #-fomit-frame-pointer

But failed to compile u-boot, it tipped:
......
/home/lion/Origen0921/arm-2010.09/bin/arm-none-eabi-ld:
arch/arm/cpu/armv7/libarmv7.o: relocation R_ARM_MOVW_ABS_NC against `a
local symbol' can not be used when making a shared object; recompile
with -fPIC
arch/arm/cpu/armv7/libarmv7.o: could not read symbols: Bad value
make[1]: *** [u-boot] Error 1
make[1]: Leaving directory `/home/lion/DebugLinux/uboot-0627/u-boot'
make: *** [smdkc100] Error 2


Best wishes,
Wolfgang Denk - July 1, 2013, 10:03 a.m.
Dear TigerLiu@viatech.com.cn,

In message <FE7ADED5C2218B4786C09CD97DC4C49F9403F2@exchbj02.viatech.com.bj> you wrote:
>
> I tried to turn off gcc optimazition by changing config.mk file:
> OPTFLAGS= -O0 #-fomit-frame-pointer
> 
> But failed to compile u-boot, it tipped:

Don't do it, then.

Q: why would you want to switch off optimization?  That's almost
always a bad idea...

Best regards,

Wolfgang Denk
TigerLiu@viatech.com.cn - July 1, 2013, 10:08 a.m.
Hi, Denk:
During debug u-boot with JTAG tools, maybe turning off optimizations was recommended.

Best wishes,
-----邮件原件-----
发件人: Wolfgang Denk [mailto:wd@denx.de] 
发送时间: 2013年7月1日 18:03
收件人: Tiger Liu
抄送: u-boot@lists.denx.de
主题: Re: [U-Boot] compiled failed when turned off gcc optimazition

Dear TigerLiu@viatech.com.cn,

In message <FE7ADED5C2218B4786C09CD97DC4C49F9403F2@exchbj02.viatech.com.bj> you wrote:
>
> I tried to turn off gcc optimazition by changing config.mk file:
> OPTFLAGS= -O0 #-fomit-frame-pointer
> 
> But failed to compile u-boot, it tipped:

Don't do it, then.

Q: why would you want to switch off optimization?  That's almost
always a bad idea...

Best regards,

Wolfgang Denk
Wolfgang Denk - July 1, 2013, 11:37 a.m.
Dear TigerLiu@viatech.com.cn,

please do not top post / full quote.

In message <FE7ADED5C2218B4786C09CD97DC4C49F9403F9@exchbj02.viatech.com.bj> you wrote:
>
> During debug u-boot with JTAG tools, maybe turning off optimizations was recommended.

Many people recommend many things. Even strange or non-working or
incorrect or dangerous ones.

Best regards,

Wolfgang Denk
Mike Dunn - July 1, 2013, 6:03 p.m.
On 07/01/2013 04:37 AM, Wolfgang Denk wrote:
> Dear TigerLiu@viatech.com.cn,
> 
> please do not top post / full quote.
> 
> In message <FE7ADED5C2218B4786C09CD97DC4C49F9403F9@exchbj02.viatech.com.bj> you wrote:
>>
>> During debug u-boot with JTAG tools, maybe turning off optimizations was recommended.
> 
> Many people recommend many things. Even strange or non-working or
> incorrect or dangerous ones.


A while back I tried this for the Linux kernel, with similiar results.  I just
let it go without investigating further, guessing that maybe some tricks with
compiler directives were incompatible with turning off optimization.

But there's a good motivation for wanting to turn off optimization.
Single-stepping with a debugger at the C source level is almost useless.  I've
since gotten better at single-stepping at the assembly level while using the
mixed c and assembly view of gdb.

Mike
Wolfgang Denk - July 1, 2013, 7:51 p.m.
Dear Mike Dunn,

In message <51D1C455.9010801@newsguy.com> you wrote:
>
> But there's a good motivation for wanting to turn off optimization.

I disagree here.  If you are hunting down a problem, you want to be as
close at the original code as possible.  Disabling optimization is
such a dramatic change to the generated code that you actually debug a
different program.

> Single-stepping with a debugger at the C source level is almost useless.  I've
> since gotten better at single-stepping at the assembly level while using the
> mixed c and assembly view of gdb.

Hm... Did you read up the documentation, say [1], and try out these
recommendations?

[1] http://www.denx.de/wiki/view/DULG/DebuggingTricks

Best regards,

Wolfgang Denk
Mike Dunn - July 2, 2013, 7:40 p.m.
On 07/01/2013 12:51 PM, Wolfgang Denk wrote:
> Dear Mike Dunn,
> 
> In message <51D1C455.9010801@newsguy.com> you wrote:
>>
>> But there's a good motivation for wanting to turn off optimization.
> 
> I disagree here.  If you are hunting down a problem, you want to be as
> close at the original code as possible.  Disabling optimization is
> such a dramatic change to the generated code that you actually debug a
> different program.


I guess it depends on what problem being debugged...  I think your point is
valid for things like tracking down race conditions or misbehaving hardware.
But for more mundane problems like logical errors in the code, optimizations get
in the way.  It would be nice to be able to experiment with optimizations off.
Just knowing that a particular problem still exists when unoptimized code is run
will provide some clues.


> 
>> Single-stepping with a debugger at the C source level is almost useless.  I've
>> since gotten better at single-stepping at the assembly level while using the
>> mixed c and assembly view of gdb.
> 
> Hm... Did you read up the documentation, say [1], and try out these
> recommendations?
> 
> [1] http://www.denx.de/wiki/view/DULG/DebuggingTricks


No, but I will.  Many thanks!
Mike
Graeme Russ - July 2, 2013, 10:38 p.m.
Hi Mike,

On Wed, Jul 3, 2013 at 5:40 AM, Mike Dunn <mikedunn@newsguy.com> wrote:

> On 07/01/2013 12:51 PM, Wolfgang Denk wrote:
> > Dear Mike Dunn,
> >
> > In message <51D1C455.9010801@newsguy.com> you wrote:
> >>
> >> But there's a good motivation for wanting to turn off optimization.
> >
> > I disagree here.  If you are hunting down a problem, you want to be as
> > close at the original code as possible.  Disabling optimization is
> > such a dramatic change to the generated code that you actually debug a
> > different program.
>
>
> I guess it depends on what problem being debugged...  I think your point is
> valid for things like tracking down race conditions or misbehaving
> hardware.
> But for more mundane problems like logical errors in the code,
> optimizations get
> in the way.  It would be nice to be able to experiment with optimizations
> off.
> Just knowing that a particular problem still exists when unoptimized code
> is run
> will provide some clues.
>
>
Do be honest, I have never used an online debugger

I have always found that a combination of hardware LEDs and printf's has
been more than sufficient, particularly for mundane issues.

I'll freely admit that an online debugger might been faster for me, but I
just wanted to point out that there is more than one way to skin a cat.

I also believe that the more onerous the debugging tool is (and it doesn't
get more so than using the hardware LEDs) the more attention you pay to the
code rather than the output ;)

Regards,

Graeme
TigerLiu@viatech.com.cn - July 5, 2013, 10:31 a.m.
Hi, experts:

Would any expert fix this question?

It seems related MOVW instruction's usuage when turning off gcc
optimazition.

 

If i just turned on "-O1" or "-O2", still failed to compile uboot code.

 

Best wishes,
Albert ARIBAUD - July 5, 2013, 2 p.m.
Hi TigerLiu@viatech.com.cn,

On Fri, 5 Jul 2013 18:31:50 +0800, <TigerLiu@viatech.com.cn> wrote:

> Hi, experts:
> 
> Would any expert fix this question?
> 
> It seems related MOVW instruction's usuage when turning off gcc
> optimazition.
> 
>  
> 
> If i just turned on "-O1" or "-O2", still failed to compile uboot code.

I have seen the issue arise, and looked it up with the help of Jeroen
Hofstee (Cc:). It is somehow related to the fact that on ARM, U-Boot
(especially the DM) relies on ARM ELF relocation records all being of
the R_ARM_RELATIVE type, while for -O{0,1,2} the compiler generates
(some) MOVW-related relocation record types.

I have local branches related to this. I need to go through them and
will post them but not for 2013.07 as these are not fixes, and there
is no bug per se since U-Boot is supposed to build with -Os, not
-O{0,1,2}.

> Best wishes,

Amicalement,

Patch

diff --git a/board/LaCie/common/common.c b/board/LaCie/common/common.c
index a62bf9f..85480e7 100644
--- a/board/LaCie/common/common.c
+++ b/board/LaCie/common/common.c
@@ -21,6 +21,12 @@ 
 #define MV88E1116_RGMII_TXTM_CTRL	(1 << 4)
 #define MV88E1116_RGMII_RXTM_CTRL	(1 << 5)
 
+#if !defined(MII_MARVELL_LED_REG)
+# define MII_MARVELL_LED_REG 16
+# define MII_MARVELL_LED_MASK 0xf0
+# define MII_MARVELL_LED_VALUE 0x0f
+#endif
+
 void mv_phy_88e1116_init(const char *name, u16 phyaddr)
 {
 	u16 reg;
@@ -53,9 +59,10 @@  void mv_phy_88e1318_init(const char *name, u16 phyaddr)
 	 * Set control mode 4 for LED[0].
 	 */
 	miiphy_write(name, phyaddr, MII_MARVELL_PHY_PAGE, 3);
-	miiphy_read(name, phyaddr, 16, &reg);
-	reg |= 0xf;
-	miiphy_write(name, phyaddr, 16, reg);
+	miiphy_read(name, phyaddr, MII_MARVELL_LED_REG, &reg);
+	reg &= MII_MARVELL_LED_MASK;
+	reg |= MII_MARVELL_LED_VALUE;
+	miiphy_write(name, phyaddr, MII_MARVELL_LED_REG, reg);
 
 	/*
 	 * Enable RGMII delay on Tx and Rx for CPU port
diff --git a/include/configs/lacie_kw.h b/include/configs/lacie_kw.h
index fe27bbe..02e0882 100644
--- a/include/configs/lacie_kw.h
+++ b/include/configs/lacie_kw.h
@@ -138,6 +138,11 @@ 
 #ifdef CONFIG_CMD_NET
 #define CONFIG_MVGBE_PORTS		{1, 0} /* enable port 0 only */
 #define CONFIG_NETCONSOLE
+#if defined(CONFIG_CLOUDBOX)
+# define MII_MARVELL_LED_REG 17
+# define MII_MARVELL_LED_MASK 0xffc0
+# define MII_MARVELL_LED_VALUE 0x15
+#endif
 #endif
 
 /*