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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 18ADFC54EE9 for ; Mon, 19 Sep 2022 11:25:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=DuYUBEtaNxX8ykl6jSYwcV3+7+sX5brxR3OanllbrUk=; b=bf4GULYYDgT6un nGOhePTecDqOVggHPMq6Edcqa3MS7JLiUHN6W82arasO1PGO9WejodmqbuY0ZFhI68ZxyWFKn1RU4 yDPgE2vNxrbai3jQbIPvdIbW8xjeC3mgInlTRoX5dywhvw0kUWwM0/IC2aqSyzPa8ljmIyIssdfS0 6d5FI6AfKFa7eiqvGD353LOOxd8VedTht0JZAdW+J2IAWpFOPpcrdBxKdR5ZOho68fFKt4D7tH0AW WAJXob5gyrYVg+i3KqDTt8eIDQnCakVE91k7PdXEXlduCUWLc45xOyCvePrTq7PuTo6FQCfhQM1b7 93/qWcrrw6QdmZw829Xg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oaEst-00BUqI-T7; Mon, 19 Sep 2022 11:24:20 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oaEsp-00BUoY-D9; Mon, 19 Sep 2022 11:24:16 +0000 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=mQIM/EJij0VCL0/KLKnA4As/obLNORXFYtrVRXtkL68=; b=Uy73segPjVyktd8MoQvEluJ9Sh 8UMlXvBQXgcmxdqKPCvnG0AJB5XTJrqI6TkyZbqzLgvj3De5Z8SaAM66xp2jYDyFPEyh5IoLJBoyV YnFSkxf+7niuxzR2BJeS+2IUuG63xeDlCLVR6SXgLpKW2GK3p64cjCvnfjEBFSlbhVt5LwUBoty/0 17xzYlZme8iC9EEtdlDJLQ6nK5GSU+zmxYvhlmyPJiY/l4nLkPJKT6yULBWYHKzZ1YSxRkPJquYcW 16AZAqjaiImN+9RUOneXZF5KblFKbB4dTrNfXa1amvIj0XOknpHPV8q1ZVxCVKiFwppyWVpbxHQNp C+IMmD4g==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:34400) 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 1oaEsa-0000no-KQ; Mon, 19 Sep 2022 12:24:00 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1oaEsX-0006Lj-R3; Mon, 19 Sep 2022 12:23:57 +0100 Date: Mon, 19 Sep 2022 12:23:57 +0100 From: "Russell King (Oracle)" To: Alexander Couzens Cc: Felix Fietkau , John Crispin , Sean Wang , Mark Lee , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Matthias Brugger , Daniel Golle , netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next v2 5/5] net: mediatek: sgmii: refactor power cycling into mtk_pcs_config() Message-ID: References: <20220919083713.730512-1-lynxis@fe80.eu> <20220919083713.730512-6-lynxis@fe80.eu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220919083713.730512-6-lynxis@fe80.eu> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220919_042415_468197_766FFD3D X-CRM114-Status: GOOD ( 22.63 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Sep 19, 2022 at 10:37:12AM +0200, Alexander Couzens wrote: > Both code paths (autonegotiated and force mode) are power cycling > the phy. Move power cycling code to the caller to remove code > duplicity. I think we can do more consolidation here - and it probably makes sense to do in another patch. > diff --git a/drivers/net/ethernet/mediatek/mtk_sgmii.c b/drivers/net/ethernet/mediatek/mtk_sgmii.c > index 4c8e8c7b1d32..50f605208295 100644 > --- a/drivers/net/ethernet/mediatek/mtk_sgmii.c > +++ b/drivers/net/ethernet/mediatek/mtk_sgmii.c > @@ -25,9 +25,6 @@ static int mtk_pcs_setup_mode_an(struct mtk_pcs *mpcs, phy_interface_t interface > { > unsigned int val; > > - /* PHYA power down */ > - regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, SGMII_PHYA_PWD); > - > /* Set SGMII phy speed */ > regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val); > val &= ~RG_PHY_SPEED_MASK; > @@ -72,9 +57,6 @@ static int mtk_pcs_setup_mode_force(struct mtk_pcs *mpcs, > { > unsigned int val; > > - /* PHYA power down */ > - regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, SGMII_PHYA_PWD); > - > regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val); > val &= ~RG_PHY_SPEED_MASK; > if (interface == PHY_INTERFACE_MODE_2500BASEX) After powering the PHY down, the next thing that is done is to configure the speed. Even with my comments on patch 4, this can still be consolidated. > @@ -115,12 +85,27 @@ static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode, > struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs); unsigned int val; > int err = 0; > > + /* PHYA power down */ > + regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, SGMII_PHYA_PWD); > + regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val); val &= ~RG_PHY_SPEED_MASK; if (interface == PHY_INTERFACE_MODE_2500BASEX) val |= RG_PHY_SPEED_3_125G; regmap_write(mpcs->regmap, mpcs->ana_rgc3, val); which would make logical sense to do here, so we always configure the speed for the PCS correctly. That then leaves the configuration of SGMSYS_PCS_CONTROL_1 and SGMSYS_SGMII_MODE, which I think could also be consolidated, but I'll leave that to those with the hardware to make that decision. Reading between the lines of the code in this driver, it looks to me like this hardware supports only SGMII, but doesn't actually support 1000base-X and 2500base-X with negotiation. Is that correct? If so, it would be good to add a mtk_pcs_validate() function that clears ETHTOOL_LINK_MODE_Autoneg_BIT for these modes. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel