[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH] ftgmac100: implement the new MDIO interface on As
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-arm] [PATCH] ftgmac100: implement the new MDIO interface on Aspeed SoC |
Date: |
Mon, 14 Jan 2019 08:32:42 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
On 1/14/19 4:29 AM, Joel Stanley wrote:
> On Fri, 11 Jan 2019 at 23:58, Cédric Le Goater <address@hidden> 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 <address@hidden>
>> ---
>> 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?
It is. Check do_phy_new_ctl() passing a 'uint8_t reg'.
There is not much point of adding these small changes without the bigger
one adding the new MDC/MDIO interfaces. That's why I merged them all in
one single patch.
>> @@ -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.
Yes. 0 is the default setting of 'Feature Register'
> Did you give this a go with both -kernel and a u-boot mtd image?
Yes.
> 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 <address@hidden>
Thanks,
C.
> Cheers,
>
> Joel
>