s-paul...@ti.com wrote:
> From: Sandeep Paulraj <s-paul...@ti.com>
> 
> The EMAC IP on DM365 and DM646x is slightly different from
> that on DM644x. This patch updates the DaVinci EMAC driver
> so that EMAC becomes operational on DM365 in U-Boot.
> A flag 'CONFIG_DAVINCI_EMAC_VERSION2' is used in the driver.
> This flag will need to be defined in the DM365 config file.
> 
> Signed-off-by: Sandeep Paulraj <s-paul...@ti.com>
> ---
> The same modifications work on DM646x in a slightly older version
> of U-Boot. So when enabled this should work on the DM6467 EVM as well.
> This has at this point of time not been tested on the DM6467 in the latest
> version of U-Boot.
>  drivers/net/davinci_emac.c |   79 ++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 77 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
> index fa8cee4..0d61c91 100644
> --- a/drivers/net/davinci_emac.c
> +++ b/drivers/net/davinci_emac.c
> @@ -107,6 +107,33 @@ static void davinci_eth_mdio_enable(void)
>       while (adap_mdio->CONTROL & MDIO_CONTROL_IDLE) {;}

checkpatch complains here.
infinite loop statement should be on the next line.

ERROR: trailing statements should be on next line
#44: FILE: drivers/net/davinci_emac.c:119:
+       while ((adap_mdio->USERACCESS0 & MDIO_USERACCESS0_GO) != 0);

>  }
>  
> +/* Read a PHY register via MDIO inteface */
> +static int mdio_read(int phy_addr, int reg_num)
> +{
> +     adap_mdio->USERACCESS0 = MDIO_USERACCESS0_GO |
> +                                     MDIO_USERACCESS0_WRITE_READ |
> +                                     ((reg_num & 0x1F) << 21) |
> +                                     ((phy_addr & 0x1F) << 16);
> +
> +     /* Wait for command to complete */
> +     while ((adap_mdio->USERACCESS0 & MDIO_USERACCESS0_GO) != 0);
> +
> +     return adap_mdio->USERACCESS0 & 0xFFFF;
> +}
> +
> +/* Write to a PHY register via MDIO inteface */
> +void mdio_write(int phy_addr, int reg_num, unsigned int data)
> +{
> +     /* Wait for User access register to be ready */
> +     while ((adap_mdio->USERACCESS0 & MDIO_USERACCESS0_GO) != 0);

checkpatch complains here.
infinite loop statement should be on the next line.

ERROR: trailing statements should be on next line
#53: FILE: drivers/net/davinci_emac.c:128:
+       while ((adap_mdio->USERACCESS0 & MDIO_USERACCESS0_GO) != 0);


> +
> +     adap_mdio->USERACCESS0 = MDIO_USERACCESS0_GO |
> +                                     MDIO_USERACCESS0_WRITE_WRITE |
> +                                     ((reg_num & 0x1F) << 21) |
> +                                     ((phy_addr & 0x1F) << 16) |
> +                                     (data & 0xFFFF);
> +}
> +
>  /*
>   * Tries to find an active connected PHY. Returns 1 if address if found.
>   * If no active PHY (or more than one PHY) found returns 0.
> @@ -248,6 +275,31 @@ static int davinci_mii_phy_write(char *devname, unsigned 
> char addr, unsigned cha
>  
>  #endif
>  
> +static void emac_gigabit_enable(void)
> +{
> +     int temp;
> +
> +     temp = mdio_read(EMAC_MDIO_PHY_NUM, 0);
> +
> +     if (temp & (1 << 6)) {

Is there a logical #define bitfield that could be used here ?

> +             /*
> +              * Check if link detected is giga-bit
> +              * If Gigabit mode detected, enable gigbit in MAC and PHY
> +              */
> +             adap_emac->MACCONTROL |= EMAC_MACCONTROL_GIGFORCE |
> +                                             EMAC_MACCONTROL_GIGABIT_ENABLE;
> +
> +             /*
> +              * The SYS_CLK which feeds the SOC for giga-bit operation
> +              * does not seem to be enabled after reset as expected.
> +              * Force enabling SYS_CLK by writing to the PHY
> +              */
> +             temp = mdio_read(EMAC_MDIO_PHY_NUM, 22);
> +             temp |= (1 << 4);
> +             mdio_write(EMAC_MDIO_PHY_NUM, 22, temp);
> +     }
> +}
> +
>  
>  /* Eth device open */
>  static int davinci_eth_open(struct eth_device *dev, bd_t *bis)
> @@ -261,10 +313,15 @@ static int davinci_eth_open(struct eth_device *dev, 
> bd_t *bis)
>       /* Reset EMAC module and disable interrupts in wrapper */
>       adap_emac->SOFTRESET = 1;
>       while (adap_emac->SOFTRESET != 0) {;}

