Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S943172AbcJSOrr (ORCPT ); Wed, 19 Oct 2016 10:47:47 -0400 Received: from mail-qk0-f177.google.com ([209.85.220.177]:33288 "EHLO mail-qk0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932654AbcJSOro (ORCPT ); Wed, 19 Oct 2016 10:47:44 -0400 Date: Wed, 19 Oct 2016 10:47:42 -0400 (EDT) From: Nicolas Pitre To: Giuseppe Cavallaro cc: netdev@vger.kernel.org, davem@davemloft.net, linux-kernel@vger.kernel.org, Alexandre TORGUE , Rayagond Kokatanur , Dan Carpenter , Richard Cochran Subject: Re: [PATCH (net.git)] stmmac: fix and review the ptp registration. In-Reply-To: <1476860801-28242-1-git-send-email-peppe.cavallaro@st.com> Message-ID: References: <1476860801-28242-1-git-send-email-peppe.cavallaro@st.com> User-Agent: Alpine 2.20 (LFD 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3912 Lines: 103 [ added Richard to CC ] On Wed, 19 Oct 2016, Giuseppe Cavallaro wrote: > The commit commit 7086605a6ab5 ("stmmac: fix error check when init ptp") > breaks the procedure added by the > commit efee95f42b5d ("ptp_clock: future-proofing drivers against PTP > subsystem becoming optional") > > So this patch tries to re-import the logic added by the latest > commit above: it makes sense to have the stmmac_ptp_register > as void function and, inside the main, the stmmac_init_ptp can fails > in case of the capability cannot be supported by the HW. > > Signed-off-by: Giuseppe Cavallaro > Cc: Alexandre TORGUE > Cc: Rayagond Kokatanur > Cc: Dan Carpenter > Cc: Nicolas Pitre Acked-by: Nicolas Pitre > --- > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++-- > drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 15 ++++----------- > 3 files changed, 9 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h > index 8dc9056..b15fc55 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h > @@ -145,7 +145,7 @@ int stmmac_mdio_register(struct net_device *ndev); > int stmmac_mdio_reset(struct mii_bus *mii); > void stmmac_set_ethtool_ops(struct net_device *netdev); > > -int stmmac_ptp_register(struct stmmac_priv *priv); > +void stmmac_ptp_register(struct stmmac_priv *priv); > void stmmac_ptp_unregister(struct stmmac_priv *priv); > int stmmac_resume(struct device *dev); > int stmmac_suspend(struct device *dev); > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 6c85b61..48e71fa 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -676,7 +676,9 @@ static int stmmac_init_ptp(struct stmmac_priv *priv) > priv->hwts_tx_en = 0; > priv->hwts_rx_en = 0; > > - return stmmac_ptp_register(priv); > + stmmac_ptp_register(priv); > + > + return 0; > } > > static void stmmac_release_ptp(struct stmmac_priv *priv) > @@ -1710,7 +1712,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp) > if (init_ptp) { > ret = stmmac_init_ptp(priv); > if (ret) > - netdev_warn(priv->dev, "PTP support cannot init.\n"); > + netdev_warn(priv->dev, "fail to init PTP.\n"); > } > > #ifdef CONFIG_DEBUG_FS > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c > index 5d61fb2..1477471 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c > @@ -177,7 +177,7 @@ static struct ptp_clock_info stmmac_ptp_clock_ops = { > * Description: this function will register the ptp clock driver > * to kernel. It also does some house keeping work. > */ > -int stmmac_ptp_register(struct stmmac_priv *priv) > +void stmmac_ptp_register(struct stmmac_priv *priv) > { > spin_lock_init(&priv->ptp_lock); > priv->ptp_clock_ops = stmmac_ptp_clock_ops; > @@ -185,17 +185,10 @@ int stmmac_ptp_register(struct stmmac_priv *priv) > priv->ptp_clock = ptp_clock_register(&priv->ptp_clock_ops, > priv->device); > if (IS_ERR(priv->ptp_clock)) { > - int ret = PTR_ERR(priv->ptp_clock); > - > + netdev_err(priv->dev, "ptp_clock_register failed\n"); > priv->ptp_clock = NULL; > - return ret; > - } > - > - spin_lock_init(&priv->ptp_lock); > - > - netdev_dbg(priv->dev, "Added PTP HW clock successfully\n"); > - > - return 0; > + } else if (priv->ptp_clock) > + netdev_info(priv->dev, "registered PTP clock\n"); > } > > /** > -- > 2.7.4 > >