Hi Ben,
Ben Warren <[EMAIL PROTECTED]> wrote:
> Gary Jennejohn wrote:
[snip]
> > #if defined(CONFIG_ETHER_ON_SCC) && defined(CONFIG_CMD_NET)
> >
> While you're mucking around with this file, please settle on a single
> CONFIG that can allow conditional compilation from the Makefile, then
> get rid of this stuff.
>
You mean get rid of CONFIG_ETHER_ON_SCC and CONFIG_CMD_NET? But isn't at
least CONFIG_CMD_NET required to get networking support in other parts of
U-Boot, which would make it a prerequisite for compiling this?
And eliminating or supplementing CONFIG_ETHER_ON_SCC with a new CONFIG
would mean changing a whole slew of configuration files, not to mention
include/net.h.
> > -int eth_send(volatile void *packet, int length)
> > +int sec_send(struct eth_device *dev, volatile void *packet, int length)
> >
> Please give all these functions, except initialize(), file scope (i.e.
> make them static). I'm not crazy about the name 'sec', but if it's
> static the objection doesn't carry much weight. I also can't think of a
> better name.
>
Yeah, I should have thought of this when I did the mods.
[snip]
> > +int sec_initialize(bd_t *bis)
> >
> For this function with global namespace, please pick a more descriptive
> name. Maybe 82xx_scc_initialize() or something?
>
I called it 82xx_scc_enet_initialize() to make its function clear.
> > --- a/net/eth.c
> > +++ b/net/eth.c
> > @@ -48,6 +48,8 @@ extern int ppc_4xx_eth_initialize(bd_t *);
> > extern int scc_initialize(bd_t*);
> > extern int npe_initialize(bd_t *);
> > extern int uec_initialize(int);
> > +extern int sec_initialize(bd_t *);
> > +extern int keymile_hdlc_enet_initialize(bd_t *);
> >
> > #ifdef CONFIG_API
> > extern void (*push_packet)(volatile void *, int);
> > @@ -196,6 +198,12 @@ int eth_initialize(bd_t *bis)
> > #if defined(CONFIG_IXP4XX_NPE)
> > npe_initialize(bis);
> > #endif
> > +#if defined(CONFIG_ETHER_ON_SCC) && defined(CONFIG_MPC8260)
> > + sec_initialize(bis);
> > +#endif
> > +#if defined(CONFIG_KEYMILE_HDLC_ENET)
> > + keymile_hdlc_enet_initialize(bis);
> > +#endif
> > if (!eth_devices) {
> > puts ("No ethernet found.\n");
> > show_boot_progress (-64);
> >
> Please don't add anything to this file. All initializations now go in
> cpu_eth_init()/board_eth_init(). There are plenty of examples you can
> draw from. Don't forget to add your initializer function to
> include/netdev.h
>
OK, that should be easy enough. I've now done it for keymile.
> You get bonus points if you move this driver to drivers/net
>
I'll look into it but make no promises.
---
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: [EMAIL PROTECTED]
*********************************************************************
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
