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 X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2AED5C433ED for ; Tue, 20 Apr 2021 11:05:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E81286135F for ; Tue, 20 Apr 2021 11:05:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231705AbhDTLFc (ORCPT ); Tue, 20 Apr 2021 07:05:32 -0400 Received: from perseus.uberspace.de ([95.143.172.134]:57011 "EHLO perseus.uberspace.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231651AbhDTLF3 (ORCPT ); Tue, 20 Apr 2021 07:05:29 -0400 Received: (qmail 15708 invoked from network); 20 Apr 2021 11:04:55 -0000 Received: from localhost (HELO localhost) (127.0.0.1) by perseus.uberspace.de with SMTP; 20 Apr 2021 11:04:55 -0000 Subject: Re: [PATCH net-next v2] net: phy: at803x: fix probe error if copper page is selected To: Michael Walle , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Andrew Lunn , Heiner Kallweit , Russell King , "David S . Miller" , Jakub Kicinski References: <20210420102929.13505-1-michael@walle.cc> From: David Bauer Message-ID: Date: Tue, 20 Apr 2021 13:04:53 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: <20210420102929.13505-1-michael@walle.cc> Content-Type: text/plain; charset=utf-8 Content-Language: de-DE Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi, On 4/20/21 12:29 PM, Michael Walle wrote: > The commit c329e5afb42f ("net: phy: at803x: select correct page on > config init") selects the copper page during probe. This fails if the > copper page was already selected. In this case, the value of the copper > page (which is 1) is propagated through phy_restore_page() and is > finally returned for at803x_probe(). Fix it, by just using the > at803x_page_write() directly. Ouch, i didn't spot that. Thanks for taking care. > > Also in case of an error, the regulator is not disabled and leads to a > WARN_ON() when the probe fails. This couldn't happen before, because > at803x_parse_dt() was the last call in at803x_probe(). It is hard to > see, that the parse_dt() actually enables the regulator. Thus move the > regulator_enable() to the probe function and undo it in case of an > error. > > Fixes: c329e5afb42f ("net: phy: at803x: select correct page on config init") > Signed-off-by: Michael Walle Reviewed-by: David Bauer Best David > --- > Changes since v1: > - take the bus lock > > drivers/net/phy/at803x.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c > index e0f56850edc5..32af52dd5aed 100644 > --- a/drivers/net/phy/at803x.c > +++ b/drivers/net/phy/at803x.c > @@ -554,10 +554,6 @@ static int at803x_parse_dt(struct phy_device *phydev) > phydev_err(phydev, "failed to get VDDIO regulator\n"); > return PTR_ERR(priv->vddio); > } > - > - ret = regulator_enable(priv->vddio); > - if (ret < 0) > - return ret; > } > > return 0; > @@ -579,15 +575,30 @@ static int at803x_probe(struct phy_device *phydev) > if (ret) > return ret; > > + if (priv->vddio) { > + ret = regulator_enable(priv->vddio); > + if (ret < 0) > + return ret; > + } > + > /* Some bootloaders leave the fiber page selected. > * Switch to the copper page, as otherwise we read > * the PHY capabilities from the fiber side. > */ > if (at803x_match_phy_id(phydev, ATH8031_PHY_ID)) { > - ret = phy_select_page(phydev, AT803X_PAGE_COPPER); > - ret = phy_restore_page(phydev, AT803X_PAGE_COPPER, ret); > + phy_lock_mdio_bus(phydev); > + ret = at803x_write_page(phydev, AT803X_PAGE_COPPER); > + phy_unlock_mdio_bus(phydev); > + if (ret) > + goto err; > } > > + return 0; > + > +err: > + if (priv->vddio) > + regulator_disable(priv->vddio); > + > return ret; > } > >