From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2AA45CCA479 for ; Sun, 17 Jul 2022 01:39:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232684AbiGQBjw (ORCPT ); Sat, 16 Jul 2022 21:39:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46852 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232621AbiGQBjv (ORCPT ); Sat, 16 Jul 2022 21:39:51 -0400 Received: from vps0.lunn.ch (vps0.lunn.ch [185.16.172.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 502C61AD9E; Sat, 16 Jul 2022 18:39:50 -0700 (PDT) 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=vHboTQVyDPjyOiJ7fhjhnLV2AN9SVlX5UirCaUIKjro=; b=flUpDHGrs8Da2Zxp4G8Z4nuLqx AMJ2U5n+ifO9ta3oX36I2yuEzpAXsPxy57F5P6mEA5xWHP8ptjACCyqPr/gYslTtc4GKPRfqnxG/J pqY2w6y4HVLV+ta47zE0TKloBZvBSfoDbb72P+MFmdz/Xvu+niBre44yGv8tGHGj15xI=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1oCtFz-00Aanj-Ju; Sun, 17 Jul 2022 03:39:39 +0200 Date: Sun, 17 Jul 2022 03:39:39 +0200 From: Andrew Lunn To: Sean Anderson Cc: "David S . Miller" , Jakub Kicinski , Madalin Bucur , netdev@vger.kernel.org, Paolo Abeni , Eric Dumazet , linux-arm-kernel@lists.infradead.org, Russell King , linux-kernel@vger.kernel.org, Alexandru Marginean , Heiner Kallweit , Vladimir Oltean Subject: Re: [PATCH net-next v3 10/47] net: phylink: Adjust link settings based on rate adaptation Message-ID: References: <20220715215954.1449214-1-sean.anderson@seco.com> <20220715215954.1449214-11-sean.anderson@seco.com> <4172fd87-8e51-e67d-bf86-fdc6829fa9b3@seco.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4172fd87-8e51-e67d-bf86-fdc6829fa9b3@seco.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > I would not do this. If the requirements for rate adaptation are not > > fulfilled, you should turn off rate adaptation. > > > > A MAC which knows rate adaptation is going on can help out, by not > > advertising 10Half, 100Half etc. Autoneg will then fail for modes > > where rate adaptation does not work. > > OK, so maybe it is better to phylink_warn here. Something along the > lines of "phy using pause-based rate adaptation, but duplex is %s". You say 1/2 duplex simply does not work with rate adaptation. So i would actually return -EINVAL at the point the MAC indicates what modes it supports if there is a 1/2 duplex mode in the list. > > > The MAC should also be declaring what sort of pause it supports, so > > disable rate adaptation if it does not have async pause. > > That's what we do in the previous patch. > > The problem is that rx_pause and tx_pause are resolved based on our > advertisement and the link partner's advertisement. However, the link > partner may not support pause frames at all. In that case, we will get > rx_pause and tx_pause as false. However, we still want to enable rx_pause, > because we know that the phy will be emitting pause frames. And of course > the user can always force disable pause frames anyway through ethtool. Right, so we need a table somewhere in the documentation listing the different combinations and what should happen. If the MAC does not support rx_pause, rate adaptation is turned off. If the negotiation results in no rx_pause, force it on anyway with Pause based adaptation. If ethtool turns pause off, turn off rate adaptation. Does 802.3 say anything about this? We might also want to add an additional state to the ethtool get for pause, to indicate rx_pause is enabled because of rate adaptation, not because of autoneg. Andrew