Re: [PATCH v2 2/4] net: ethoc: don't advertise gigabit speed on attached PHY

Max Filippov
2014-01-29T18:32:23Z

On Wed, Jan 29, 2014 at 9:12 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On Jan 28, 2014 11:01 PM, "Max Filippov" <jcmvbkbc@gmail.com> wrote:
>>
>> On Wed, Jan 29, 2014 at 10:47 AM, Florian Fainelli <f.fainelli@gmail.com>
>> wrote:
>> > Hi Max,
>> >
>> > Le 28/01/2014 22:00, Max Filippov a écrit :
>> >
>> >> OpenCores 10/100 Mbps MAC does not support speeds above 100 Mbps, but
>> >> does
>> >> not disable advertisement when PHY supports them. This results in
>> >> non-functioning network when the MAC is connected to a gigabit PHY
>> >> connected
>> >> to a gigabit switch.
>> >>
>> >> The fix is to disable gigabit speed advertisement on attached PHY
>> >> unconditionally.
>> >>
>> >> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>> >> ---
>> >> Changes v1->v2:
>> >> - disable both gigabit advertisement and support.
>> >>
>> >> drivers/net/ethernet/ethoc.c | 8 ++++++++
>> >> 1 file changed, 8 insertions(+)
>> >>
>> >> diff --git a/drivers/net/ethernet/ethoc.c
>> >> b/drivers/net/ethernet/ethoc.c
>> >> index 4de8cfd..5643b2d 100644
>> >> --- a/drivers/net/ethernet/ethoc.c
>> >> +++ b/drivers/net/ethernet/ethoc.c
>> >> @@ -688,6 +688,14 @@ static int ethoc_mdio_probe(struct net_device
>> >> *dev)
>> >> }
>> >>
>> >> priv->phy = phy;
>> >> + phy_update_advert(phy,
>> >> + ADVERTISED_1000baseT_Full |
>> >> + ADVERTISED_1000baseT_Half, 0);
>> >> + phy_start_aneg(phy);
>> >
>> >
>> > This does not look necessary, you should not have to call
>> > phy_start_aneg()
>> > because the PHY state machine is not yet started, at best this calls
>> > does
>> > nothing.
>>
>> This call actually makes the whole thing work, because otherwise once
>> gigabit
>> support is cleared from the supported mask genphy_config_advert does not
>> update gigabit advertisement register, leaving it enabled.
>
> OK, then we need to figure out what is wrong with ethoc since this is
> unusual.

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.

--
Thanks.
-- Max