Victor,
On Saturday 30 August 2008, Victor Gallardo wrote:
> This patch adds GPCS, SGMII and M88E1112 PHY support
> for the AMCC PPC460GT/EX processors.
Please find some comments below.
> Signed-off-by: Victor Gallardo <[EMAIL PROTECTED]>
> ---
> cpu/ppc4xx/4xx_enet.c | 116
> ++++++++++++++++++++++++++++++++++++++++++++++++- cpu/ppc4xx/miiphy.c |
> 40 ++++++++++++++++-
> include/ppc4xx_enet.h | 4 ++
> 3 files changed, 155 insertions(+), 5 deletions(-)
>
> diff --git a/cpu/ppc4xx/4xx_enet.c b/cpu/ppc4xx/4xx_enet.c
> index 8a38335..4729800 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,12 @@
> #define MAL_RX_CHAN_MUL 1
> #endif
>
> +#if !defined(CONFIG_PHY_LESS)
> +#define CONFIG_PHY_LESS 0xFFFFFFFF /* PHY-less mode */
> +#define CONFIG_PHY_LESS_SPEED 1000
> +#define CONFIG_PHY_LESS_DUPLEX FULL
> +#endif
Could you please explain this PHY_LESS mode and especially the CONFIG_PHY_LESS
define a little more. To what value will this be defined for Arches for
example?
> +
>
> /*-------------------------------------------------------------------------
>----+ * Global variables. TX and RX descriptors and buffers.
>
> *--------------------------------------------------------------------------
>---*/ @@ -611,8 +618,19 @@ int ppc_4xx_eth_setup_bridge(int devnum, bd_t *
> bis)
>
> #if defined(CONFIG_460EX)
> mode = 9;
> + mfsdr(SDR0_ETH_CFG, eth_cfg);
> + if (((eth_cfg & SDR0_ETH_CFG_SGMII0_ENABLE) > 0) &&
> + ((eth_cfg & SDR0_ETH_CFG_SGMII1_ENABLE) > 0)) {
> + mode = 11; /* config SGMII */
> +}
Make this a little simpler and drop the brackets (for single line statements):
if ((eth_cfg & SDR0_ETH_CFG_SGMII0_ENABLE) &&
(eth_cfg & SDR0_ETH_CFG_SGMII1_ENABLE))
mode = 11; /* config SGMII */
> #else
> mode = 10;
> + mfsdr(SDR0_ETH_CFG, eth_cfg);
> + if (((eth_cfg & SDR0_ETH_CFG_SGMII0_ENABLE) > 0) &&
> + ((eth_cfg & SDR0_ETH_CFG_SGMII1_ENABLE) > 0) &&
> + ((eth_cfg & SDR0_ETH_CFG_SGMII2_ENABLE) > 0)) {
> + mode = 12; /* config SGMII */
> +}
Same here please.
> #endif
>
> /* TODO:
> @@ -635,6 +653,8 @@ int ppc_4xx_eth_setup_bridge(int devnum, bd_t * bis)
> /*
> * Right now only 2*RGMII is supported. Please extend when needed.
> * sr - 2008-02-19
> + * Add SGMII support.
> + * vg - 2008-07-28
> */
> switch (mode) {
> case 1:
> @@ -761,6 +781,20 @@ int ppc_4xx_eth_setup_bridge(int devnum, bd_t * bis)
> bis->bi_phymode[2] = BI_PHYMODE_RGMII;
> bis->bi_phymode[3] = BI_PHYMODE_RGMII;
> break;
> + case 11:
> + /* 2 SGMII - 460EX */
> + bis->bi_phymode[0] = BI_PHYMODE_SGMII;
> + bis->bi_phymode[1] = BI_PHYMODE_SGMII;
> + bis->bi_phymode[2] = BI_PHYMODE_NONE;
> + bis->bi_phymode[3] = BI_PHYMODE_NONE;
> + break;
> + case 12:
> + /* 3 SGMII - 460GT */
> + bis->bi_phymode[0] = BI_PHYMODE_SGMII;
> + bis->bi_phymode[1] = BI_PHYMODE_SGMII;
> + bis->bi_phymode[2] = BI_PHYMODE_SGMII;
> + bis->bi_phymode[3] = BI_PHYMODE_NONE;
> + break;
> default:
> break;
> }
> @@ -945,6 +979,48 @@ static int ppc_4xx_eth_init (struct eth_device *dev,
> bd_t * bis) out_be32((void *)EMAC_M1 + hw_p->hw_addr, mode_reg);
> #endif /* defined(CONFIG_440GX) || defined(CONFIG_440SP) */
>
> +#if defined(CONFIG_GPCS_PHY_ADDR) || defined(CONFIG_GPCS_PHY1_ADDR) || \
> + defined(CONFIG_GPCS_PHY2_ADDR) || defined(CONFIG_GPCS_PHY3_ADDR)
> + if (bis->bi_phymode[devnum] == BI_PHYMODE_SGMII) {
> + /*
> + * In SGMII mode, GPCS access is needed for
> + * communication with the internal SGMII SerDes.
> + */
> + switch (devnum) {
> +#if defined(CONFIG_GPCS_PHY_ADDR)
> + case 0:
> + reg = CONFIG_GPCS_PHY_ADDR;
> + break;
> +#endif
> +#if defined(CONFIG_GPCS_PHY1_ADDR)
> + case 1:
> + reg = CONFIG_GPCS_PHY1_ADDR;
> + break;
> +#endif
> +#if defined(CONFIG_GPCS_PHY2_ADDR)
> + case 2:
> + reg = CONFIG_GPCS_PHY2_ADDR;
> + break;
> +#endif
> +#if defined(CONFIG_GPCS_PHY3_ADDR)
> + case 3:
> + reg = CONFIG_GPCS_PHY3_ADDR;
> + break;
> +#endif
> + }
> +
> + mode_reg = in_be32((void *)EMAC_M1 + hw_p->hw_addr);
> + mode_reg |= EMAC_M1_MF_1000GPCS | EMAC_M1_IPPA_SET(reg);
> + out_be32((void *)EMAC_M1 + hw_p->hw_addr, mode_reg);
> +
> + /* Configure the GPCS interface */
> + miiphy_reset(dev->name, reg);
> + miiphy_write(dev->name, reg, 0x04, 0x8120);
> + miiphy_write(dev->name, reg, 0x07, 0x2801);
> + miiphy_write(dev->name, reg, 0x00, 0x0140);
Please add some comments for those "magic number" above.
> + }
> +#endif /* defined(CONFIG_GPCS_PHY_ADDR) */
> +
> /* wait for PHY to complete auto negotiation */
> reg_short = 0;
> #ifndef CONFIG_CS8952_PHY
> @@ -974,6 +1050,9 @@ static int ppc_4xx_eth_init (struct eth_device *dev,
> bd_t * bis)
>
> bis->bi_phynum[devnum] = reg;
>
> + if (reg == CONFIG_PHY_LESS)
> + goto GET_SPEED;
Don't use uppercase names for labels.
> +
> #if defined(CONFIG_PHY_RESET)
> /*
> * Reset the phy, only if its the first time through
> @@ -986,6 +1065,33 @@ static int ppc_4xx_eth_init (struct eth_device *dev,
> bd_t * bis) miiphy_write (dev->name, reg, 0x09, 0x0e00);
> miiphy_write (dev->name, reg, 0x04, 0x01e1);
> #endif
> +#if defined(CONFIG_M88E1112_PHY)
> + if (bis->bi_phymode[devnum] == BI_PHYMODE_SGMII) {
> + /*
> + * Marvell 88E1112 PHY needs to have the SGMII MAC
> + * interace (page 2) properly configured to
> + * communicate with the 460EX/GT GPCS interface.
> + */
> +
> + /* Set access to Page 2 */
> + miiphy_write(dev->name, reg, 0x16, 0x0002);
> +
> + miiphy_write(dev->name, reg, 0x00, 0x0040); /* 1Gbps */
> + miiphy_read(dev->name, reg, 0x10, ®_short);
> + reg_short &= ~0x0C00; /* Preferred Media MASK */
> + reg_short |= 0x0800; /* Preferred Media Copper */
> + reg_short &= ~0x0380; /* Mode Select MASK */
> + reg_short |= 0x0280; /* Mode Select Copper only */
> + miiphy_write(dev->name, reg, 0x10, reg_short);
> + miiphy_read(dev->name, reg, 0x1a, ®_short);
> + reg_short |= 0x8000; /* bypass Auto-Negotiation */
> + miiphy_write(dev->name, reg, 0x1a, reg_short);
> + miiphy_reset(dev->name, reg); /* reset MAC interface */
> +
> + /* Reset access to Page 0 */
> + miiphy_write(dev->name, reg, 0x16, 0x0000);
> + }
> +#endif /* defined(CONFIG_M88E1112_PHY) */
> miiphy_reset (dev->name, reg);
>
> #if defined(CONFIG_440GX) || \
> @@ -1080,8 +1186,14 @@ static int ppc_4xx_eth_init (struct eth_device *dev,
> bd_t * bis) }
> #endif /* #ifndef CONFIG_CS8952_PHY */
>
> - speed = miiphy_speed (dev->name, reg);
> - duplex = miiphy_duplex (dev->name, reg);
> +GET_SPEED:
> + if (reg == CONFIG_PHY_LESS) {
> + speed = CONFIG_PHY_LESS_SPEED;
> + duplex = CONFIG_PHY_LESS_DUPLEX;
> + } else {
> + speed = miiphy_speed(dev->name, reg);
> + duplex = miiphy_duplex(dev->name, reg);
> + }
>
> if (hw_p->print_speed) {
> hw_p->print_speed = 0;
> diff --git a/cpu/ppc4xx/miiphy.c b/cpu/ppc4xx/miiphy.c
> index c882720..abc88f7 100644
> --- a/cpu/ppc4xx/miiphy.c
> +++ b/cpu/ppc4xx/miiphy.c
> @@ -180,8 +180,10 @@ int phy_setup_aneg (char *devname, unsigned char addr)
> *
> * sr: Currently on 460EX only EMAC0 works with MDIO, so we always
> * return EMAC0 offset here
> + * vg: For 460EX/460GT if internal GPCS PHY address is specified
> + * return appropriate EMAC offset
> */
> -unsigned int miiphy_getemac_offset (void)
> +unsigned int miiphy_getemac_offset(u8 addr)
> {
> #if (defined(CONFIG_440) && \
> !defined(CONFIG_440SP) && !defined(CONFIG_440SPE) && \
> @@ -233,6 +235,38 @@ unsigned int miiphy_getemac_offset (void)
> return 0x100;
> #endif
>
> +#if defined(CONFIG_460EX) || defined(CONFIG_460GT)
> + u32 mode_reg;
> + u32 eoffset = 0;
Nitpick: Please add empty line after variable declarations.
> + switch (addr) {
> +#if defined(CONFIG_HAS_ETH1) && defined(CONFIG_GPCS_PHY1_ADDR)
> + case CONFIG_GPCS_PHY1_ADDR:
> + mode_reg = in_be32((void *)EMAC_M1 + 0x100);
> + if (addr == EMAC_M1_IPPA_GET(mode_reg))
> + eoffset = 0x100;
> + break;
> +#endif
> +#if defined(CONFIG_HAS_ETH2) && defined(CONFIG_GPCS_PHY2_ADDR)
> + case CONFIG_GPCS_PHY2_ADDR:
> + mode_reg = in_be32((void *)EMAC_M1 + 0x300);
> + if (addr == EMAC_M1_IPPA_GET(mode_reg))
> + eoffset = 0x300;
> + break;
> +#endif
> +#if defined(CONFIG_HAS_ETH3) && defined(CONFIG_GPCS_PHY3_ADDR)
> + case CONFIG_GPCS_PHY3_ADDR:
> + mode_reg = in_be32((void *)EMAC_M1 + 0x400);
> + if (addr == EMAC_M1_IPPA_GET(mode_reg))
> + eoffset = 0x400;
> + break;
> +#endif
> + default:
> + eoffset = 0;
> + break;
> + }
> + return eoffset;
> +#endif
> +
> return 0;
> #endif
> }
> @@ -262,7 +296,7 @@ static int emac_miiphy_command(u8 addr, u8 reg, int
> cmd, u16 value) u32 emac_reg;
> u32 sta_reg;
>
> - emac_reg = miiphy_getemac_offset();
> + emac_reg = miiphy_getemac_offset(addr);
>
> /* wait for completion */
> if (emac_miiphy_wait(emac_reg) != 0)
> @@ -311,7 +345,7 @@ int emac4xx_miiphy_read (char *devname, unsigned char
> addr, unsigned char reg, unsigned long sta_reg;
> unsigned long emac_reg;
>
> - emac_reg = miiphy_getemac_offset ();
> + emac_reg = miiphy_getemac_offset(addr);
>
> if (emac_miiphy_command(addr, reg, EMAC_STACR_READ, 0) != 0)
> return -1;
> diff --git a/include/ppc4xx_enet.h b/include/ppc4xx_enet.h
> index b74c6fc..689b056 100644
> --- a/include/ppc4xx_enet.h
> +++ b/include/ppc4xx_enet.h
> @@ -376,6 +376,7 @@ typedef struct emac_4xx_hw_st {
> #define EMAC_M1_APP (0x08000000)
> #define EMAC_M1_RSVD (0x06000000)
> #define EMAC_M1_IST (0x01000000)
> +#define EMAC_M1_MF_1000GPCS (0x00C00000)
> #define EMAC_M1_MF_1000MBPS (0x00800000) /* 0's for 10MBPS */
> #define EMAC_M1_MF_100MBPS (0x00400000)
> #define EMAC_M1_RFS_MASK (0x00380000)
> @@ -394,6 +395,9 @@ typedef struct emac_4xx_hw_st {
> #define EMAC_M1_MWSW (0x00007000)
> #define EMAC_M1_JUMBO_ENABLE (0x00000800)
> #define EMAC_M1_IPPA (0x000007c0)
> +#define EMAC_M1_IPPA_SET(id) (((id) & 0x1f) << 6)
> +#define EMAC_M1_IPPA_GET(id) (((id) >> 6) & 0x1f)
> +
> #define EMAC_M1_OBCI_GT100 (0x00000020)
> #define EMAC_M1_OBCI_100 (0x00000018)
> #define EMAC_M1_OBCI_83 (0x00000010)
Please clean up and resubmit.
Thanks.
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
