Re: [U-Boot] [PATCH V3 07/11] fec_mxc: add support for MX51 processor

Hi Stefano,

Sorry for taking so long to provide feedback here. My requests should be pretty quick :)

Stefano Babic wrote:

The patch add support for the Freescale mx51 processor to the FEC ethernet driver.

Signed-off-by: Stefano Babic sbabic@denx.de

drivers/net/fec_mxc.c | 68 +++++++++++++++++++++--------------------------- 1 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 19116f2..d440e7a 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -108,6 +108,17 @@ static int fec_miiphy_read(char *dev, uint8_t phyAddr, uint8_t regAddr, return 0; }

+static void fec_mii_setspeed(struct fec_priv *fec) +{

• /*

• * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
  

• * and do not drop the Preamble.
  

• */
  

• writel((((imx_get_fecclk() / 1000000) + 2) / 5) << 1,

• 	&fec->eth->mii_speed);
  

• debug("fec_init: mii_speed %#lx\n",

• 	fec->eth->mii_speed);
  

+} static int fec_miiphy_write(char *dev, uint8_t phyAddr, uint8_t regAddr, uint16_t data) { @@ -236,7 +247,7 @@ static int fec_rbd_init(struct fec_priv *fec, int count, int size) fec->rdb_ptr = malloc(size * count + DB_DATA_ALIGNMENT); p = (uint32_t)fec->rdb_ptr; if (!p) {

• puts("fec_imx27: not enough malloc memory!\n");
  

• puts("fec_mxc: not enough malloc memory\n");
  

  return -ENOMEM; } memset((void *)p, 0, size * count + DB_DATA_ALIGNMENT);

@@ -299,6 +310,13 @@ static void fec_rbd_clean(int last, struct fec_bd *pRbd)

static int fec_get_hwaddr(struct eth_device *dev, unsigned char *mac) { +/*

• • The MX27 can store the mac address in internal eeprom
• • This mechanism is not supported now by MX51
• */

+#ifdef CONFIG_MX51

• return -1;

+#else struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE; int i;

@@ -306,6 +324,7 @@ static int fec_get_hwaddr(struct eth_device *dev, unsigned char *mac) mac[6-1-i] = readl(&iim->iim_bank_area0[IIM0_MAC + i]);

return is_valid_ether_addr(mac); +#endif }

static int fec_set_hwaddr(struct eth_device *dev, unsigned char *mac) @@ -373,7 +392,7 @@ static int fec_init(struct eth_device *dev, bd_t* bd) sizeof(struct fec_bd) + DB_ALIGNMENT); base = (uint32_t)fec->base_ptr; if (!base) {

• puts("fec_imx27: not enough malloc memory!\n");
  

• puts("fec_mxc: not enough malloc memory\n");
  

  return -ENOMEM; } memset((void *)base, 0, (2 + FEC_RBD_NUM) *

@@ -411,14 +430,8 @@ static int fec_init(struct eth_device *dev, bd_t* bd) * Frame length=1518; MII mode; */ writel(0x05ee0024, &fec->eth->r_cntrl); /* FIXME 0x05ee0004 */

• /*
  

•  * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
  

•  * and do not drop the Preamble.
  

•  */
  

• writel((((imx_get_ahbclk() / 1000000) + 2) / 5) << 1,
  

• 		&fec->eth->mii_speed);
  

• debug("fec_init: mii_speed %#lx\n",
  

• 		(((imx_get_ahbclk() / 1000000) + 2) / 5) << 1);
  

• fec_mii_setspeed(fec);
  

  } /*
  • Set Opcode/Pause Duration Register

@@ -522,7 +535,7 @@ static int fec_send(struct eth_device *dev, volatile void* packet, int length) * Check for valid length of data. */ if ((length > 1500) || (length <= 0)) {

• printf("Payload (%d) to large!\n", length);
  

• printf("Payload (%d) to large\n", length);
  
  

s/to/too/

return -1;

}

@@ -651,22 +664,14 @@ static int fec_recv(struct eth_device *dev)

static int fec_probe(bd_t *bd) {

• struct pll_regs *pll = (struct pll_regs *)IMX_PLL_BASE; struct eth_device *edev; struct fec_priv *fec = &gfec;

• unsigned char ethaddr_str[20]; unsigned char ethaddr[6];

• char *tmp = getenv("ethaddr");

• char *end;

• /* enable FEC clock */

• writel(readl(&pll->pccr1) | PCCR1_HCLK_FEC, &pll->pccr1);

• writel(readl(&pll->pccr0) | PCCR0_FEC_EN, &pll->pccr0);

  /* create and fill edev struct */ edev = (struct eth_device *)malloc(sizeof(struct eth_device)); if (!edev) {

• puts("fec_imx27: not enough malloc memory!\n");
  

• puts("fec_mxc: not enough malloc memory\n");
  

  return -ENOMEM; } edev->priv = fec;

@@ -702,14 +707,7 @@ static int fec_probe(bd_t *bd) * Frame length=1518; MII mode; */ writel(0x05ee0024, &fec->eth->r_cntrl); /* FIXME 0x05ee0004 */

• /*

• * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
  

• * and do not drop the Preamble.
  

• */
  

• writel((((imx_get_ahbclk() / 1000000) + 2) / 5) << 1,

• 	&fec->eth->mii_speed);
  

• debug("fec_init: mii_speed %#lx\n",

• 	(((imx_get_ahbclk() / 1000000) + 2) / 5) << 1);
  

• fec_mii_setspeed(fec);

  sprintf(edev->name, "FEC_MXC");

@@ -717,17 +715,11 @@ static int fec_probe(bd_t *bd)

eth_register(edev);

• if ((NULL != tmp) && (12 <= strlen(tmp))) {

• int i;
  

• /* convert MAC from string to int */
  

• for (i = 0; i < 6; i++) {
  

• 	ethaddr[i] = tmp ? simple_strtoul(tmp, &end, 16) : 0;
  

• 	if (tmp)
  

• 		tmp = (*end) ? end + 1 : end;
  

• if (!eth_getenv_enetaddr("ethaddr", ethaddr)) {

• if (fec_get_hwaddr(edev, ethaddr) == 0) {
  

• 	printf("got MAC address from EEPROM: %pM\n", ethaddr);
  

• 	setenv("ethaddr", (char *)ethaddr);
  

  }

• } else if (fec_get_hwaddr(edev, ethaddr) == 0) {

• printf("got MAC address from EEPROM: %pM\n", ethaddr);
  

• setenv("ethaddr", (char *)ethaddr_str);
  

  } memcpy(edev->enetaddr, ethaddr, 6); fec_set_hwaddr(edev, ethaddr);

Your driver shouldn't touch the environment (i.e. no getenv/setenv calls). Just save the value from ROM into edev->enetaddr, like this: if (fec_get_hwaddr(edev, ethaddr) == 0) memcpy(edev->enetaddr, ethaddr, 6);

The core net code will take care of overwriting this if there's a value in the environment. Please see README.enetaddr if this isn't clear.

regards, Ben