Thanks a lot Stefan for your review feedback... You are thinking more similar to Linux Implementation :-)
I will try to address your suggestions, it provides more flexibility and readability. On the other hand I am currently putting my efforts to reduce patch size cutting some features. I will do it step-by-step. Regards.. Prafulla . . . > -----Original Message----- > From: Stefan Roese [mailto:s...@denx.de] > Sent: Tuesday, May 26, 2009 8:26 PM > To: u-boot@lists.denx.de > Cc: Prafulla Wadaskar; Manas Saksena; Ronen Shitrit; Nicolas > Pitre; Ashish Karkare; Prabhanjan Sarnaik; Lennert Buijtenhek > Subject: Re: [U-Boot] [PATCH v4] Gbe Controller driver > support for kirkwood SOCs > > Hi Prafulla, > > On Thursday 21 May 2009 22:24:32 Prafulla Wadaskar wrote: > > Contributors: > > Yotam Admon <yo...@marvell.com> > > Michael Blostein <michae...@marvell.com > > > > Reviewed by: Ronen Shitrit <rshit...@marvell.com> > > Signed-off-by: Prafulla Wadaskar <prafu...@marvell.com> > > --- > > Change log: > > v2: entire rewrite/restructure of v1 > > used small names for variable/function names readl/writel used to > > access SoC registers Soc registers accssed using pointers > through net > > device struct miiphy registration done for external smi read/write > > access miiphy_link used to detect phy link presence cleaned for > > cosmetic changes > > Please find a comment below. > > > diff --git a/drivers/net/kirkwood_egiga.h > > b/drivers/net/kirkwood_egiga.h new file mode 100644 index > > 0000000..9f3908e > > --- /dev/null > > +++ b/drivers/net/kirkwood_egiga.h > > @@ -0,0 +1,828 @@ > > +/* > > + * (C) Copyright 2009 > > + * Marvell Semiconductor <www.marvell.com> > > + * Prafulla Wadaskar <prafu...@marvell.com> > > + * > > + * based on - Driver for MV64360X ethernet ports > > + * Copyright (C) 2002 rab...@galileo.co.il > > + * > > + * See file CREDITS for list of people who contributed to this > > + * project. > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License as > > + * published by the Free Software Foundation; either version 2 of > > + * the License, or (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General > Public License > > + * along with this program; if not, write to the Free Software > > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, > > + * MA 02110-1301 USA > > + */ > > + > > +#ifndef __EGIGA_H__ > > +#define __EGIGA_H__ > > + > > +#define MAX_KWGBE_DEVS 2 /*controller has two ports */ > > <snip> > > > +/* Default port serial control value */ > > +#define PORT_SERIAL_CONTROL_VALUE_TMP \ > > + KWGBE_FORCE_LINK_PASS | \ > > + KWGBE_DIS_AUTO_NEG_FOR_DUPLX | \ > > + KWGBE_DIS_AUTO_NEG_FOR_FLOW_CTRL | \ > > + KWGBE_ADV_NO_FLOW_CTRL | \ > > + KWGBE_FORCE_FC_MODE_NO_PAUSE_DIS_TX | \ > > + KWGBE_FORCE_BP_MODE_NO_JAM | \ > > + (1 << 9) | \ > > + KWGBE_DO_NOT_FORCE_LINK_FAIL | \ > > + KWGBE_DIS_AUTO_NEG_SPEED_GMII | \ > > You disable speed auto-negotiation here. Why are you doing > this? Using auto- negotiation should be the default operation > mode from my point of view. > > > + KWGBE_DTE_ADV_0 | \ > > + KWGBE_MIIPHY_MAC_MODE | \ > > + KWGBE_AUTO_NEG_NO_CHANGE | \ > > + KWGBE_MAX_RX_PACKET_1552BYTE | \ > > + KWGBE_CLR_EXT_LOOPBACK | \ > > + KWGBE_SET_FULL_DUPLEX_MODE | \ > > + KWGBE_DIS_FLOW_CTRL_TX_RX_IN_FULL_DUPLEX | \ > > + KWGBE_SET_MII_SPEED_TO_100 > > + > > +#ifdef CONFIG_SYS_MII_MODE > > +#define PORT_SERIAL_CONTROL_VALUE \ > > + PORT_SERIAL_CONTROL_VALUE_TMP | \ > > + KWGBE_SET_GMII_SPEED_TO_10_100 > > +#else > > +#define PORT_SERIAL_CONTROL_VALUE \ > > + PORT_SERIAL_CONTROL_VALUE_TMP | \ > > + KWGBE_SET_GMII_SPEED_TO_1000 > > +#endif > > If you need a fixed link speed configuration we should > perhaps add a configuration option for this. What do you think? > > Best regards, > Stefan > > ===================================================================== > 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: > off...@denx.de > ===================================================================== > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
