Received: by 2002:a05:6a10:f347:0:0:0:0 with SMTP id d7csp3410299pxu; Mon, 30 Nov 2020 02:28:27 -0800 (PST) X-Google-Smtp-Source: ABdhPJzu6uY0I9v2Q1/U5PccaPoAcLhqILizbLssBbbau4u6bDBVrZe9p5ndALDb+izAmUIaUBNE X-Received: by 2002:a50:cfcf:: with SMTP id i15mr20876928edk.351.1606732107737; Mon, 30 Nov 2020 02:28:27 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1606732107; cv=none; d=google.com; s=arc-20160816; b=lXLkBV5w/oyMNq+jCocHZuPTJAHJiKNLtlVi1YheEIsQJoastw9jG0hu6FoxCDwfDo kzUmS0xIeD8FoH0eflGfCOLGQRfu3ND7ecCfkh2uiwJhm7aXWCj+mpoqzG5YpzhPSOGn zjBkbgG+mO8DzRDgIah7j3Ud8yH+xgTvPirfVTStNuwHUp5fjxA9ZSxBbwitfRByAbBf 1TMYPWgvjnMSEW4mMI3uwo3Vsdh+blD1cFxzVy8PNBkyiws3UzsbDnTzu3ZNPTAx2F6E JnFY4414JatofoF6uIJ0g6iJ/ILmlW9ewW3luVJrsl6byNJyGkDQKB2n/Vi/QQEy1AX0 oHNA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=vEW1++ZGtkTahDMwrYyBH6nvIhUYOGLhVMOksh4ZP+I=; b=CZ0yTK2oXfV4/OlxUoz723UhLSzdCrq67muhNJkgkypvPYN/Cms8p2y9A6iZaugLd8 C563F9K25MnqJCd3PRYHl7r+9u+4HmXXfCD7cQRs5yVIPP6pbC96ierl9M62C/5hiN1w alnRRE+gCUnJokZ0aupAgAw/P83QQStIun0FdT5EsCbQZXkx+5TZkFMyI2KyNkF3A/B/ gljofqq7RY4wpbX/ePtkNkXkEDlQCq/xIsHDU3WeSEUAAdazi0/0TQX6QgjUgHp1621j qwsBVZRI2bbAeMP4XhfD+E+80PsQ9YC21vnFvovkC14HuOb7enLEPq2YEPUgZardKOAG pxWg== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail (test mode) header.i=@armlinux.org.uk header.s=pandora-2019 header.b=mQTfXfm4; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=armlinux.org.uk Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r20si9574399edq.561.2020.11.30.02.28.05; Mon, 30 Nov 2020 02:28:27 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=fail (test mode) header.i=@armlinux.org.uk header.s=pandora-2019 header.b=mQTfXfm4; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=armlinux.org.uk Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727713AbgK3K0l (ORCPT + 99 others); Mon, 30 Nov 2020 05:26:41 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37798 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726498AbgK3K0k (ORCPT ); Mon, 30 Nov 2020 05:26:40 -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 9FB9BC0613CF; Mon, 30 Nov 2020 02:26:00 -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=vEW1++ZGtkTahDMwrYyBH6nvIhUYOGLhVMOksh4ZP+I=; b=mQTfXfm4g4w+luNlPlFzp1/Ri EqaXltUc9BwMansy2Kg861bA+07iAU1sH5E8mWceS0ed/5IlK+OBfDaRLO6fkisGFbc3NbusPs+w1 35iux4THBzPdFVgQAvG1Z5IOjgR8DI2cUHoDl3pJoD7r1rRE0JD0kRpU12bLulgPxgNftbEdoKvA5 ul9E7CUIF8eL2Fg95KlZP16IUQG4ORg/Mxz8cIoGFKkL8WJHuzq5iUwHInVyw85X7h8x2zMVq/gkZ uePwUpFzk5IoSJUA6XrPbUKd8d/r/kOBiJzurPjNXY2vReS4gSpAI56jQRbUhUge3X6T4sgGnNA3H tTZoVp79Q==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:37950) by pandora.armlinux.org.uk with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kjgNZ-0006Zb-KR; Mon, 30 Nov 2020 10:25:57 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.92) (envelope-from ) id 1kjgNY-0004vj-CH; Mon, 30 Nov 2020 10:25:56 +0000 Date: Mon, 30 Nov 2020 10:25:56 +0000 From: Russell King - ARM Linux admin To: Yejune Deng Cc: andrew@lunn.ch, hkallweit1@gmail.com, davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] net: phy: marvell: replace __phy_modify() Message-ID: <20201130102556.GU1551@shell.armlinux.org.uk> References: <1606731399-8772-1-git-send-email-yejune.deng@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1606731399-8772-1-git-send-email-yejune.deng@gmail.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: Russell King - ARM Linux admin Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 30, 2020 at 06:16:39PM +0800, Yejune Deng wrote: > a set of phy_set_bits() looks more neater > > Signed-off-by: Yejune Deng Sorry, but NAK. You seem to be doing a mechanical code change without first understanding the code, as the patch shows no sign of an understanding of the difference between phy_modify() and __phy_modify(). This means you are introducing new bugs with this change. Please investigate the differences between the two variants of phy_modify() and post a more correct patch. Thanks. > --- > drivers/net/phy/marvell.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > index 587930a..f402e7f 100644 > --- a/drivers/net/phy/marvell.c > +++ b/drivers/net/phy/marvell.c > @@ -1132,8 +1132,8 @@ static int m88e1510_config_init(struct phy_device *phydev) > return err; > > /* PHY reset is necessary after changing MODE[2:0] */ > - err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1, 0, > - MII_88E1510_GEN_CTRL_REG_1_RESET); > + err = phy_set_bits(phydev, MII_88E1510_GEN_CTRL_REG_1, > + MII_88E1510_GEN_CTRL_REG_1_RESET); > if (err < 0) > return err; > > @@ -1725,7 +1725,7 @@ static int m88e1318_set_wol(struct phy_device *phydev, > __phy_read(phydev, MII_M1011_IEVENT); > > /* Enable the WOL interrupt */ > - err = __phy_modify(phydev, MII_88E1318S_PHY_CSIER, 0, > + err = phy_set_bits(phydev, MII_88E1318S_PHY_CSIER, > MII_88E1318S_PHY_CSIER_WOL_EIE); > if (err < 0) > goto error; > @@ -1735,10 +1735,10 @@ static int m88e1318_set_wol(struct phy_device *phydev, > goto error; > > /* Setup LED[2] as interrupt pin (active low) */ > - err = __phy_modify(phydev, MII_88E1318S_PHY_LED_TCR, > - MII_88E1318S_PHY_LED_TCR_FORCE_INT, > - MII_88E1318S_PHY_LED_TCR_INTn_ENABLE | > - MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW); > + err = phy_modify(phydev, MII_88E1318S_PHY_LED_TCR, > + MII_88E1318S_PHY_LED_TCR_FORCE_INT, > + MII_88E1318S_PHY_LED_TCR_INTn_ENABLE | > + MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW); > if (err < 0) > goto error; > > @@ -1764,7 +1764,7 @@ static int m88e1318_set_wol(struct phy_device *phydev, > goto error; > > /* Clear WOL status and enable magic packet matching */ > - err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, 0, > + err = phy_set_bits(phydev, MII_88E1318S_PHY_WOL_CTRL, > MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS | > MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE); > if (err < 0) > @@ -1775,9 +1775,9 @@ static int m88e1318_set_wol(struct phy_device *phydev, > goto error; > > /* Clear WOL status and disable magic packet matching */ > - err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, > - MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE, > - MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS); > + err = phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, > + MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE, > + MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS); > if (err < 0) > goto error; > } > @@ -1995,7 +1995,7 @@ static int marvell_cable_test_start_common(struct phy_device *phydev) > return bmsr; > > if (bmcr & BMCR_ANENABLE) { > - ret = phy_modify(phydev, MII_BMCR, BMCR_ANENABLE, 0); > + ret = phy_clear_bits(phydev, MII_BMCR, BMCR_ANENABLE); > if (ret < 0) > return ret; > ret = genphy_soft_reset(phydev); > -- > 1.9.1 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!