> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Wednesday, January 26, 2022 12:18 PM
> To: Ismail, Mohammad Athari <mohammad.athari.ismail@xxxxxxxxx>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@xxxxxx>; Alexandre Torgue
> <alexandre.torgue@xxxxxx>; Jose Abreu <joabreu@xxxxxxxxxxxx>; David
> S . Miller <davem@xxxxxxxxxxxxx>; Maxime Coquelin
> <mcoquelin.stm32@xxxxxxxxx>; Ong, Boon Leong
> <boon.leong.ong@xxxxxxxxx>; Voon, Weifeng <weifeng.voon@xxxxxxxxx>;
> Wong, Vee Khee <vee.khee.wong@xxxxxxxxx>; Huacai Chen
> <chenhuacai@xxxxxxxxxx>; Alexandre Torgue
> <alexandre.torgue@xxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-stm32@st-
> md-mailman.stormreply.com; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> 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");
