[U-Boot] [PATCH] Support multiple SGMII/TBI interfaces for TSEC ethernet

The original code only supported using 1 TSEC port in SGMII mode using an internal TBI PHY. Additionally, the TBI internal PHY was being accessed at the same register offset as the external PHY for the given TSEC port. This hardwiring of the TBI PHY register address based on external PHY address will break in many hardware configurations.
Signed-off-by: Peter Tyser ptyser@xes-inc.com ---
The above limitations were noticed on an mpc8572-based board with 2 sgmii interfaces with external PHYs. Both external PHYs are connected to the MDIO interface of TSEC1 which caused TSEC2 to be non functional since writes to TSEC2's TBI PHY were actually going to TSEC1's TBI PHY.
Due to a hardware limitations I've only been able to test TSEC2's functionality for what its worth.
drivers/net/tsec.c | 15 +++++++++------ include/tsec.h | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c index f81211a..38d4214 100644 --- a/drivers/net/tsec.c +++ b/drivers/net/tsec.c @@ -283,11 +283,13 @@ uint tsec_local_mdio_read(volatile tsec_t *phyregs, uint phyid, uint regnum) /* Configure the TBI for SGMII operation */ static void tsec_configure_serdes(struct tsec_private *priv) { - tsec_local_mdio_write(priv->phyregs, CFG_TBIPA_VALUE, TBI_ANA, + /* Access TBI PHY registers at given TSEC register offset as opposed to the + * offset used for external PHY/MII accesses */ + tsec_local_mdio_write(priv->regs, priv->regs->tbipa, TBI_ANA, TBIANA_SETTINGS); - tsec_local_mdio_write(priv->phyregs, CFG_TBIPA_VALUE, TBI_TBICON, + tsec_local_mdio_write(priv->regs, priv->regs->tbipa, TBI_TBICON, TBICON_CLK_SELECT); - tsec_local_mdio_write(priv->phyregs, CFG_TBIPA_VALUE, TBI_CR, + tsec_local_mdio_write(priv->regs, priv->regs->tbipa, TBI_CR, TBICR_SETTINGS); }
@@ -301,11 +303,12 @@ static int init_phy(struct eth_device *dev) struct phy_info *curphy; volatile tsec_t *phyregs = priv->phyregs; volatile tsec_t *regs = priv->regs; + static int tbi_offset = 0;
- /* Assign a Physical address to the TBI */ - regs->tbipa = CFG_TBIPA_VALUE; - phyregs->tbipa = CFG_TBIPA_VALUE; + /* Assign a unique physical address to the internal TBI PHY */ + regs->tbipa = CFG_TBIPA_VALUE + tbi_offset; asm("sync"); + tbi_offset++;
/* Reset MII (due to new addresses) */ priv->phyregs->miimcfg = MIIMCFG_RESET; diff --git a/include/tsec.h b/include/tsec.h index 2db4deb..69f068e 100644 --- a/include/tsec.h +++ b/include/tsec.h @@ -110,7 +110,7 @@ #define miim_read -1
#ifndef CFG_TBIPA_VALUE - #define CFG_TBIPA_VALUE 0x1f + #define CFG_TBIPA_VALUE 0x10 #endif #define MIIMCFG_INIT_VALUE 0x00000003 #define MIIMCFG_RESET 0x80000000

On Mon, Sep 8, 2008 at 6:39 PM, Peter Tyser ptyser@xes-inc.com wrote:
The original code only supported using 1 TSEC port in SGMII mode using an internal TBI PHY. Additionally, the TBI internal PHY was being accessed at the same register offset as the external PHY for the given TSEC port. This hardwiring of the TBI PHY register address based on external PHY address will break in many hardware configurations.
Hm. This doesn't sound quite right. I agree with the first part of the patch (I was sure I had tested more than TSEC1, but it doesn't look like I did it right). However, I don't understand what you mean by the TBI internal PHY being accessed at the same address as the external PHY. It's assigned to the value of CFG_TBIPA_VALUE. It's ok for all of the controllers to use the same value. If the value conflicts with that of your external PHY, change CFG_TBIPA_VALUE in your board's config file. I'm not really interested in allocating PHY address space for the TBI PHYs. It will make things more difficult in the future, since it effectively halves the number of available addresses.
Andy

Hi Andy,
On Mon, 2008-09-08 at 19:34 -0500, Andy Fleming wrote:
On Mon, Sep 8, 2008 at 6:39 PM, Peter Tyser ptyser@xes-inc.com wrote:
The original code only supported using 1 TSEC port in SGMII mode using an internal TBI PHY. Additionally, the TBI internal PHY was being accessed at the same register offset as the external PHY for the given TSEC port. This hardwiring of the TBI PHY register address based on external PHY address will break in many hardware configurations.
Hm. This doesn't sound quite right. I agree with the first part of the patch (I was sure I had tested more than TSEC1, but it doesn't look like I did it right). However, I don't understand what you mean by the TBI internal PHY being accessed at the same address as the external PHY.
When I was referring to the "register offset" in "the TBI internal PHY was being accessed at the same register offset as the external PHY for the given TSEC port", I was talking about the address the CPU read/writes from to generate an access to the external or TBI PHY.
For example, if a board had 4 sgmii interfaces with 4 external PHYs on an mpc8572 - all 4 PHYs could be physically connected to the MDIO bus of TSEC1. Thus to access the external PHYs, the CPU would have to access the the 85xx's registers at offset 0x24000 of the IMMR.
My understanding is that to access the TBI PHYs, you'd have to use the following offsets into the IMMR: TSEC1 - 0x24000 TSEC2 - 0x25000 TSEC3 - 0x26000 ...
So to configure TSEC2's PHYs in this configuration (my configuration by chance:), you'd have to read/write from TSEC1 @ 0x24000 to access the external PHY, but TSEC2 @ 0x25000 to access the TBI PHY. This was my understanding after reading the manual, and believe this is necessary after debugging the driver on my hardware.
The original code was accessing the TBI PHY via the same register offset in the 8572 as was used to access the external PHY (eg TSEC1 @ 0x24000 in this example), which didn't appear to work.
It's assigned to the value of CFG_TBIPA_VALUE. It's ok for all of the controllers to use the same value. If the value conflicts with that of your external PHY, change CFG_TBIPA_VALUE in your board's config file. I'm not really interested in allocating PHY address space for the TBI PHYs. It will make things more difficult in the future, since it effectively halves the number of available addresses.
I see, the possibility of having the same PHY address didn't cross my mind:) How would the "mii" commands treat having multiple PHYs at the same address, but on different buses?
I can try giving all TBI PHYs the address of CFG_TBIPA_VALUE tomorrow and make sure it works. Assuming it does, I'll resubmit, keeping the 1st part the same while changing the 2nd section of the patch to:
/* Assign a Physical address to the TBI */ regs->tbipa = CFG_TBIPA_VALUE; - phyregs->tbipa = CFG_TBIPA_VALUE; asm("sync");
Let me know if I'm missing something...
Thanks, Peter
participants (2)
-
Andy Fleming
-
Peter Tyser