[U-Boot-Users] PATCH: support board-specific ethernet PHY init for ARM

[U-Boot-Users] PATCH: support board-specific ethernet PHY init for ARM

[Michael Schwingen]

Hi,

The following patch adds support for board-specific PHY initialization on
ARM targets (IXP42x in my case), similar to the PPC board code. 

I have one boards where the LED pins on the PHY need to be setup to function
properly, and one board where the ethernet switch starts up with all ports
disabled.

Signed-off-by: Michael Schwingen <michael@schwingen.org>

diff --git a/lib_arm/board.c b/lib_arm/board.c
index 7e97f13..c5a941d 100644
--- a/lib_arm/board.c
+++ b/lib_arm/board.c
@@ -430,6 +430,10 @@ extern void dm644x_eth_set_mac_addr (const u_int8_t *addr);
 	puts ("Net:   ");
 #endif
 	eth_initialize(gd->bd);
+#if defined(CONFIG_RESET_PHY_R)
+	debug ("Reset Ethernet PHY\n");
+	reset_phy();
+#endif
 #endif
 	/* main_loop() can return to retry autoboot, if so just run it again. */
 	for (;;) {

[U-Boot-Users] PATCH: support board-specific ethernet PHY init for ARM

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:29 Sat 10 Nov     , Michael Schwingen wrote:
> Hi,
> 
> The following patch adds support for board-specific PHY initialization on
> ARM targets (IXP42x in my case), similar to the PPC board code. 
> 
> I have one boards where the LED pins on the PHY need to be setup to function
> properly, and one board where the ethernet switch starts up with all ports
> disabled.
> 
> Signed-off-by: Michael Schwingen <michael@schwingen.org>
> 
> diff --git a/lib_arm/board.c b/lib_arm/board.c
> index 7e97f13..c5a941d 100644
> --- a/lib_arm/board.c
> +++ b/lib_arm/board.c
> @@ -430,6 +430,10 @@ extern void dm644x_eth_set_mac_addr (const u_int8_t *addr);
>  	puts ("Net:   ");
>  #endif
>  	eth_initialize(gd->bd);
> +#if defined(CONFIG_RESET_PHY_R)
> +	debug ("Reset Ethernet PHY\n");
> +	reset_phy();
> +#endif
>  #endif
>  	/* main_loop() can return to retry autoboot, if so just run it again. */
>  	for (;;) {

	The phy reset will be call in the mac layer drivers not in the board
	part.

Best Regards
J.

[U-Boot-Users] PATCH: support board-specific ethernet PHY init for ARM

[Michael Schwingen]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> 	The phy reset will be call in the mac layer drivers not in the board
> 	part.
>   
It seems I am a bit lost in the network code.
As I see it, I have to place board-specific PHY reset code after the 
call to eth_initialize in board.c, but before main_loop. If there is 
already such a hook in generic code, could you please tell me where?

cu
Michael

[U-Boot-Users] PATCH: support board-specific ethernet PHY init for ARM

[Jean-Christophe PLAGNIOL-VILLARD]

On 23:32 Sat 08 Dec     , Michael Schwingen wrote:
> Jean-Christophe PLAGNIOL-VILLARD wrote:
> > 	The phy reset will be call in the mac layer drivers not in the board
> > 	part.
> >   
> It seems I am a bit lost in the network code.
> As I see it, I have to place board-specific PHY reset code after the 
> call to eth_initialize in board.c, but before main_loop. If there is 
> already such a hook in generic code, could you please tell me where?

I'm currently adding the last_stage_init support for IXP, you could add
you're phy_reset until the new phylib arrive.

The phylib may take care of phy_reset.

Best Regards,
J.-

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/lib_arm/board.c b/lib_arm/board.c
index 6f35aa0..0eadc3a 100644
--- a/lib_arm/board.c
+++ b/lib_arm/board.c
@@ -431,6 +431,14 @@ extern void dm644x_eth_set_mac_addr (const u_int8_t *addr);
 #endif
 	eth_initialize(gd->bd);
 #endif
+#ifdef CONFIG_LAST_STAGE_INIT
+	/*
+	 * Some parts can be only initialized if all others (like
+	 * Interrupts) are up and running (i.e. the PC-style ISA
+	 * keyboard).
+	 */
+	last_stage_init ();
+#endif
 	/* main_loop() can return to retry autoboot, if so just run it again. */
 	for (;;) {
 		main_loop ();
-- 
1.5.3.7

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Wolfgang Denk]

In message <1197193806-26116-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> diff --git a/lib_arm/board.c b/lib_arm/board.c
> index 6f35aa0..0eadc3a 100644
> --- a/lib_arm/board.c
> +++ b/lib_arm/board.c
> @@ -431,6 +431,14 @@ extern void dm644x_eth_set_mac_addr (const u_int8_t *addr);
>  #endif
>  	eth_initialize(gd->bd);
>  #endif
> +#ifdef CONFIG_LAST_STAGE_INIT
> +	/*
> +	 * Some parts can be only initialized if all others (like
> +	 * Interrupts) are up and running (i.e. the PC-style ISA
> +	 * keyboard).
> +	 */
> +	last_stage_init ();
> +#endif
>  	/* main_loop() can return to retry autoboot, if so just run it again. */
>  	for (;;) {
>  		main_loop ();
> -- 

Please, not yet another such thing.

Is there any specific reason why you cannot use any of the existing
entry points, like for example board_late_init() ?

Also, if this is really, really necessary (which I seriously doubt)
please use a weak function declaration instead of an #ifdef.

We want to get rid of this #ifdef mess...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Eureka!                                                 -- Archimedes

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Michael Schwingen]

Wolfgang Denk wrote:
>>  #endif
>>  	eth_initialize(gd->bd);
>>  #endif
>> +#ifdef CONFIG_LAST_STAGE_INIT
>> +	/*
>> +	 * Some parts can be only initialized if all others (like
>> +	 * Interrupts) are up and running (i.e. the PC-style ISA
>> +	 * keyboard).
>> +	 */
>> +	last_stage_init ();
>> +#endif
> Please, not yet another such thing.
>
> Is there any specific reason why you cannot use any of the existing
> entry points, like for example board_late_init() ?
>   
In my case, I need board-specific init code for the ethernet 
PHY/Switches, which needs to run after eth_inizialize, so 
board_late_init (at least on arm) is still too early.

If we could move board_late_init after eth_initialize (ie. directly 
before calling main_loop), that would work in my case, and would remove 
the necessity for my reset_phy patch.

cu
Michael

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Haavard Skinnemoen]

On Sun, 09 Dec 2007 17:38:13 +0100
Michael Schwingen <rincewind@discworld.dascon.de> wrote:

> If we could move board_late_init after eth_initialize (ie. directly 
> before calling main_loop), that would work in my case, and would remove 
> the necessity for my reset_phy patch.

Or even better, move eth_initialize into the board code. That would
allow us to kill a ridiculous number of #ifdefs in net/eth.c and would
allow the board to do any fixups it needs before and after the generic
code.

Haavard

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Michael Schwingen]

