From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753680AbdCNBHI (ORCPT ); Mon, 13 Mar 2017 21:07:08 -0400 Received: from vps0.lunn.ch ([178.209.37.122]:46865 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752113AbdCNBHH (ORCPT ); Mon, 13 Mar 2017 21:07:07 -0400 Date: Tue, 14 Mar 2017 02:06:51 +0100 From: Andrew Lunn To: Doug Berger Cc: f.fainelli@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com, davem@davemloft.net, rafal@milecki.pl, xow@google.com, joel@jms.id.au, jon.mason@broadcom.com, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, pgynther@google.com, jaedon.shin@gmail.com Subject: Re: [PATCH net-next 02/12] net: phy: bcm7xxx: add support for 28nm EPHY Message-ID: <20170314010651.GO15842@lunn.ch> References: <20170314004142.4746-1-opendmb@gmail.com> <20170314004142.4746-3-opendmb@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170314004142.4746-3-opendmb@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 13, 2017 at 05:41:32PM -0700, Doug Berger wrote: > +static int bcm7xxx_28nm_ephy_01_afe_config_init(struct phy_device *phydev) > +{ > + int ret; > + > + /* set shadow mode 2 */ > + ret = phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, > + MII_BCM7XXX_SHD_MODE_2, 0); > + if (ret < 0) > + return ret; > + > + /* Set current trim values INT_trim = -1, Ext_trim =0 */ > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_BIAS_TRIM, 0x3BE0); > + if (ret < 0) > + goto reset_shadow_mode; > + > + /* Cal reset */ > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_ADDR_CTRL, > + MII_BCM7XXX_SHD_3_TL4); > + if (ret < 0) > + goto reset_shadow_mode; Hi Doug It would be nice to have a few blank lines here and there... > + ret = phy_set_clr_bits(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT, > + MII_BCM7XXX_TL4_RST_MSK, 0); > + if (ret < 0) > + goto reset_shadow_mode; > + > + /* Cal reset disable */ > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_ADDR_CTRL, > + MII_BCM7XXX_SHD_3_TL4); > + if (ret < 0) > + goto reset_shadow_mode; ... just to break things up into readable chunks. > + ret = phy_set_clr_bits(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT, > + 0, MII_BCM7XXX_TL4_RST_MSK); > + if (ret < 0) > + goto reset_shadow_mode; > + > +reset_shadow_mode: > + /* reset shadow mode 2 */ > + ret = phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, 0, > + MII_BCM7XXX_SHD_MODE_2); > + if (ret < 0) > + return ret; > + > + return 0; How about: return phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, 0, MII_BCM7XXX_SHD_MODE_2); > +/* The 28nm EPHY does not support Clause 45 (MMD) used by bcm-phy-lib */ > +static int bcm7xxx_28nm_ephy_apd_enable(struct phy_device *phydev) > +{ > + int ret; > + > + /* set shadow mode 1 */ > + ret = phy_set_clr_bits(phydev, MII_BRCM_FET_BRCMTEST, > + MII_BRCM_FET_BT_SRE, 0); > + if (ret < 0) > + return ret; > + > + /* Enable auto-power down */ > + ret = phy_set_clr_bits(phydev, MII_BRCM_FET_SHDW_AUXSTAT2, > + MII_BRCM_FET_SHDW_AS2_APDE, 0); > + if (ret < 0) > + return ret; > + > + /* reset shadow mode 1 */ > + ret = phy_set_clr_bits(phydev, MII_BRCM_FET_BRCMTEST, 0, > + MII_BRCM_FET_BT_SRE); > + if (ret < 0) > + return ret; > + > + return 0; How about just return phy_set_clr_bits(phydev, MII_BRCM_FET_BRCMTEST, 0, MII_BRCM_FET_BT_SRE); > +} > + > +static int bcm7xxx_28nm_ephy_eee_enable(struct phy_device *phydev) > +{ > + int ret; > + > + /* set shadow mode 2 */ > + ret = phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, > + MII_BCM7XXX_SHD_MODE_2, 0); > + if (ret < 0) > + return ret; > + > + /* Advertise supported modes */ > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_ADDR_CTRL, > + MII_BCM7XXX_SHD_3_AN_EEE_ADV); > + if (ret < 0) > + goto reset_shadow_mode; blank... > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT, > + MDIO_EEE_100TX); > + if (ret < 0) > + goto reset_shadow_mode; > + > + /* Restore Defaults */ > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_ADDR_CTRL, > + MII_BCM7XXX_SHD_3_PCS_CTRL_2); > + if (ret < 0) > + goto reset_shadow_mode; > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT, > + MII_BCM7XXX_PCS_CTRL_2_DEF); > + if (ret < 0) > + goto reset_shadow_mode; > + > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_ADDR_CTRL, > + MII_BCM7XXX_SHD_3_EEE_THRESH); > + if (ret < 0) > + goto reset_shadow_mode; > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT, > + MII_BCM7XXX_EEE_THRESH_DEF); > + if (ret < 0) > + goto reset_shadow_mode; > + > + /* Enable EEE autonegotiation */ > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_ADDR_CTRL, > + MII_BCM7XXX_SHD_3_AN_STAT); > + if (ret < 0) > + goto reset_shadow_mode; > + ret = phy_write(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT, > + (MII_BCM7XXX_AN_NULL_MSG_EN | MII_BCM7XXX_AN_EEE_EN)); > + if (ret < 0) > + goto reset_shadow_mode; > + > +reset_shadow_mode: > + /* reset shadow mode 2 */ > + ret = phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, 0, > + MII_BCM7XXX_SHD_MODE_2); > + if (ret < 0) > + return ret; > + > + /* Restart autoneg */ > + phy_write(phydev, MII_BMCR, > + (BMCR_SPEED100 | BMCR_ANENABLE | BMCR_ANRESTART)); > + > + return 0; return phy_write(.....); ? > +} Andrew