netdev - Re: [PATCH net-next v5 1/4] net: wangxun: Add support for PTP clock

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: <9390f920-a89f-43d3-a75f-664fd05df655@linux.dev>
Date: Fri, 17 Jan 2025 14:15:01 +0000
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Jiawen Wu <jiawenwu@...stnetic.com>, andrew+netdev@...n.ch,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, richardcochran@...il.com, linux@...linux.org.uk,
 horms@...nel.org, jacob.e.keller@...el.com, netdev@...r.kernel.org
Cc: mengyuanlou@...-swift.com
Subject: Re: [PATCH net-next v5 1/4] net: wangxun: Add support for PTP clock

On 17/01/2025 06:20, Jiawen Wu wrote:
> Implement support for PTP clock on Wangxun NICs.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>

[...]

> +/**
> + * wx_ptp_create_clock
> + * @wx: the private board structure
> + *
> + * Returns 0 on success, negative value on failure
> + *
> + * This function performs setup of the user entry point function table and
> + * initalizes the PTP clock device used by userspace to access the clock-like
> + * features of the PTP core. It will be called by wx_ptp_init, and may
> + * re-use a previously initialized clock (such as during a suspend/resume
> + * cycle).
> + */
> +static long wx_ptp_create_clock(struct wx *wx)
> +{
> +	struct net_device *netdev = wx->netdev;
> +	long err;
> +
> +	/* do nothing if we already have a clock device */
> +	if (!IS_ERR_OR_NULL(wx->ptp_clock))
> +		return 0;
> +
> +	snprintf(wx->ptp_caps.name, sizeof(wx->ptp_caps.name),
> +		 "%s", netdev->name);
> +	wx->ptp_caps.owner = THIS_MODULE;
> +	wx->ptp_caps.n_alarm = 0;
> +	wx->ptp_caps.n_ext_ts = 0;
> +	wx->ptp_caps.n_per_out = 0;
> +	wx->ptp_caps.pps = 0;
> +	wx->ptp_caps.adjfine = wx_ptp_adjfine;
> +	wx->ptp_caps.adjtime = wx_ptp_adjtime;
> +	wx->ptp_caps.gettimex64 = wx_ptp_gettimex64;
> +	wx->ptp_caps.settime64 = wx_ptp_settime64;
> +	if (wx->mac.type == wx_mac_em)
> +		wx->ptp_caps.max_adj = 500000000;
> +	else
> +		wx->ptp_caps.max_adj = 250000000;
> +
> +	wx->ptp_clock = ptp_clock_register(&wx->ptp_caps, &wx->pdev->dev);
> +	if (IS_ERR(wx->ptp_clock)) {
> +		err = PTR_ERR(wx->ptp_clock);
> +		wx->ptp_clock = NULL;
> +		wx_err(wx, "ptp clock register failed\n");
> +		return err;
> +	} else if (wx->ptp_clock) {

there is no way ptp_clock_register() will return NULL, that means
this `else if` construction is always true, so it's meaning less.
Please remove it if there will be need for another version.

Otherwise LGTM,

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>


> +		dev_info(&wx->pdev->dev, "registered PHC device on %s\n",
> +			 netdev->name);
> +	}



[...]