linux-kernel - RE: [PATCH net v2 2/2] net: stmmac: skip only stmmac_ptp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 26 Jan 2022 07:23:22 +0000
From:   "Ismail, Mohammad Athari" <mohammad.athari.ismail@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        Jose Abreu <joabreu@...opsys.com>,
        "David S . Miller" <davem@...emloft.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        "Ong, Boon Leong" <boon.leong.ong@...el.com>,
        "Voon, Weifeng" <weifeng.voon@...el.com>,
        "Wong, Vee Khee" <vee.khee.wong@...el.com>,
        "Huacai Chen" <chenhuacai@...nel.org>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH net v2 2/2] net: stmmac: skip only stmmac_ptp_register
 when resume from suspend



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