netdev - Re: [PATCH 1/2] net: stmmac: retain PTP-clock at hwtstamp

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: <ae5371c0-ea53-6885-a25b-b44e9fe0b615@pengutronix.de>
Date:   Thu, 17 Dec 2020 09:25:48 +0100
From:   Ahmad Fatoum <a.fatoum@...gutronix.de>
To:     Jakub Kicinski <kuba@...nel.org>,
        Holger Assmann <h.assmann@...gutronix.de>
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>,
        Rayagond Kokatanur <rayagond@...avyalabs.com>,
        kernel@...gutronix.de, Michael Olbrich <m.olbrich@...gutronix.de>,
        Jose Abreu <Jose.Abreu@...opsys.com>, netdev@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH 1/2] net: stmmac: retain PTP-clock at hwtstamp_set

Hello,

On 17.12.20 02:13, Jakub Kicinski wrote:
>> +			netdev_warn(priv->dev, "HW Timestamping init failed: %pe\n",
>> +					ERR_PTR(ret));
> 
> why convert to ERR_PTR and use %pe and not just %d?

To get a symbolic error name if support is compiled in (note the `e' after %p).

> 
> also continuation misaligned
> 
>> +		} else {
>> +			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");
>> +		}
>>  	}
>>  
>>  	priv->eee_tw_timer = STMMAC_DEFAULT_TWT_LS;
>> @@ -5290,8 +5330,7 @@ int stmmac_resume(struct device *dev)
>>  		/* enable the clk previously disabled */
>>  		clk_prepare_enable(priv->plat->stmmac_clk);
>>  		clk_prepare_enable(priv->plat->pclk);
>> -		if (priv->plat->clk_ptp_ref)
>> -			clk_prepare_enable(priv->plat->clk_ptp_ref);
>> +		stmmac_init_hwtstamp(priv);
> 
> This was optional, now you always init?

Indeed, omitting the if condition here will lead to a needless warning on every reset.

Cheers,
Ahmad

> 
>>  		/* reset the phy so that it's ready */
>>  		if (priv->mii)
>>  			stmmac_mdio_reset(priv->mii);
>>
>> base-commit: 3db1a3fa98808aa90f95ec3e0fa2fc7abf28f5c9
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |