Re: [U-Boot] [PATCH v2 1/6] tsec: use IO accessors for IO accesses

Hi Andy,

From: Mingkai Hu Mingkai.hu@freescale.com

Signed-off-by: Mingkai Hu Mingkai.hu@freescale.com Acked-by: Andy Fleming afleming@freescale.com Signed-off-by: Kumar Gala galak@kernel.crashing.org

---

Fixed the commit message's spelling

drivers/net/tsec.c | 242 +++++++++++++++++++++++++++------------------------- include/tsec.h | 8 +- 2 files changed, 130 insertions(+), 120 deletions(-)

Despite what Kumar says I can't see that patch in mpc85xx/next so let's see.

diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c index 9a91b9e..4ea9813 100644 --- a/drivers/net/tsec.c +++ b/drivers/net/tsec.c

[...]

@@ -230,17 +230,18 @@ static int tsec_init(struct eth_device *dev, bd_t * bd) }

/* Writes the given phy's reg with value, using the specified MDIO regs */ -static void tsec_local_mdio_write(volatile tsec_mdio_t *phyregs, uint addr, +static void tsec_local_mdio_write(tsec_mdio_t *phyregs, uint addr, uint reg, uint value) { int timeout = 1000000;

• phyregs->miimadd = (addr << 8) | reg;
• phyregs->miimcon = value;

• out_be32(&phyregs->miimadd, (addr << 8) | reg);
• out_be32(&phyregs->miimcon, value); asm("sync");

We don't need the sync here anymore, as this is done by the next in_be32.

timeout = 1000000;

• while ((phyregs->miimind & MIIMIND_BUSY) && timeout--) ;

• while ((in_be32(&phyregs->miimind) & MIIMIND_BUSY) && timeout--)

• ;
  

}

@@ -254,28 +255,28 @@ static void tsec_local_mdio_write(volatile tsec_mdio_t *phyregs, uint addr,

• notvalid bit cleared), and the bus to cease activity (miimind
• busy bit cleared), and then returns the value

*/ -static uint tsec_local_mdio_read(volatile tsec_mdio_t *phyregs,

• 		uint phyid, uint regnum)
  

+static uint tsec_local_mdio_read(tsec_mdio_t *phyregs, uint phyid, uint regnum) { uint value;

/* Put the address of the phy, and the register * number into MIIMADD */

• phyregs->miimadd = (phyid << 8) | regnum;

• out_be32(&phyregs->miimadd, (phyid << 8) | regnum);

  /* Clear the command register, and wait */

• phyregs->miimcom = 0;

• out_be32(&phyregs->miimcom, 0); asm("sync");

Not needed anymore, done by the next accessor.

/* Initiate a read command, and wait */

• phyregs->miimcom = MIIM_READ_COMMAND;

• out_be32(&phyregs->miimcom, MIIM_READ_COMMAND); asm("sync");

Dito.

/* Wait for the the indication that the read is done */

• while ((phyregs->miimind & (MIIMIND_NOTVALID | MIIMIND_BUSY))) ;

• while ((in_be32(&phyregs->miimind) & (MIIMIND_NOTVALID | MIIMIND_BUSY)))

• ;
  
  

  /* Grab the value read from the PHY */

• value = phyregs->miimstat;

• value = in_be32(&phyregs->miimstat);

  return value;

} @@ -321,18 +322,19 @@ static int init_phy(struct eth_device *dev) { struct tsec_private *priv = (struct tsec_private *)dev->priv; struct phy_info *curphy;

• volatile tsec_t *regs = priv->regs;

• tsec_t *regs = priv->regs;

  /* Assign a Physical address to the TBI */

• regs->tbipa = CONFIG_SYS_TBIPA_VALUE;

• out_be32(&regs->tbipa, CONFIG_SYS_TBIPA_VALUE); asm("sync");

dito

/* Reset MII (due to new addresses) */

• priv->phyregs->miimcfg = MIIMCFG_RESET;

• out_be32(&priv->phyregs->miimcfg, MIIMCFG_RESET); asm("sync");

dito

• priv->phyregs->miimcfg = MIIMCFG_INIT_VALUE;

• out_be32(&priv->phyregs->miimcfg, MIIMCFG_INIT_VALUE); asm("sync");

once more

• while (priv->phyregs->miimind & MIIMIND_BUSY) ;

• while (in_be32(&priv->phyregs->miimind) & MIIMIND_BUSY)

• ;
  
  

  /* Get the cmd structure corresponding to the attached

  • PHY */

@@ -345,7 +347,7 @@ static int init_phy(struct eth_device *dev) return 0; }

• if (regs->ecntrl & ECNTRL_SGMII_MODE)

• if (in_be32(&regs->ecntrl) & ECNTRL_SGMII_MODE) tsec_configure_serdes(priv);

  priv->phyinfo = curphy;

@@ -838,16 +840,17 @@ static uint mii_parse_dm9161_scsr(uint mii_reg, struct tsec_private * priv) static uint mii_cis8204_fixled(uint mii_reg, struct tsec_private * priv) { uint phyid;

• volatile tsec_mdio_t *regbase = priv->phyregs;

• tsec_mdio_t *regbase = priv->phyregs; int timeout = 1000000;

  for (phyid = 0; phyid < 4; phyid++) {

• regbase->miimadd = (phyid << 8) | mii_reg;
  

• regbase->miimcon = MIIM_CIS8204_SLEDCON_INIT;
  

• out_be32(&regbase->miimadd, (phyid << 8) | mii_reg);
  

• out_be32(&regbase->miimcon, MIIM_CIS8204_SLEDCON_INIT);
  

  asm("sync");

and again

timeout = 1000000;

• while ((regbase->miimind & MIIMIND_BUSY) && timeout--) ;
  

• while ((in_be32(&regbase->miimind) & MIIMIND_BUSY) && timeout--)
  

• 	;
  

  }

  return MIIM_CIS8204_SLEDCON_INIT;

[...]

@@ -922,44 +925,49 @@ static void init_registers(volatile tsec_t * regs) static void adjust_link(struct eth_device *dev) { struct tsec_private *priv = (struct tsec_private *)dev->priv;

• volatile tsec_t *regs = priv->regs;

• tsec_t *regs = priv->regs;
• u32 ecntrl, maccfg2;

• if (priv->link) {

• if (priv->duplexity != 0)
  

• 	regs->maccfg2 |= MACCFG2_FULL_DUPLEX;
  

• else
  

• 	regs->maccfg2 &= ~(MACCFG2_FULL_DUPLEX);
  

• if (!priv->link) {

• printf("%s: No link.\n", dev->name);
  

• return;
  

• }

• switch (priv->speed) {
  

• case 1000:
  

• 	regs->maccfg2 = ((regs->maccfg2 & ~(MACCFG2_IF))
  

• 			 | MACCFG2_GMII);
  

• 	break;
  

• case 100:
  

• case 10:
  

• 	regs->maccfg2 = ((regs->maccfg2 & ~(MACCFG2_IF))
  

• 			 | MACCFG2_MII);
  

• /* clear all bits relative with interface mode */
• ecntrl = in_be32(&regs->ecntrl);
• ecntrl &= ~ECNTRL_R100;

• 	/* Set R100 bit in all modes although
  

• 	 * it is only used in RGMII mode
  

• 	 */
  

• 	if (priv->speed == 100)
  

• 		regs->ecntrl |= ECNTRL_R100;
  

• 	else
  

• 		regs->ecntrl &= ~(ECNTRL_R100);
  

• 	break;
  

• default:
  

• 	printf("%s: Speed was bad\n", dev->name);
  

• 	break;
  

• }
  

• maccfg2 = in_be32(&regs->maccfg2);
• maccfg2 &= ~(MACCFG2_IF | MACCFG2_FULL_DUPLEX);

• printf("Speed: %d, %s duplex%s\n", priv->speed,
  

•        (priv->duplexity) ? "full" : "half",
  

•        (priv->flags & TSEC_FIBER) ? ", fiber mode" : "");
  

• if (priv->duplexity)

• maccfg2 |= MACCFG2_FULL_DUPLEX;
  
  

• } else {

• printf("%s: No link.\n", dev->name);
  

• switch (priv->speed) {
• case 1000:

• maccfg2 |= MACCFG2_GMII;
  

• break;
  

• case 100:
• case 10:

• maccfg2 |= MACCFG2_MII;
  

• /* Set R100 bit in all modes although
  

•  * it is only used in RGMII mode
  

•  */
  

• if (priv->speed == 100)
  

• 	ecntrl |= ECNTRL_R100;
  

• break;
  

• default:

• printf("%s: Speed was bad\n", dev->name);
  

• break;
  

  }
• out_be32(&regs->ecntrl, ecntrl);
• out_be32(&regs->maccfg2, maccfg2);
• printf("Speed: %d, %s duplex%s\n", priv->speed,

• 	(priv->duplexity) ? "full" : "half",
  

• 	(priv->flags & TSEC_FIBER) ? ", fiber mode" : "");
  

}

Hm, this hunk is definitely more than changing to accessors, but if I read it correctly, it introduces a short-cut way out and thus changes indentation but does not change functionality. Still would have been nice to split this out into a change before and only do whats advertised in the patch description here.

Cheers Detlev