[U-Boot] [PATCH] TI: DaVinci: Updating EMAC driver for DM365, DM646x and DA8XX

[Sriramakrishnan <srk@ti.com>]

Nick Thompson <nick.thompson <at> ge.com> writes:

> +/* 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 (readl(&adap_mdio->USERACCESS0) & MDIO_USERACCESS0_GO);
> +

Arent these functions duplicating functionality in davinci_eth_phy_write() and 
davinci_eth_phy_read()?  Is the intent here to move from accessing registers 
as overlay to using writel/readl functions. If so, we need to get rid of one 
of these implementations

> +static void emac_gigabit_enable(void)
> +{
> +#ifdef DAVINCI_EMAC_GIG_ENABLE
> +	int temp
> +
> +	if (mdio_read(EMAC_MDIO_PHY_NUM, 0) & (1 << 6)) {
> +		/*
> +		 * Check if link detected is giga-bit
> +		 * If Gigabit mode detected, enable gigbit in MAC and PHY
> +		 */
> +		writel(EMAC_MACCONTROL_GIGFORCE |
> +		       EMAC_MACCONTROL_GIGABIT_ENABLE,
> +		       &adap_emac->MACCONTROL);
> +
> +		/*
> +		 * 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);
> +	}
> +#endif
> +}
PHY register 22 is vendor specific(specific to a PHY) and the same PHY need 
not be Used on all platforms which use this driver. Also, can this not be done 
once during initialization alone?

> 
> +#if defined(DAVINCI_EMAC_VERSION2)
> +	writel(1, &adap_ewrap->softrst);
> +	while (readl(&adap_ewrap->softrst) != 0);
> +#else
>  	adap_ewrap->EWCTL = 0;
>  	for (cnt = 0; cnt < 5; cnt++) {
>  		clkdiv = adap_ewrap->EWCTL;
>  	}
> +#endif

Again some registers are accessed through overlays and some are accessed 
through readl/writel(). Should we make this consistent? Also the register 
names use CAPs in some instances and not for others - is this intentional?

>  	/* Init MDIO & get link state */
>  	clkdiv = (EMAC_MDIO_BUS_FREQ / EMAC_MDIO_CLOCK_FREQ) - 1;
>  	adap_mdio->CONTROL = ((clkdiv & 0xff) | MDIO_CONTROL_ENABLE | 
MDIO_CONTROL_FAULT);
> 
> +#if defined(DAVINCI_EMAC_VERSION2)
> +	/* We need to wait for MDIO to start */
> +	udelay(1000);
> +#endif
> +
 
The controller doesnt specify/expect a delay for EMAC version2. How is the 
delay specific to EMAC version2?