Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1596358 > unrolled thread

Re: [PATCH 2/7] net: stmmac: Balance PTP reference clock enable/disable

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/7] net: stmmac: Balance PTP reference clock  enable/disable Thierry Reding <thierry.reding@gmail.com> - 2017-03-09 20:40 +0100

#1596358 — Re: [PATCH 2/7] net: stmmac: Balance PTP reference clock enable/disable

[Multipart message — attachments visible in raw view] — view raw

On Mon, Feb 27, 2017 at 11:31:39AM +0200, Mikko Perttunen wrote:
> On 23.02.2017 19:24, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > clk_prepare_enable() and clk_disable_unprepare() for this clock aren't
> > properly balanced, which can trigger a WARN_ON() in the common clock
> > framework.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 3cbe09682afe..6b7a5ce19589 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -1711,6 +1711,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
> >  	stmmac_mmc_setup(priv);
> > 
> >  	if (init_ptp) {
> > +		ret = clk_prepare_enable(priv->plat->clk_ptp_ref);
> > +		if (ret < 0)
> > +			netdev_warn(priv->dev, "failed to enable PTP reference clock: %d\n", ret);
> > +
> 
> Should we return an error code if the clock enable fails?

Yeah, that's probably a good idea.

> >  		ret = stmmac_init_ptp(priv);
> >  		if (ret == -EOPNOTSUPP)
> >  			netdev_warn(priv->dev, "PTP not supported by HW\n");
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index 5b18355c0d2b..d285d6cfbd0d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -365,7 +365,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> >  		plat->clk_ptp_ref = NULL;
> >  		dev_warn(&pdev->dev, "PTP uses main clock\n");
> >  	} else {
> > -		clk_prepare_enable(plat->clk_ptp_ref);
> >  		plat->clk_ptp_rate = clk_get_rate(plat->clk_ptp_ref);
> >  		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
> >  	}
> > 
> 
> It seems like there will still be a refcount mismatch for the clock if any
> of the request_irqs that are after stmmac_hw_setup in stmmac_open fail.

Looks like there's a few more things that could be cleaned up on failure
to request those interrupts. I've added another patch to the series that
will attempt to do this.

Thierry

[toc] | [standalone]

Back to top | Article view | linux.kernel

csiph-web