Subject: Re: [Intel-wired-lan] [PATCH iwl-next v5 06/12] iavf: add initial framework for registering PTP clock

From: Rahul Rameshbabu <[email-masked]>
Date: Thu, 18 Apr 2024 12:28:06 -0700

On Thu, 18 Apr, 2024 01:24:54 -0400 Mateusz Polchlopek <[email-masked]> wrote:
> From: Jacob Keller <[email-masked]>
>
> Add the iavf_ptp.c file and fill it in with a skeleton framework to
> allow registering the PTP clock device.
> Add implementation of helper functions to check if a PTP capability
> is supported and handle change in PTP capabilities.
> Enabling virtual clock would be possible, though it would probably
> perform poorly due to the lack of direct time access.
>
> Reviewed-by: Sai Krishna <[email-masked]>
> Reviewed-by: Wojciech Drewek <[email-masked]>
> Signed-off-by: Jacob Keller <[email-masked]>
> Co-developed-by: Ahmed Zaki <[email-masked]>
> Signed-off-by: Ahmed Zaki <[email-masked]>
> Co-developed-by: Mateusz Polchlopek <[email-masked]>
> Signed-off-by: Mateusz Polchlopek <[email-masked]>
> ---
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.c b/drivers/net/ethernet/intel/iavf/iavf_ptp.c
<snip>
> +/**
> + * iavf_ptp_release - Disable PTP support
> + * @adapter: private adapter structure
> + *
> + * Release all PTP resources that were previously initialized.
> + */
> +void iavf_ptp_release(struct iavf_adapter *adapter)
> +{
> +	if (!IS_ERR_OR_NULL(adapter->ptp.clock)) {
> +		dev_info(&adapter->pdev->dev, "removing PTP clock %s\n",
> +			 adapter->ptp.info.name);
> +		ptp_clock_unregister(adapter->ptp.clock);
> +		adapter->ptp.clock = NULL;
> +	}
> +
> +	adapter->ptp.initialized = false;

I think teardown should be LIFO order to initialization. I would move
this line to the beginning of the function before any resources are
actually released.

> +}

<snip>

--
Thanks,

Rahul Rameshbabu