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?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