Haavard Skinnemoen wrote:
> Or even better, move eth_initialize into the board code. That would
> allow us to kill a ridiculous number of #ifdefs in net/eth.c and would
> allow the board to do any fixups it needs before and after the generic
> code.
>   
Hm - yes, looking at eth_initialize, that does sound like a good idea, 
although some common init code and the code handling ethact / ethprime 
(what is that good for, anyway?) would need to be factored out.

cu
Michael

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Ben Warren]

Haavard Skinnemoen wrote:
> On Sun, 09 Dec 2007 17:38:13 +0100
> Michael Schwingen <rincewind@discworld.dascon.de> wrote:
>
>   
>> If we could move board_late_init after eth_initialize (ie. directly 
>> before calling main_loop), that would work in my case, and would remove 
>> the necessity for my reset_phy patch.
>>     
>
> Or even better, move eth_initialize into the board code. That would
> allow us to kill a ridiculous number of #ifdefs in net/eth.c and would
> allow the board to do any fixups it needs before and after the generic
> code.
>
>   
eth_initialize() is definitely a mess, but I don't think moving it to 
board code is the right thing to do. On the other hand, one thing I've 
been thinking about was a new eth_initialze() that steps through an 
array of board-defined controller configurations. This way we could 
specify all sorts of information about controller type, PHY type, 
addresses etc. on a per-controller basis, and of course provide default 
macros to abstract away the mundane.

