Re: [PATCH net-next] net: phy: fix two issues with linkmode bitmaps

[Andrew Lunn <andrew@lunn.ch>]

On Sun, Nov 25, 2018 at 03:23:42PM +0100, Heiner Kallweit wrote:
> I wondered why ethtool suddenly reports that link partner doesn't
> support aneg and GBit modes. It turned out that this is caused by two
> bugs in conversion to linkmode bitmaps.
> 
> 1. In genphy_read_status the value of phydev->lp_advertising is
>    overwritten, thus GBit modes aren't reported any longer.
> 2. In mii_lpa_to_linkmode_lpa_t the aneg bit was overwritten by the
>    call to mii_adv_to_linkmode_adv_t.

Hi Heiner

Thanks for looking into this.

There are more bugs :-(

static inline void mii_lpa_to_linkmode_lpa_t(unsigned long *lp_advertising,
                                             u32 lpa)
{
        if (lpa & LPA_LPACK)
                linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
                                 lp_advertising);

        mii_adv_to_linkmode_adv_t(lp_advertising, lpa);
}

But

static inline void mii_adv_to_linkmode_adv_t(unsigned long *advertising,
                                             u32 adv)
{
        linkmode_zero(advertising);

        if (adv & ADVERTISE_10HALF)
                linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
                                 advertising);
 
So the Autoneg_BIT gets cleared.

I think the better fix is to take the linkmode_zero() out from here.

Then:

        if (adv & ADVERTISE_10HALF)
               linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
                                advertising);
+	else
+              linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+                                 advertising);

for all the bits mii_adv_to_linkmode_adv_t() looks at.

So mii_adv_to_linkmode_adv_t() only modifies bits it is responsible
for, and leaves the others alone.

    Andrew