netdev - Re: [PATCH net-next v2 7/9] ptp: ptp_clockmatrix: Add .getmaxphase 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 25 May 2023 14:08:43 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Rahul Rameshbabu <rrameshbabu@...dia.com>, netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>, Jacob Keller
	 <jacob.e.keller@...el.com>, Gal Pressman <gal@...dia.com>, Tariq Toukan
	 <tariqt@...dia.com>, Saeed Mahameed <saeed@...nel.org>, Richard Cochran
	 <richardcochran@...il.com>, Vincent Cheng <vincent.cheng.xh@...esas.com>
Subject: Re: [PATCH net-next v2 7/9] ptp: ptp_clockmatrix: Add .getmaxphase
 ptp_clock_info callback

On Tue, 2023-05-23 at 13:54 -0700, Rahul Rameshbabu wrote:
> Advertise the maximum offset the .adjphase callback is capable of
> supporting in nanoseconds for IDT ClockMatrix devices.
> 
> Cc: Richard Cochran <richardcochran@...il.com>
> Cc: Vincent Cheng <vincent.cheng.xh@...esas.com>
> Signed-off-by: Rahul Rameshbabu <rrameshbabu@...dia.com>
> ---
>  drivers/ptp/ptp_clockmatrix.c | 36 +++++++++++++++++------------------
>  drivers/ptp/ptp_clockmatrix.h |  2 +-
>  2 files changed, 18 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
> index c9d451bf89e2..f6f9d4adce04 100644
> --- a/drivers/ptp/ptp_clockmatrix.c
> +++ b/drivers/ptp/ptp_clockmatrix.c
> @@ -1692,14 +1692,23 @@ static int initialize_dco_operating_mode(struct idtcm_channel *channel)
>  /* PTP Hardware Clock interface */
>  
>  /*
> - * Maximum absolute value for write phase offset in picoseconds
> - *
> - * @channel:  channel
> - * @delta_ns: delta in nanoseconds
> + * Maximum absolute value for write phase offset in nanoseconds
>   *
>   * Destination signed register is 32-bit register in resolution of 50ps
>   *
> - * 0x7fffffff * 50 =  2147483647 * 50 = 107374182350
> + * 0x7fffffff * 50 =  2147483647 * 50 = 107374182350 ps
> + * Represent 107374182350 ps as 107374182 ns
> + */
> +static s32 idtcm_getmaxphase(struct ptp_clock_info *ptp __always_unused)
> +{
> +	return MAX_ABS_WRITE_PHASE_NANOSECONDS;
> +}

This introduces a functional change WRT the current code. Prior to this
patch ClockMatrix tries to adjust phase delta even above
MAX_ABS_WRITE_PHASE_NANOSECONDS, limiting the delta to such value.
After this patch it will error out.

Perhaps a more conservative approach would be keeping the existing
logic in _idtcm_adjphase and let idtcm_getmaxphase return  
S32_MAX?

Note that even that will error out for delta == S32_MIN so perhaps an
API change to allow the driver specify unlimited delta would be useful
(possibly regardless of the above).

Cheers,

Paolo