* [PATCH net] net: phy: smsc: add missed clk_disable_unprepare in smsc_phy_probe()
@ 2020-11-12 11:23 Zhang Changzhong
2020-11-14 19:26 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Zhang Changzhong @ 2020-11-12 11:23 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, kuba, m.felsch, f.fainelli
Cc: netdev, linux-kernel
Add the missing clk_disable_unprepare() before return from
smsc_phy_probe() in the error handling case.
Fixes: bedd8d78aba3 ("net: phy: smsc: LAN8710/20: add phy refclk in support")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com>
---
drivers/net/phy/smsc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index ec97669..0fc39ac 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -291,8 +291,10 @@ static int smsc_phy_probe(struct phy_device *phydev)
return ret;
ret = clk_set_rate(priv->refclk, 50 * 1000 * 1000);
- if (ret)
+ if (ret) {
+ clk_disable_unprepare(priv->refclk);
return ret;
+ }
return 0;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] net: phy: smsc: add missed clk_disable_unprepare in smsc_phy_probe() 2020-11-12 11:23 [PATCH net] net: phy: smsc: add missed clk_disable_unprepare in smsc_phy_probe() Zhang Changzhong @ 2020-11-14 19:26 ` Jakub Kicinski 2020-11-14 19:45 ` Florian Fainelli 0 siblings, 1 reply; 5+ messages in thread From: Jakub Kicinski @ 2020-11-14 19:26 UTC (permalink / raw) To: Zhang Changzhong Cc: andrew, hkallweit1, linux, davem, m.felsch, f.fainelli, netdev, linux-kernel On Thu, 12 Nov 2020 19:23:59 +0800 Zhang Changzhong wrote: > Add the missing clk_disable_unprepare() before return from > smsc_phy_probe() in the error handling case. > > Fixes: bedd8d78aba3 ("net: phy: smsc: LAN8710/20: add phy refclk in support") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com> > --- > drivers/net/phy/smsc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c > index ec97669..0fc39ac 100644 > --- a/drivers/net/phy/smsc.c > +++ b/drivers/net/phy/smsc.c > @@ -291,8 +291,10 @@ static int smsc_phy_probe(struct phy_device *phydev) > return ret; > > ret = clk_set_rate(priv->refclk, 50 * 1000 * 1000); > - if (ret) > + if (ret) { > + clk_disable_unprepare(priv->refclk); > return ret; > + } > > return 0; > } Applied, thanks! The code right above looks highly questionable as well: priv->refclk = clk_get_optional(dev, NULL); if (IS_ERR(priv->refclk)) dev_err_probe(dev, PTR_ERR(priv->refclk), "Failed to request clock\n"); ret = clk_prepare_enable(priv->refclk); if (ret) return ret; I don't think clk_prepare_enable() will be too happy to see an error pointer. This should probably be: priv->refclk = clk_get_optional(dev, NULL); if (IS_ERR(priv->refclk)) return dev_err_probe(dev, PTR_ERR(priv->refclk), "Failed to request clock\n"); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: phy: smsc: add missed clk_disable_unprepare in smsc_phy_probe() 2020-11-14 19:26 ` Jakub Kicinski @ 2020-11-14 19:45 ` Florian Fainelli 2020-11-16 9:26 ` Marco Felsch 0 siblings, 1 reply; 5+ messages in thread From: Florian Fainelli @ 2020-11-14 19:45 UTC (permalink / raw) To: Jakub Kicinski, Zhang Changzhong Cc: andrew, hkallweit1, linux, davem, m.felsch, netdev, linux-kernel On 11/14/2020 11:26 AM, Jakub Kicinski wrote: > On Thu, 12 Nov 2020 19:23:59 +0800 Zhang Changzhong wrote: >> Add the missing clk_disable_unprepare() before return from >> smsc_phy_probe() in the error handling case. >> >> Fixes: bedd8d78aba3 ("net: phy: smsc: LAN8710/20: add phy refclk in support") >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com> >> --- >> drivers/net/phy/smsc.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c >> index ec97669..0fc39ac 100644 >> --- a/drivers/net/phy/smsc.c >> +++ b/drivers/net/phy/smsc.c >> @@ -291,8 +291,10 @@ static int smsc_phy_probe(struct phy_device *phydev) >> return ret; >> >> ret = clk_set_rate(priv->refclk, 50 * 1000 * 1000); >> - if (ret) >> + if (ret) { >> + clk_disable_unprepare(priv->refclk); >> return ret; >> + } >> >> return 0; >> } > > Applied, thanks! > > The code right above looks highly questionable as well: > > priv->refclk = clk_get_optional(dev, NULL); > if (IS_ERR(priv->refclk)) > dev_err_probe(dev, PTR_ERR(priv->refclk), "Failed to request clock\n"); > > ret = clk_prepare_enable(priv->refclk); > if (ret) > return ret; > > I don't think clk_prepare_enable() will be too happy to see an error > pointer. This should probably be: > > priv->refclk = clk_get_optional(dev, NULL); > if (IS_ERR(priv->refclk)) > return dev_err_probe(dev, PTR_ERR(priv->refclk), > "Failed to request clock\n"); Right, especially if EPROBE_DEFER must be returned because the clock provider is not ready yet, we should have a chance to do that. -- Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: phy: smsc: add missed clk_disable_unprepare in smsc_phy_probe() 2020-11-14 19:45 ` Florian Fainelli @ 2020-11-16 9:26 ` Marco Felsch 2020-11-16 17:45 ` Jakub Kicinski 0 siblings, 1 reply; 5+ messages in thread From: Marco Felsch @ 2020-11-16 9:26 UTC (permalink / raw) To: Florian Fainelli Cc: Jakub Kicinski, Zhang Changzhong, andrew, hkallweit1, linux, davem, netdev, linux-kernel On 20-11-14 11:45, Florian Fainelli wrote: > > > On 11/14/2020 11:26 AM, Jakub Kicinski wrote: > > On Thu, 12 Nov 2020 19:23:59 +0800 Zhang Changzhong wrote: > >> Add the missing clk_disable_unprepare() before return from > >> smsc_phy_probe() in the error handling case. > >> > >> Fixes: bedd8d78aba3 ("net: phy: smsc: LAN8710/20: add phy refclk in support") > >> Reported-by: Hulk Robot <hulkci@huawei.com> > >> Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com> > >> --- > >> drivers/net/phy/smsc.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c > >> index ec97669..0fc39ac 100644 > >> --- a/drivers/net/phy/smsc.c > >> +++ b/drivers/net/phy/smsc.c > >> @@ -291,8 +291,10 @@ static int smsc_phy_probe(struct phy_device *phydev) > >> return ret; > >> > >> ret = clk_set_rate(priv->refclk, 50 * 1000 * 1000); > >> - if (ret) > >> + if (ret) { > >> + clk_disable_unprepare(priv->refclk); > >> return ret; > >> + } > >> > >> return 0; > >> } > > > > Applied, thanks! > > > > The code right above looks highly questionable as well: > > > > priv->refclk = clk_get_optional(dev, NULL); > > if (IS_ERR(priv->refclk)) > > dev_err_probe(dev, PTR_ERR(priv->refclk), "Failed to request clock\n"); > > > > ret = clk_prepare_enable(priv->refclk); > > if (ret) > > return ret; > > > > I don't think clk_prepare_enable() will be too happy to see an error > > pointer. This should probably be: > > > > priv->refclk = clk_get_optional(dev, NULL); > > if (IS_ERR(priv->refclk)) > > return dev_err_probe(dev, PTR_ERR(priv->refclk), > > "Failed to request clock\n"); > > Right, especially if EPROBE_DEFER must be returned because the clock > provider is not ready yet, we should have a chance to do that. Hi, damn.. I missed the return here. Thanks for covering that. Should I send a fix or did you do that already? Regards, Marco ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: phy: smsc: add missed clk_disable_unprepare in smsc_phy_probe() 2020-11-16 9:26 ` Marco Felsch @ 2020-11-16 17:45 ` Jakub Kicinski 0 siblings, 0 replies; 5+ messages in thread From: Jakub Kicinski @ 2020-11-16 17:45 UTC (permalink / raw) To: Marco Felsch Cc: Florian Fainelli, Zhang Changzhong, andrew, hkallweit1, linux, davem, netdev, linux-kernel On Mon, 16 Nov 2020 10:26:07 +0100 Marco Felsch wrote: > > > The code right above looks highly questionable as well: > > > > > > priv->refclk = clk_get_optional(dev, NULL); > > > if (IS_ERR(priv->refclk)) > > > dev_err_probe(dev, PTR_ERR(priv->refclk), "Failed to request clock\n"); > > > > > > ret = clk_prepare_enable(priv->refclk); > > > if (ret) > > > return ret; > > > > > > I don't think clk_prepare_enable() will be too happy to see an error > > > pointer. This should probably be: > > > > > > priv->refclk = clk_get_optional(dev, NULL); > > > if (IS_ERR(priv->refclk)) > > > return dev_err_probe(dev, PTR_ERR(priv->refclk), > > > "Failed to request clock\n"); > > > > Right, especially if EPROBE_DEFER must be returned because the clock > > provider is not ready yet, we should have a chance to do that. > > damn.. I missed the return here. Thanks for covering that. Should I send > a fix or did you do that already? Please do, I don't see any fix for this issue in patchwork right now. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-16 17:46 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-12 11:23 [PATCH net] net: phy: smsc: add missed clk_disable_unprepare in smsc_phy_probe() Zhang Changzhong 2020-11-14 19:26 ` Jakub Kicinski 2020-11-14 19:45 ` Florian Fainelli 2020-11-16 9:26 ` Marco Felsch 2020-11-16 17:45 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).