> -----Original Message----- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf > Of [EMAIL PROTECTED] > Sent: 09 January 2008 20:26 > To: [EMAIL PROTECTED] > Cc: u-boot-users@lists.sourceforge.net > Subject: Re: [U-Boot-Users] TSEC Ethernet driver patch - RFC > > > > -----Original Message----- > > From: Ben Warren [mailto:[EMAIL PROTECTED] > > Sent: 08 January 2008 16:42 > > To: Firth,MJC,Michael,DMM R > > Cc: u-boot-users@lists.sourceforge.net > > Subject: Re: [U-Boot-Users] Possible TSEC Ethernet driver patch > > > > [EMAIL PROTECTED] 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 <[EMAIL PROTECTED]> > Does the lack of comments on this mean that people are happy with it, or that they haven't had a chance to consider it yet?
Thanks Michael ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
