From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from vps0.lunn.ch (vps0.lunn.ch [156.67.10.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 98F2F348C4B; Fri, 3 Jul 2026 20:37:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=156.67.10.101 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783111064; cv=none; b=jfeWUybPNCxxswrgQysLrBSZXgeiuHI5ybrutj1QCCOvzKkRDoVWH1wVUznju+pxagdwUgwdui8EzB95NxzzIag3H+djV2TERyGhdJ3DSgaCQxLIrMEEtkEX7pBZn/UVibDaWj8/Z9uMN0Pb8XsUkB8g6LZ+qqby+ovvw+SyzGU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783111064; c=relaxed/simple; bh=Gu/0HMyu/VRKn+7s9pXyNtz7xFiVrICskxvLGDADA4o=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=huKVvWxZT9tiHpYiWGB9bz9bvfKxfOE7HOB4rdMBpB4du9SSFN2cKXOkl0lrmHTAVkSfZQaolfW9PbNXOIZ2yNsvI9juJ45pX5Nif2PaPWhUAQbM4jUY2VJ7N4j6vjcRJl6oD6G4u82WGzuYWrVmrsm171n7h1ilcEoPOekzSTs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lunn.ch; spf=pass smtp.mailfrom=lunn.ch; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b=kDtPeHY8; arc=none smtp.client-ip=156.67.10.101 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lunn.ch Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=lunn.ch Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="kDtPeHY8" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=XRM9uENndVb9k8FDKtQxbN4i2xsVS7kwtcnJa3qVais=; b=kDtPeHY8J/uTFtszkVtL01Knah g5VSNO5X+1KNoDB+WgXIRri+0noH6dBNaO+UXcQ+LY4UiZqPSa7mRQxy9TjWdPs5NQ7lIQztkNzGO 10UnuIxvz0IQk/fF517RGoXOBv3blU1xF2tgn11z1fA5mS7v/voSubZhS71Mj+4oU7sc=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1wfkde-00AbYr-9b; Fri, 03 Jul 2026 22:37:30 +0200 Date: Fri, 3 Jul 2026 22:37:30 +0200 From: Andrew Lunn To: javen Cc: hkallweit1@gmail.com, nic_swsd@realtek.com, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, horms@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next v4 2/6] r8169: add support for phylink Message-ID: References: <20260703092459.1124-1-javen_xu@realsil.com.cn> <20260703092459.1124-3-javen_xu@realsil.com.cn> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260703092459.1124-3-javen_xu@realsil.com.cn> > + if (jumbo) { > + if (!tp->jumbo_pause_saved) { > + struct ethtool_link_ksettings cmd = {}; > + bool adv_pause, adv_asym; > + > + phylink_ethtool_get_pauseparam(tp->phylink, &tp->saved_pause); > + if (tp->saved_pause.autoneg) { > + phylink_ethtool_ksettings_get(tp->phylink, &cmd); > + adv_pause = ethtool_link_ksettings_test_link_mode(&cmd, > + advertising, > + Pause); > + adv_asym = ethtool_link_ksettings_test_link_mode(&cmd, > + advertising, > + Asym_Pause); > + if (adv_pause && !adv_asym) { > + tp->saved_pause.rx_pause = 1; > + tp->saved_pause.tx_pause = 1; This does not look correct. Pause is negotiated. In order to determine how to program the MAC you need to look at what the local side is advertising, and what the link peer is advertising. I don't see anything here about lp_. I forget what the issues is. Is it something like, if you are using a normal MTU, pause is supported? But with jumbo MTU it is not? For this to work correctly, i would expect a change of MTU to trigger a new autoneg, with the local advertisement changed. That might require changes in phylink, since it does not expect this sort of thing. > @@ -5288,6 +5326,8 @@ static void rtl_remove_one(struct pci_dev *pdev) > r8169_remove_leds(tp->leds); > > unregister_netdev(tp->dev); > + if (tp->phylink) > + phylink_destroy(tp->phylink); I've not looked in detail, but is tp->phylink optional? I would expect the probe to fail if it could not create it. > +static int rtl_mac_enable_tx_lpi(struct phylink_config *config, u32 timer, bool tx_clk_stop) > +{ > + struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config); > + > + if (!rtl_supports_eee(tp)) > + return -EOPNOTSUPP; Can that happen? You should only be telling phylink EEE is supported if EEE is actually supported. > +static int rtl_init_phylink(struct rtl8169_private *tp) > +{ > + struct phylink *pl; > + phy_interface_t phy_mode; > + > + tp->phylink_config.dev = &tp->dev->dev; > + tp->phylink_config.type = PHYLINK_NETDEV; > + tp->phylink_config.mac_managed_pm = true; > + tp->phylink_config.lpi_capabilities = rtl8169_get_lpi_caps(tp); > + tp->phylink_config.mac_capabilities |= MAC_ASYM_PAUSE | MAC_SYM_PAUSE; > + > + if (tp->sfp_mode) { > + phy_mode = PHY_INTERFACE_MODE_INTERNAL; > + tp->phylink_config.mac_capabilities |= MAC_10000FD; Only 10G? Is it not possible to slow down to 1G for a 1G SFP? Andrew