[PATCH net v2 2/2] net: stmmac: skip only stmmac_ptp_register when resume from suspend

[Ismail, Mohammad Athari]

> -----Original Message-----
> From: Jakub Kicinski <kuba at kernel.org>
> Sent: Wednesday, January 26, 2022 12:18 PM
> To: Ismail, Mohammad Athari <mohammad.athari.ismail at intel.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro at st.com>; Alexandre Torgue
> <alexandre.torgue at st.com>; Jose Abreu <joabreu at synopsys.com>; David
> S . Miller <davem at davemloft.net>; Maxime Coquelin
> <mcoquelin.stm32 at gmail.com>; Ong, Boon Leong
> <boon.leong.ong at intel.com>; Voon, Weifeng <weifeng.voon at intel.com>;
> Wong, Vee Khee <vee.khee.wong at intel.com>; Huacai Chen
> <chenhuacai at kernel.org>; Alexandre Torgue
> <alexandre.torgue at foss.st.com>; netdev at vger.kernel.org; linux-stm32 at st-
> md-mailman.stormreply.com; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; stable at vger.kernel.org
> Subject: Re: [PATCH net v2 2/2] net: stmmac: skip only stmmac_ptp_register
> when resume from suspend
> 
> On Tue, 25 Jan 2022 11:23:24 +0800 Mohammad Athari Bin Ismail wrote:
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index d7e261768f73..b8e5e19e6f7b 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -880,11 +880,12 @@
> EXPORT_SYMBOL_GPL(stmmac_init_tstamp_counter);
> >  /**
> >   * stmmac_init_ptp - init PTP
> >   * @priv: driver private structure
> > + * @ptp_register: register PTP if set
> >   * Description: this is to verify if the HW supports the PTPv1 or PTPv2.
> >   * This is done by looking at the HW cap. register.
> >   * This function also registers the ptp driver.
> >   */
> > -static int stmmac_init_ptp(struct stmmac_priv *priv)
> > +static int stmmac_init_ptp(struct stmmac_priv *priv, bool
> > +ptp_register)
> >  {
> >  	bool xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
> >  	int ret;
> > @@ -914,7 +915,8 @@ static int stmmac_init_ptp(struct stmmac_priv
> *priv)
> >  	priv->hwts_tx_en = 0;
> >  	priv->hwts_rx_en = 0;
> >
> > -	stmmac_ptp_register(priv);
> > +	if (ptp_register)
> > +		stmmac_ptp_register(priv);
> 
> stmmac_init_ptp() only has one caller, and the registration step is last.
> Wouldn't it be better to move the stmmac_ptp_register() call out to
> stmmac_hw_setup()? That way we don't need to pass extra arguments to
> init.

Yes agree with you suggestion. Will fix it in v3. Thanks.

-Athari-

> 
> >  	return 0;
> >  }
> > @@ -3241,7 +3243,7 @@ static int stmmac_fpe_start_wq(struct
> > stmmac_priv *priv)
> >  /**
> >   * stmmac_hw_setup - setup mac in a usable state.
> >   *  @dev : pointer to the device structure.
> > - *  @init_ptp: initialize PTP if set
> > + *  @ptp_register: register PTP if set
> >   *  Description:
> >   *  this is the main function to setup the HW in a usable state because the
> >   *  dma engine is reset, the core registers are configured (e.g. AXI,
> > @@ -3251,7 +3253,7 @@ static int stmmac_fpe_start_wq(struct
> stmmac_priv *priv)
> >   *  0 on success and an appropriate (-)ve integer as defined in errno.h
> >   *  file on failure.
> >   */
> > -static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
> > +static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
> >  {
> >  	struct stmmac_priv *priv = netdev_priv(dev);
> >  	u32 rx_cnt = priv->plat->rx_queues_to_use; @@ -3308,13 +3310,11
> @@
> > static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
> >
> >  	stmmac_mmc_setup(priv);
> >
> > -	if (init_ptp) {
> > -		ret = stmmac_init_ptp(priv);
> > -		if (ret == -EOPNOTSUPP)
> > -			netdev_warn(priv->dev, "PTP not supported by
> HW\n");
> > -		else if (ret)
> > -			netdev_warn(priv->dev, "PTP init failed\n");
> > -	}
> > +	ret = stmmac_init_ptp(priv, ptp_register);
> > +	if (ret == -EOPNOTSUPP)
> > +		netdev_warn(priv->dev, "PTP not supported by HW\n");
> > +	else if (ret)
> > +		netdev_warn(priv->dev, "PTP init failed\n");