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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html