checkpatch complains here.
infinite loop statement should be on the next line.

ERROR: trailing statements should be on next line
#103: FILE: drivers/net/davinci_emac.c:318:
+       while (adap_ewrap->SOFTRST != 0);


> +#if defined(CONFIG_DAVINCI_EMAC_VERSION2)
> +     adap_ewrap->SOFTRST = 1;
> +     while (adap_ewrap->SOFTRST != 0);
> +#else
>       adap_ewrap->EWCTL = 0;
>       for (cnt = 0; cnt < 5; cnt++) {
>               clkdiv = adap_ewrap->EWCTL;
>       }
> +#endif

You may want to handle this like

#if defined(CONFIG_DAVINIC_EMAC_VERSION) && CONFIG_DAVINCI_EMAC_VERSION >= 2

>  
>       rx_desc = emac_rx_desc;
>  
> @@ -282,6 +339,10 @@ static int davinci_eth_open(struct eth_device *dev, bd_t 
> *bis)
>       adap_emac->MACADDRLO =
>               (davinci_eth_mac_addr[5] << 8) |
>               (davinci_eth_mac_addr[4]);
> +#if defined(CONFIG_DAVINCI_EMAC_VERSION2)
> +     /* Set the Match and Valid Bits */
> +     adap_emac->MACADDRLO |= (1 << 19) | (1 << 20);

Are there some logical #define bitfields that could be used here ?

> +#endif
>  
>       adap_emac->MACHASH1 = 0;
>       adap_emac->MACHASH2 = 0;
> @@ -347,8 +408,15 @@ static int davinci_eth_open(struct eth_device *dev, bd_t 
> *bis)
>       clkdiv = (EMAC_MDIO_BUS_FREQ / EMAC_MDIO_CLOCK_FREQ) - 1;
>       adap_mdio->CONTROL = ((clkdiv & 0xff) | MDIO_CONTROL_ENABLE | 
> MDIO_CONTROL_FAULT);
>  
> +#if defined(CONFIG_DAVINCI_EMAC_VERSION2)
> +     /* We need to wait for MDIO to start */
> +     udelay(1000);
> +#endif
> +
>       if (!phy.get_link_speed(active_phy_addr))
>               return(0);
> +     else
> +             emac_gigabit_enable();
>  
>       /* Start receive process */
>       adap_emac->RX0HDP = (u_int32_t)emac_rx_desc;
> @@ -411,7 +479,11 @@ static void davinci_eth_close(struct eth_device *dev)
>  
>       /* Reset EMAC module and disable interrupts in wrapper */
>       adap_emac->SOFTRESET = 1;
> +#if defined(CONFIG_DAVINCI_EMAC_VERSION2)
> +     adap_ewrap->SOFTRST = 1;
> +#else
>       adap_ewrap->EWCTL = 0;
> +#endif
>  
>       debug_emac("- emac_close\n");
>  }
> @@ -433,7 +505,8 @@ static int davinci_eth_send_packet (struct eth_device 
> *dev,
>       if (!phy.get_link_speed (active_phy_addr)) {
>               printf ("WARN: emac_send_packet: No link\n");
>               return (ret_status);
> -     }
> +     } else
> +             emac_gigabit_enable();

This is a dangling 'else'
This is not explicitly mentioned in the coding style.
My opinion is to surround with {}


>  
>       /* Check packet size and if < EMAC_MIN_ETHERNET_PKT_SIZE, pad it up */
>       if (length < EMAC_MIN_ETHERNET_PKT_SIZE) {
> @@ -456,7 +529,9 @@ static int davinci_eth_send_packet (struct eth_device 
> *dev,
>               if (!phy.get_link_speed (active_phy_addr)) {
>                       davinci_eth_ch_teardown (EMAC_CH_TX);
>                       return (ret_status);
> -             }
> +             } else
> +                     emac_gigabit_enable();
> +
Another dangling else

>               if (adap_emac->TXINTSTATRAW & 0x01) {
>                       ret_status = length;
>                       break;

Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to