Re: [V5 02/18] net/hinic3: add basic header files

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed,  2 Jul 2025 10:09:25 +0800
Feifei Wang <wff_light@vip.163.com> wrote:

> +static inline unsigned long
> +clock_gettime_ms(void)
> +{
> +	struct timespec tv;
> +
> +	clock_gettime(CLOCK_TYPE, &tv);
> +
> +	return (unsigned long)tv.tv_sec * HINIC3_S_TO_MS_UNIT +
> +	       (unsigned long)tv.tv_nsec / HINIC3_S_TO_NS_UNIT;
> +}
> +
> +#define jiffies		      clock_gettime_ms()
> +#define msecs_to_jiffies(ms)  (ms)
> +#define time_before(now, end) ((now) < (end))
> +

Use of clock_gettime() is discouraged. Instead, drivers should use
TSC directly like the rest of the DPDK.

Also, that version of time_before() macro is incorrect for the case of jiffie
wraparound. Better off to use uint64_t.

> +
> +static inline u16
> +ilog2(u32 n)
> +{
> +	u16 res = 0;
> +
> +	while (n > 1) {
> +		n >>= 1;
> +		res++;
> +	}
> +
> +	return res;
> +}
> +

If it matters, there are faster ways to do this function.

> +static inline int
> +hinic3_mutex_lock(pthread_mutex_t *pthreadmutex)
> +{
> +	int err;
> +
> +	err = pthread_mutex_lock(pthreadmutex);
> +	if (err)
> +		PMD_DRV_LOG(ERR, "Mutex lock failed, err: %d", err);
> +
> +	return err;
> +}
> +
> +static inline void
> +hinic3_mutex_unlock(pthread_mutex_t *pthreadmutex)
> +{
> +	pthread_mutex_unlock(pthreadmutex);
> +}

Why are you using pthread_mutex? DPDK drivers in general do not
do locking in the control path, it is assumed the application is
coded correctly to only do control operations from single context.

The driver needs to be more like other drivers.
"Snowflake" drivers mean applications have to deal with different
semantics for each device.