lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 31 Jan 2014 17:36:29 -0800 From: Florian Fainelli <f.fainelli@...il.com> To: Max Filippov <jcmvbkbc@...il.com> Cc: Marc Gauthier <marc@...ence.com>, Ben Hutchings <ben@...adent.org.uk>, LKML <linux-kernel@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>, Chris Zankel <chris@...kel.net>, "linux-xtensa@...ux-xtensa.org" <linux-xtensa@...ux-xtensa.org>, netdev <netdev@...r.kernel.org> Subject: Re: [PATCH v2 2/4] net: ethoc: don't advertise gigabit speed on attached PHY 2014-01-31 Max Filippov <jcmvbkbc@...il.com>: > On Sat, Feb 1, 2014 at 4:54 AM, Florian Fainelli <f.fainelli@...il.com> wrote: >> 2014-01-31 Florian Fainelli <f.fainelli@...il.com>: >>>>> Maybe they boot up with gigabit advertisement disabled in their PHY >>>>> and thus they don't see the problem? >>>>> >>>>>> Other drivers do the following: >>>>>> >>>>>> - connect to the PHY >>>>>> - phydev->supported = PHY_BASIC_FEATURES >>>>>> - phydev->advertising &= phydev->supported >>>>>> - start the PHY state machine >>>>>> >>>>>> And they work just fine. Is the PHY driver you are bound to the "Generic >>>>>> PHY" or something else which does something funky in config_aneg()? >>>>> >>>>> It's marvell 88E1111 from the KC-705 board, but the behaviour doesn't >>>>> change if I disable it and the generic phy is used. >>>> >>>> Florian, >>>> >>>> I don't see how the generic genphy_config_advert can ever change >>>> gigabit advertisement if phydev->supported has gigabit speeds masked >>>> off. >>> >>> It does not, but it makes sure that phydev->advertising gets masked >>> with phydev->supported. So, prior to starting your PHY state machine, >>> if you do: >>> >>> phydev->supported &= PHY_BASIC_FEATURES; >>> phydev->advertising = phydev->supported; >> >> It looks like there might be one problem however, if we have the following: >> >> - phydev->supported masks off the Gigabit features >> - PHY device comes out of reset/default with Gigabit features set in >> MII_CTRL1000 > > That's exactly my case. > >> Could you try the following patch: >> >> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c >> index 78bf1a4..b607f4a 100644 >> --- a/drivers/net/phy/phy_device.c >> +++ b/drivers/net/phy/phy_device.c >> @@ -749,25 +749,24 @@ static int genphy_config_advert(struct phy_device *phydev) >> } >> >> /* Configure gigabit if it's supported */ >> + adv = phy_read(phydev, MII_CTRL1000); >> + if (adv < 0) >> + return adv; >> + >> + oldadv = adv; >> + adv &= ~(ADVERTISE_1000FULL | ADVERTISE_1000HALF); >> + >> if (phydev->supported & (SUPPORTED_1000baseT_Half | >> SUPPORTED_1000baseT_Full)) { >> - adv = phy_read(phydev, MII_CTRL1000); >> - if (adv < 0) >> - return adv; >> - >> - oldadv = adv; >> - adv &= ~(ADVERTISE_1000FULL | ADVERTISE_1000HALF); >> adv |= ethtool_adv_to_mii_ctrl1000_t(advertise); >> - >> - if (adv != oldadv) { >> - err = phy_write(phydev, MII_CTRL1000, adv); >> - >> - if (err < 0) >> - return err; >> + if (adv != oldadv) >> changed = 1; >> - } >> } >> >> + err = phy_write(phydev, MII_CTRL1000, adv); > > > I don't think this is correct: MII_CTRL1000 is reserved for 10/100 PHYs, > we probably need to read/write it conditionally, depending on MII_BMSR > BMSR_ESTATEN bit. > Otherwise yes, it works for me too. You are right, that needs to be made conditional. I will give this patch some more proper testing on a truly non-gigabit PHY. Thanks! -- Florian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists