[U-Boot] [PATCH 4/5] powerpc: keymile: Add a check for the PIGGY debug board

[Gary Jennejohn]

On Thu, 13 Nov 2008 09:30:51 -0800
Ben Warren <biggerbadderben at gmail.com> wrote:

> Heiko Schocher wrote:
> > Hello Ben
> >
> > Ben Warren wrote:
> >   
> >> Heiko Schocher wrote:
> >>   
> >>     
> >>> Check the presence of the PIGGY on the keymile boards mgcoge,
> >>> mgsuvd and kmeter1. If the PIGGY is not present, dont register
> >>> this Ethernet device.
> >>>
> >>> Signed-off-by: Heiko Schocher <hs at denx.de>
> >>> ---
> >>>     
> >>>       
> >> This looks like useful stuff to have, but I'd prefer that you put the 
> >> check logic in board_eth_init() rather than adding to the individual 
> >> device drivers.  I know the 8260 SCC driver is the older style, which 
> >> precludes the use of board_eth_init, but I'll convert it if you're able 
> >> to test.
> >>   

Unfortunately, this approach won't work.  First of all, the 82xx SCC
driver is now initialized in cpu_eth_init(), which knows nothing about
board-specific peculiarities like the PIGGY.  Secondly, the HDLC
driver for Keymile has to be initialized in board_eth_init(), and it
has nothing to do with the PIGGY.  Putting the check in board_eth_init()
would break it completely.  I looked at Heiko's latest patch and couldn't
figure out a way to cleanly differentiate between initializing the HDLC
driver and checking whether the PIGGY was present fo the other ENET drivers.

> >>     
> >
> > Yes, I could test such a change for you, but hmm... I am not sure, if
> > board_eth_init () is the right place for my purpose.
> > I need for every Ethernet device a selection, if this device is present or
> > not.
> > Correct me if I am wrong, but it looks like board_eth_init ()
> > is not made for this purpose. (Ok, I can do a specific device init
> > in board_eth_init (), but then we must do something, that prevents
> > that the device is again initialized in eth_initialize () ...
> >
> >   
> board_eth_init() was introduced for exactly this sort of thing.  Have a 
> look at the net repo (I've sent a pull request to Wolfgang so the 
> current changes will make it into the 12.2008 release).  There aren't 
> any device initializations left in eth_initialize(), so there's no issue 
> of a device being initialized twice.  The goal is for all devices to be 
> started by cpu_eth_int() or board_eth_init().
> > Hmm... while writing this it comes a idea in my mind:
> > we could move all the *_initialize functions in eth_initialize () in a
> > seperate function, say eth_hardware_init() and maybe making this
> > function "weak", so a board writer can write his own
> > eth_hardware_init() ... in such a function, I could check which
> > device is present, and only initialize the present devices ...
> > what do you think?
> >   
> That's what board_eth_init() and cpu_eth_init() are for.  In addition, I 
> forgot to mention that a couple of days ago Gary Jennejohn submitted a 
> patch for changing the 82xx SCC driver over to CONFIG_NET_MULTI style.
> 

Correct, and it must explicitly call ethernet_present() - see above.

---
Gary Jennejohn
*********************************************************************
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
*********************************************************************