More to the problem seen here - IMHO, a MAC's initialize() function 
shouldn't do anything other than populate and register one or more 
ethernet devices with the network subsystem. Any PHY initialization 
stuff should happen in the specific driver's init() function, which only 
gets called when the port is used. If you want to do initialization of 
any aspect of a controller that isn't used by u-boot, it should be in 
board-specific init code, and there should be a board-instantiable 
global function that is guaranteed to be after all the common code 
(immediately before the main loop begins). I haven't looked at the code 
in a while, but I gather one of the problems here was that 
board_late_init() (or whatever it's called) comes before 
eth_initialize(). If that's really the case, we need a 
board_really_late_init().

Thoughts?

regards,
Ben

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Haavard Skinnemoen]

On Mon, 10 Dec 2007 15:28:40 -0500
Ben Warren <bwarren@qstreams.com> wrote:

> Haavard Skinnemoen wrote:
> > On Sun, 09 Dec 2007 17:38:13 +0100
> > Michael Schwingen <rincewind@discworld.dascon.de> wrote:
> >
> >   
> >> If we could move board_late_init after eth_initialize (ie. directly 
> >> before calling main_loop), that would work in my case, and would remove 
> >> the necessity for my reset_phy patch.
> >>     
> >
> > Or even better, move eth_initialize into the board code. That would
> > allow us to kill a ridiculous number of #ifdefs in net/eth.c and would
> > allow the board to do any fixups it needs before and after the generic
> > code.

To clarify: I don't think we should move all of eth_initialize into
board code, but I think the whole initialization process should
originate from board code; for example introduce a new function
board_eth_init() which calls eth_initialize() after initializing the
ethernet controllers. Then we can remove all the calls to
board-specific functions from the beginning of eth_initialize().

> eth_initialize() is definitely a mess, but I don't think moving it to 
> board code is the right thing to do. On the other hand, one thing I've 
> been thinking about was a new eth_initialze() that steps through an 
> array of board-defined controller configurations. This way we could 
> specify all sorts of information about controller type, PHY type, 
> addresses etc. on a per-controller basis, and of course provide default 
> macros to abstract away the mundane.

Yeah, that might make sense too. But then again, perhaps we should go
all the way and put all driver initcalls in an array and initialize all
of them from one common do_initcalls() function like Linux does.

> More to the problem seen here - IMHO, a MAC's initialize() function 
> shouldn't do anything other than populate and register one or more 
> ethernet devices with the network subsystem. Any PHY initialization 
> stuff should happen in the specific driver's init() function, which only 
> gets called when the port is used.

It should probably initialize the PHY interface (i.e. the MDIO
interface) early, but I agree that it should delay initializing the
actual PHY until it is needed.

> If you want to do initialization of 
> any aspect of a controller that isn't used by u-boot, it should be in 
> board-specific init code, and there should be a board-instantiable 
> global function that is guaranteed to be after all the common code 
> (immediately before the main loop begins). I haven't looked at the code 
> in a while, but I gather one of the problems here was that 
> board_late_init() (or whatever it's called) comes before 
> eth_initialize(). If that's really the case, we need a 
> board_really_late_init().

Or a board_eth_init(), which will simply call the generic
eth_initialize() and do any remaining fixups that depend on ethernet
being up and running after that. The advantage is that we won't need
yet another initcall in the arch code, we just need to replace one of
them.

Haavard

[U-Boot-Users] [PATCH] ARM: support board-specific ethernet PHY init

[Jean-Christophe PLAGNIOL-VILLARD]

Add until the new phylib will be arrived

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/lib_arm/board.c b/lib_arm/board.c
index 6f35aa0..7e7a282 100644
--- a/lib_arm/board.c
+++ b/lib_arm/board.c
@@ -430,6 +430,10 @@ extern void dm644x_eth_set_mac_addr (const u_int8_t *addr);
 	puts ("Net:   ");
 #endif
 	eth_initialize(gd->bd);
+#if defined(CONFIG_RESET_PHY_R)
+	debug ("Reset Ethernet PHY\n");
+	reset_phy();
+#endif
 #endif
 	/* main_loop() can return to retry autoboot, if so just run it again. */
 	for (;;) {
-- 
1.5.3.7

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Wolfgang Denk]

In message <20071211095906.77701bb0@dhcp-252-066.norway.atmel.com> you wrote:
>
> It should probably initialize the PHY interface (i.e. the MDIO
> interface) early, but I agree that it should delay initializing the
> actual PHY until it is needed.

Please keep in mind that U-Boot shall initialize hardware if and only
if it is using it (and that it must deactivate it after use).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Programmer's Lament: (Shakespeare, Macbeth, Act I, Scene vii)
        "That we but teach bloody instructions,
        which, being taught, return to plague the inventor..."

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Mike Frysinger]

