[U-Boot-Users] TSEC Ethernet driver patch - RFC

michael.firth at bt.com
2008-01-09T20:26:24Z

> -----Original Message-----
> From: Ben Warren [mailto:biggerbadderben at gmail.com]
> Sent: 08 January 2008 16:42
> To: Firth,MJC,Michael,DMM R
> Cc: u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] Possible TSEC Ethernet driver patch
>
> michael.firth at bt.com wrote:
> > While debugging a board recently I found that the MDIO
> (mii) command
> > support in the TSEC Ethernet driver is somewhat unhelpful.
> >
> > Currently, even though there is a comment in the code that
> "For now,
> > only TSEC1 (index 0) has access to the PHYs, so all of the entries
> > have '0'", all MDIO commands are processed by searching for a TSEC
> > instance that has the requested MDIO address associated
> with it, and
> > then using that instance to run the command, even though,
> because of
> > the aforementioned comment, all instances process MDIO commands
> > through the same port.
> >
> > This means that it is only possible to communicate with
> MDIO addresses
> > that have a TSEC instance associated with them, even though the
> > hardware is capable of accessing any address. It also means
> that there
> > is a list search that isn't needed.
> >
> > I have patched the 1.3.1 U-Boot code to remove this search, and to
> > interrogate the requested PHY directly. This means that it
> is possible
> > to directly access all 32 PHY addresses.
> >
> > Is this a change that would be more generally useful to the U-Boot
> > community, and, if so, how should I submit a more general patch for
> > this?
> >
> >
> Why don't you post what you have, clearly label it as 'RFC'
> and we'll have a look. In my spare time (very spare indeed)
> I'm trying to decouple PHYs from MACs, but time is hard to
> find and meanwhile things need to work.
>
> regards,
> Ben
>
Patch below.

The main area I wasn't sure on how to handle was how to replace the
other calls to 'read_phy_reg' and 'write_phy_reg' in the code. Currently
I've used a #define to map these on to the modified functions that take
the phy address as a parameter.

--- u-boot-1.3.1-orig/drivers/net/tsec.c 2007-12-06
09:21:19.000000000 +0000
+++ u-boot-1.3.1/drivers/net/tsec.c 2008-01-09 20:19:36.000000000
+0000
@@ -241,10 +244,9 @@
* It will wait for the write to be done (or for a timeout to
* expire) before exiting
*/
-void write_phy_reg(struct tsec_private *priv, uint regnum, uint value)
+void write_any_phy_reg(struct tsec_private *priv, uint phyid, uint
regnum, uint value)
{
volatile tsec_t *regbase = priv->phyregs;
- uint phyid = priv->phyaddr;
int timeout = 1000000;

regbase->miimadd = (phyid << 8) | regnum;
@@ -255,17 +257,19 @@
while ((regbase->miimind & MIIMIND_BUSY) && timeout--) ;
}

+/* #define to provide old write_phy_reg functionality without
duplicating code */
+#define write_phy_reg(priv, regnum, value)
write_any_phy_reg(priv,priv->phyaddr,regnum,value)
+
/* Reads register regnum on the device's PHY through the
* registers specified in priv. It lowers and raises the read
* command, and waits for the data to become valid (miimind
* notvalid bit cleared), and the bus to cease activity (miimind
* busy bit cleared), and then returns the value
*/
-uint read_phy_reg(struct tsec_private *priv, uint regnum)
+uint read_any_phy_reg(struct tsec_private *priv, uint phyid, uint
regnum)
{
uint value;
volatile tsec_t *regbase = priv->phyregs;
- uint phyid = priv->phyaddr;

/* Put the address of the phy, and the register
* number into MIIMADD */
@@ -288,6 +292,9 @@
return value;
}

+/* #define to provide old read_phy_reg functionality without
duplicating code */
+#define read_phy_reg(priv,regnum)
read_any_phy_reg(priv,priv->phyaddr,regnum)
+
/* Discover which PHY is attached to the device, and configure it
* properly. If the PHY is not recognized, then return 0
* (failure). Otherwise, return 1
@@ -1487,18 +1494,6 @@
#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) \
&& !defined(BITBANGMII)

-struct tsec_private *get_priv_for_phy(unsigned char phyaddr)
-{
- int i;
-
- for (i = 0; i < MAXCONTROLLERS; i++) {
- if (privlist[i]->phyaddr == phyaddr)
- return privlist[i];
- }
-
- return NULL;
-}
-
/*
* Read a MII PHY register.
*
@@ -1509,14 +1504,14 @@
unsigned char reg, unsigned short *value)
{
unsigned short ret;
- struct tsec_private *priv = get_priv_for_phy(addr);
+ struct tsec_private *priv = privlist[0];

if (NULL == priv) {
printf("Can't read PHY at address %d\n", addr);
return -1;
}

- ret = (unsigned short)read_phy_reg(priv, reg);
+ ret = (unsigned short)read_any_phy_reg(priv, addr, reg);
*value = ret;

return 0;
@@ -1531,14 +1526,14 @@
static int tsec_miiphy_write(char *devname, unsigned char addr,
unsigned char reg, unsigned short value)
{
- struct tsec_private *priv = get_priv_for_phy(addr);
+ struct tsec_private *priv = privlist[0];

if (NULL == priv) {
printf("Can't write PHY at address %d\n", addr);
return -1;
}

- write_phy_reg(priv, reg, value);
+ write_any_phy_reg(priv, addr, reg, value);

return 0;
}

Signed-off-by: Michael Firth <michael.firth@bt.com>