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 00682C6379F for ; Fri, 17 Feb 2023 14:26:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229823AbjBQO0O (ORCPT ); Fri, 17 Feb 2023 09:26:14 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55504 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229762AbjBQO0N (ORCPT ); Fri, 17 Feb 2023 09:26:13 -0500 Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:32c8:5054:ff:fe00:142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2FC37E384 for ; Fri, 17 Feb 2023 06:26:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=iFTFSjUDEc09Dg+zzleu8t2Pvc3oX+TINsQ4PoDARnM=; b=i+k9sgMvno2vjlXYHrJEpwOvhS sXtp8Reh0RpQ4oq+wOywpTf8pQ24RQi5UU0YOcbfYJT+pQtBbdKGvO8JQM8mPDRZ7qo8VAR/br2Si NAQAOse3I0XwR4MUTQ8DuAwtM1pAnw2GSV+P6oXxPiiHzdt0G99cq+/P7tvFKrBUDRp1ZJlxgV4ws QDmP9s1jkwXWmLg6obOf5oOa1f0sbFWTJaYvqk6kYsRlr5TwEhQ4sm13pqEeJunsldkJlZTeMQPkR n8yDK/kxTc9ZVOJQAa48v2uUzQ0787ZdPHjx4fyvgNgDBhFzMkb3dAZzDokR9fmiCTow1Z3dSV2eQ s5RnZbYA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:49278) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1pT1gZ-00016C-HA; Fri, 17 Feb 2023 14:26:03 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1pT1gT-0006sL-7P; Fri, 17 Feb 2023 14:25:57 +0000 Date: Fri, 17 Feb 2023 14:25:57 +0000 From: "Russell King (Oracle)" To: Andrew Lunn Cc: netdev , Florian Fainelli , Vladimir Oltean , Sean Wang , Landen Chao , DENG Qingfang , Matthias Brugger , AngeloGioacchino Del Regno , Doug Berger , Broadcom internal kernel review list , Wei Fang , Shenwei Wang , Clark Wang , NXP Linux Team , UNGLinuxDriver@microchip.com, Byungho An , Giuseppe Cavallaro , Alexandre Torgue , Jose Abreu , Maxime Coquelin , Heiner Kallweit , Woojung Huh , Oleksij Rempel Subject: Re: [PATCH RFC 00/18] Rework MAC drivers EEE support Message-ID: References: <20230217034230.1249661-1-andrew@lunn.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230217034230.1249661-1-andrew@lunn.ch> Sender: Russell King (Oracle) Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Feb 17, 2023 at 04:42:12AM +0100, Andrew Lunn wrote: > phy_init_eee() is supposed to be called once auto-neg has been > completed to determine if EEE should be used with the current link > mode. The MAC hardware should then be configured to either enable or > disable EEE. Many drivers get this wrong, calling phy_init_eee() once, > or only in the ethtool set_eee callback. Looking at some of the recent EEE changes (not related to this patch set) I've come across: commit 9b01c885be364526d8c05794f8358b3e563b7ff8 Author: Oleksij Rempel Date: Sat Feb 11 08:41:10 2023 +0100 net: phy: c22: migrate to genphy_c45_write_eee_adv() This part of the patch is wrong: __genphy_config_aneg(): - if (genphy_config_eee_advert(phydev)) + err = genphy_c45_write_eee_adv(phydev, phydev->supported_eee); The problem here is that these are not equivalent. genphy_config_eee_advert() only clears the broken EEE modes in the advertisement, it doesn't actually set the advertisement to anything in particular. The replacement code _configures_ the advertisement to whatever the second argument is, which means each time the advertisement is changed (and thus __genphy_config_aneg() is called) the EEE advertisement will ignore whatever the user configured via the set_eee() APIs, and be restored to the full EEE capabilities in the supported mask. This is an obvious regression that needs fixing, especially as the merge window is potentially due to open this weekend. Moreover, it looks like Oleksij's patch was not Cc'd to me (I can't find it in my mailbox) and as I'm listed in MAINTAINERS for phylib, this should have been brought up _before_ Oleksij's patch was applied to net-next (despite me being unlikely to reply to it due to covid, it still would be nice to have reviewed it, or even reply to the damn patch about these concerns.) But I'm having to pick some other damn series to bring up this concern. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!