From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH net-next 08/10] r8169: remove rtl8169_set_speed_xmii Date: Mon, 2 Jul 2018 23:54:54 +0200 Message-ID: <30dcf9f0-b5a0-1690-6964-a0afff6c3fec@gmail.com> References: <096a5326-963c-9bef-6218-29fcde004111@gmail.com> <20180702212150.GG12564@lunn.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: David Miller , Florian Fainelli , Realtek linux nic maintainers , "netdev@vger.kernel.org" To: Andrew Lunn Return-path: Received: from mail-wm0-f52.google.com ([74.125.82.52]:33783 "EHLO mail-wm0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753224AbeGBVzD (ORCPT ); Mon, 2 Jul 2018 17:55:03 -0400 Received: by mail-wm0-f52.google.com with SMTP id z6-v6so8854447wma.0 for ; Mon, 02 Jul 2018 14:55:02 -0700 (PDT) In-Reply-To: <20180702212150.GG12564@lunn.ch> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 02.07.2018 23:21, Andrew Lunn wrote: >> - auto_nego |= ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM; > > This bit you probably want to keep. The PHY never says it support > Pause. The MAC needs to enable pause if the MAC supports pause. > Actually I assumed that phylib would do this for me. But: In phy_probe() first phydev->supported is copied to phydev->advertising, and only after this both pause flags are added to phydev->supported. Therefore I think they are not advertised. Is this intentional? It sounds a little weird to me to add the pause flags to the supported features per default, but not advertise them. Except e.g. we call by chance phy_set_max_speed(), which copies phydev->supported to phydev->advertising after having adjusted the supported speeds. If this is not a bug, then where would be the right place to add the pause flags to phydev->advertising? > Andrew > Heiner