Re: [dpdk-dev] [PATCH v4 3/3] rxtx_callbacks: Add support for HW timestamp

From: Thomas Monjalon <thomas@monjalon.net>
To: Tom Barbette <barbette@kth.se>
Cc: dev@dpdk.org, bruce.richardson@intel.com,
	john.mcnamara@intel.com, Ferruh Yigit <ferruh.yigit@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	Shahaf Shuler <shahafs@mellanox.com>,
	Yongseok Koh <yskoh@mellanox.com>,
	olivier.matz@6wind.com
Subject: Re: [dpdk-dev] [PATCH v4 3/3] rxtx_callbacks: Add support for HW timestamp
Date: Thu, 13 Jun 2019 14:55:37 +0900	[thread overview]
Message-ID: <4877681.EENiiDfvPL@xps> (raw)
In-Reply-To: <20190502121135.18775-4-barbette@kth.se>

02/05/2019 21:11, Tom Barbette:
> +	if (hw_timestamping && ticks_per_cycle_mult  == 0) {
> +		uint64_t cycles_base = rte_rdtsc();
> +		uint64_t ticks_base;
> +		retval = rte_eth_read_clock(port, &ticks_base);
> +		if (retval != 0)
> +			return retval;
> +		rte_delay_ms(100);
> +		uint64_t cycles = rte_rdtsc();
> +		uint64_t ticks;
> +		rte_eth_read_clock(port, &ticks);
> +		uint64_t c_freq = cycles - cycles_base;
> +		uint64_t t_freq = ticks - ticks_base;
> +		double freq_mult = (double)c_freq / t_freq;
> +		printf("TSC Freq ~= %lu\nHW Freq ~= %lu\nRatio : %f\n",
> +				c_freq * 10, t_freq * 10, freq_mult);
> +		/* TSC will be faster than internal ticks so freq_mult is > 0
> +		 * We convert the multiplication to an integer shift & mult
> +		 */
> +		ticks_per_cycle_mult = (1 << TICKS_PER_CYCLE_SHIFT) / freq_mult;
> +	}

I see two issues in this code:
1/ statements are mixed with variable declarations
2/ %lu is used for 64-bit variables, which does not work on 32-bit system.

I am fixing item 2 when merging.
I hope item 1 won't be an issue for some old compilers.