On Fri, 11 Jan 2019 at 23:58, Cédric Le Goater <c...@kaod.org> wrote:
>
> The PHY behind the MAC of an Aspeed SoC can be controlled using two
> different MDC/MDIO interfaces. The same registers PHYCR (MAC60) and
> PHYDATA (MAC64) are involved but they have a different layout.
>
> BIT31 of the Feature Register (MAC40) controls which MDC/MDIO
> interface is active.
>
> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> ---
> hw/net/ftgmac100.c | 80 +++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 68 insertions(+), 12 deletions(-)
> @@ -269,9 +281,9 @@ static void phy_reset(FTGMAC100State *s)
> s->phy_int = 0;
> }
>
> -static uint32_t do_phy_read(FTGMAC100State *s, int reg)
> +static uint16_t do_phy_read(FTGMAC100State *s, uint8_t reg)
> {
> - uint32_t val;
> + uint16_t val;
Unrelated?
> @@ -336,7 +348,7 @@ static uint32_t do_phy_read(FTGMAC100State *s, int reg)
> MII_BMCR_FD | MII_BMCR_CTST)
> #define MII_ANAR_MASK 0x2d7f
>
> -static void do_phy_write(FTGMAC100State *s, int reg, uint32_t val)
> +static void do_phy_write(FTGMAC100State *s, uint8_t reg, uint16_t val)
Also unrelated?
> @@ -711,14 +771,11 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
> break;
>
> case FTGMAC100_PHYCR: /* PHY Device control */
> - reg = FTGMAC100_PHYCR_REG(value);
> s->phycr = value;
> - if (value & FTGMAC100_PHYCR_MIIWR) {
> - do_phy_write(s, reg, s->phydata & 0xffff);
> - s->phycr &= ~FTGMAC100_PHYCR_MIIWR;
> + if (s->revr & FTGMAC100_REVR_NEW_MDIO_INTERFACE) {
> + do_phy_new_ctl(s);
> } else {
> - s->phydata = do_phy_read(s, reg) << 16;
> - s->phycr &= ~FTGMAC100_PHYCR_MIIRD;
> + do_phy_ctl(s);
I assume the guest code programs the correct phy mode so this will
work fine. This change appears to keep the existing default of the old
mode.
Did you give this a go with both -kernel and a u-boot mtd image?
Looks good to me. If you feel like splitting out the unrelated changes
do that, but I'm not fussed either way.
Reviewed-by: Joel Stanley <j...@jms.id.au>
Cheers,
Joel