On Tuesday 11 December 2007, Wolfgang Denk wrote:
> In message <20071211095906.77701bb0@dhcp-252-066.norway.atmel.com> you 
wrote:
> > It should probably initialize the PHY interface (i.e. the MDIO
> > interface) early, but I agree that it should delay initializing the
> > actual PHY until it is needed.
>
> Please keep in mind that U-Boot shall initialize hardware if and only
> if it is using it (and that it must deactivate it after use).

with many pieces of hardware, you can draw a line between initializing it and 
actually enabling/using it.

for example, shouldnt the initialization/programming of the MAC address always 
happen regardless of the network being used ?  not all MAC's have/support 
eeprom's which contain the MAC address, so the implicit address programming 
that happens in hardware cannot happen.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20071211/e4f3f569/attachment.pgp 

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Michael Schwingen]

Haavard Skinnemoen wrote:
> It should probably initialize the PHY interface (i.e. the MDIO
> interface) early, but I agree that it should delay initializing the
> actual PHY until it is needed.
>   
That may depend on board/PHY: on some PHYs, the LED pin functions need 
to be set in software, so on power-up the ethernet LEDs may light up and 
display bogus states, so I would prefer to configure that early, even if 
the port is not used.

cu
Michael

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Wolfgang Denk]

In message <200712111205.48970.vapier@gentoo.org> you wrote:
>
> > Please keep in mind that U-Boot shall initialize hardware if and only
> > if it is using it (and that it must deactivate it after use).
> 
> with many pieces of hardware, you can draw a line between initializing it and 
> actually enabling/using it.

Correct. If make such a difference, then initialization shall be
performed when you enable it for the very first time in U-Boot.

There is many reasons for such a design; for example, think about the
system boot time.

> for example, shouldnt the initialization/programming of the MAC address always 
> happen regardless of the network being used ?  not all MAC's have/support 

Not, it should not. It must not.

> eeprom's which contain the MAC address, so the implicit address programming
> that happens in hardware cannot happen.

The MAC is not needed before you access the Ethernet  port,  so  ther
eis  no need to do it before - or to ever do it, if you don't use the
network in U-Boot.

Please see also the FAQ. 

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
My play was a complete success.  The audience was a failure.

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Wolfgang Denk]

In message <475ED93E.2070407@discworld.dascon.de> you wrote:
> Haavard Skinnemoen wrote:
> > It should probably initialize the PHY interface (i.e. the MDIO
> > interface) early, but I agree that it should delay initializing the
> > actual PHY until it is needed.
> >   
> That may depend on board/PHY: on some PHYs, the LED pin functions need 
> to be set in software, so on power-up the ethernet LEDs may light up and 
> display bogus states, so I would prefer to configure that early, even if 
> the port is not used.

I think that things like this should be done in board specific init
code, then.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Is not that the nature of men and women -- that the  pleasure  is  in
the learning of each other?
	-- Natira, the High Priestess of Yonada, "For the World is
	   Hollow and I Have Touched the Sky", stardate 5476.3.

[U-Boot-Users] [PATCH] ARM: add support of CONFIG_LAST_STAGE_INIT for arm

[Michael Schwingen]

Wolfgang Denk wrote:
>> That may depend on board/PHY: on some PHYs, the LED pin functions need 
>> to be set in software, so on power-up the ethernet LEDs may light up and 
>> display bogus states, so I would prefer to configure that early, even if 
>> the port is not used.
>>     
>
> I think that things like this should be done in board specific init
> code, then.
>   
I fully agree - after all, this is definitely very board-specific.

cu
Michael