From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH] net: ethernet: mediatek: fix warning in phy_start_aneg
Date: Tue, 8 Jan 2019 20:42:53 +0100
Message-ID: <8f58fd47-f7ff-399d-4c20-76fe4d72ba9a@gmail.com>
References: <20190108160526.18928-1-frank-w@public-files.de>
 <7acce775-4ab7-c867-c801-25ee004f6b38@gmail.com>
 <trinity-4ceb1d55-424f-46b8-b8e7-989770b249a2-1546974413848@3c-app-gmx-bs35>
 <CAGp9LzrUK=QWJsDwiTVG7vD7v8=jFp6bAm9ZiOh3c2dQk029uA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: Felix Fietkau <nbd@openwrt.org>, John Crispin <john@phrozen.org>,
        Nelson Chang <nelson.chang@mediatek.com>,
        "David S. Miller" <davem@davemloft.net>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
To: Sean Wang <sean.wang@kernel.org>,
        Frank Wunderlich <frank-w@public-files.de>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-wr1-f66.google.com ([209.85.221.66]:36994 "EHLO
        mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1731824AbfAHTnD (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 8 Jan 2019 14:43:03 -0500
In-Reply-To: <CAGp9LzrUK=QWJsDwiTVG7vD7v8=jFp6bAm9ZiOh3c2dQk029uA@mail.gmail.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 08.01.2019 20:29, Sean Wang wrote:
> On Tue, Jan 8, 2019 at 11:07 AM Frank Wunderlich
> <frank-w@public-files.de> wrote:
>>
>>
>>> In mtk_phy_connect() I would say this complete block can be removed.
>>> All this is handled internally by phylib.
>>>
>>>       dev->phydev->autoneg = AUTONEG_ENABLE;
>>>       dev->phydev->speed = 0;
>>>       dev->phydev->duplex = 0;
>>>
>>>       phy_set_max_speed(dev->phydev, SPEED_1000);
>>>       phy_support_asym_pause(dev->phydev);
>>>       linkmode_copy(dev->phydev->advertising, dev->phydev->supported);
>>>       linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
>>>                        dev->phydev->advertising);
>>>       phy_start_aneg(dev->phydev);
>>>
>>> Could you please test this?
>>
>> seems to work well, no warning in dmesg
>>
>> traffic 940Mbit sending, 900 receive
>>
> 
> The change to phy initialized valve determined by phylib looks good to
> me and simplify the mac driver a lot. You could have it in v2.
> 
>>> And a further proposal:
>>> Instead of the open-coded dev_info() in mtk_phy_connect_node()
>>> you could use phy_attached_info().
>>
>> have not tried this yet...because it's only an info-message and needs different parameters...have to look how i can convert it
> 
> I think it can be done in a separate patch
> 
>>
>>> Calling phy_start() in the ndo_open callback is fine, you can leave
>>> it there.
>>
>> have it re-added
>>
>> pushed to my repo: https://github.com/frank-w/BPI-R2-4.14/commits/5.0-rc
>>
>> regards Frank
> 
When talking about improvements: Also this

if (dev->phydev->link)
		netif_carrier_on(dev);
	else
		netif_carrier_off(dev);

in mtk_phy_link_adjust() isn't needed. It's done by phy_link_change() in phylib.