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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220125201756.1606e1c4@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Tue, 25 Jan 2022 20:17:56 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Mohammad Athari Bin Ismail <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@...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.

>  	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");