On Wednesday 03 September 2008, Victor Gallardo wrote:
> This patch adds GPCS, SGMII and M88E1112 PHY support
> for the AMCC PPC460GT/EX processors.
>
> Signed-off-by: Victor Gallardo <[EMAIL PROTECTED]>
> ---
A good idea is to keep a history of what changed in the patch revisions here
in this area (after the "---"). Something like:
v2:
- Added comments to GPCS PHY setup
- Minor coding style cleanup
v3:
- Generalized the PHY-less configuration even more
Please find some more comments below.
> cpu/ppc4xx/4xx_enet.c | 162
> ++++++++++++++++++++++++++++++++++++++++++++++++- cpu/ppc4xx/miiphy.c |
> 41 ++++++++++++-
> include/ppc4xx_enet.h | 3 +
> 3 files changed, 201 insertions(+), 5 deletions(-)
>
> diff --git a/cpu/ppc4xx/4xx_enet.c b/cpu/ppc4xx/4xx_enet.c
> index 8a38335..e137bac 100644
> --- a/cpu/ppc4xx/4xx_enet.c
> +++ b/cpu/ppc4xx/4xx_enet.c
> @@ -198,6 +198,7 @@
> #define BI_PHYMODE_RMII 8
> #endif
> #endif
> +#define BI_PHYMODE_SGMII 9
>
> #if defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
> defined(CONFIG_440EPX) || defined(CONFIG_440GRX) || \
> @@ -216,6 +217,52 @@
> #define MAL_RX_CHAN_MUL 1
> #endif
>
> +/*--------------------------------------------------------------------+
> + * PHY-less support for Ethernet Ports.
> + *--------------------------------------------------------------------*/
> +
> +/*
> + * Some boards do not have a PHY for each ethernet port.
> + * For example on Arches board (2 CPU system) eth0 does not have
> + * a PHY, both CPU's are wired directly together (AC coupled)
> + * using SGMII0.
> + *
> + * In these cases :
> + * 1) set the appropriate CONFIG_PHY_ADDR equal to CONFIG_PHY_LESS
> + * to detect that the specified ethernet port does not have a PHY.
> + * 2) Then define CFG_PHY_LESS_PORT and CFG_PHY_LESS_PORTS in board
> + * configuration file. For example on the Arches board we would do
> + * the following.
> + * #define CFG_PHY_LESS_PORT(devnum,speed,duplex) \
> + * { devnum, speed, duplex},
> + * #define CFG_PHY_LESS_PORTS \
> + * CFG_PHY_LESS_PORT(0,1000,FULL)
> + */
> +#if !defined(CONFIG_PHY_LESS)
> +#define CONFIG_PHY_LESS 0xFFFFFFFF /* PHY-less mode */
> +#endif
If we agree that this is a good generic approach for this PHY-less handling,
then we should probably move this to a common header so that other ethernet
driver can use this too.
Ben, what do you think?
And the description should be moved to a common place too. Either the toplevel
README, or a new README.xxx in the doc directory.
> +
> +#define DFLT_PHY_LESS_SPEED 100
> +#define DFLT_PHY_LESS_DUPLEX FULL
Do we really need these defaults? Please see my comment further down.
> +
> +/*
> + * CFG_PHY_LESS_PORTS tells us about ethernet ports that have no PHY
> + * attached to them.
> + */
> +#ifndef CFG_PHY_LESS_PORTS
> +#define CFG_PHY_LESS_PORTS
> +#endif
> +
> +struct phy_less_port {
> + unsigned int devnum; /* ethernet port */
> + unsigned int speed; /* specified speed 10,100 or 1000 */
> + unsigned int duplex; /* specified duplex FULL or HALF */
> +};
Again, if we agree that this is a good "solution", then this should be moved
into a common header, probably net.h.
<snip>
> - speed = miiphy_speed (dev->name, reg);
> - duplex = miiphy_duplex (dev->name, reg);
> +get_speed:
> + if (reg == CONFIG_PHY_LESS) {
> + speed = DFLT_PHY_LESS_SPEED;
> + duplex = DFLT_PHY_LESS_DUPLEX;
I don't think we need this defaults here. Remove them and...
> + for (i = 0; i < ARRAY_SIZE(phy_less_port); i++) {
> + if (devnum == phy_less_port[i].devnum) {
> + speed = phy_less_port[i].speed;
> + duplex = phy_less_port[i].duplex;
> + break;
> + }
> + }
...add this here:
if (i == ARRAY_SIZE(phy_less_port)) {
printf("ERROR: PHY (%s) not configured correctly!\n",
dev->name);
return -1;
}
If the PHY-less device is not in the list, then this is a misconfiguration
which should be fixed IMHO.
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: [EMAIL PROTECTED]
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
