Re: [Linuxptp-devel] [PATCH] Handle error returned by kernel for clock adjustments

[Wojtek Wasko via Linuxptp-devel]

> -----Original Message-----
> From: Miroslav Lichvar <mlich...@redhat.com>
> Sent: Monday, January 30, 2023 4:56 PM
> To: Wojtek Wasko <wwa...@nvidia.com>
> Cc: linuxptp-devel@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH] Handle error returned by kernel for
> clock adjustments
> 
> External email: Use caution opening links or attachments
> 
> 
> On Mon, Jan 30, 2023 at 03:39:45PM +0000, Wojtek Wasko via Linuxptp-devel
> wrote:
> > @@ -62,8 +62,11 @@ void clockadj_set_freq(clockid_t clkid, double
> > freq)
> >
> >       tx.modes |= ADJ_FREQUENCY;
> >       tx.freq = (long) (freq * 65.536);
> > -     if (clock_adjtime(clkid, &tx) < 0)
> > +     if (clock_adjtime(clkid, &tx) < 0) {
> >               pr_err("failed to adjust the clock: %m");
> > +             return -1;
> > +     }
> 
> Why not follow clockadj_get_freq() and simply exit here instead of trying
> to recover?
> 
> What PHC/driver is expected to fail temporarily?

Getting frequency is a readonly operation and as such when it fails, it may 
very well indicate a fatal/non-recoverable error.

Setting frequency is read-write and may fail at runtime for multiple reasons. 
For example, device might be busy and an internal semaphore might be taken 
because of another operation taking place.

Theoretical reasons aside and taking a quick look at the upstream, various 
drivers temporarily return error codes from e.g. their .adjfreq implementations:
- due to overall command failure/device resources not being temporarily 
available (e.g. mlx5_ptp_adjfreq, bnxt_ptp_adjfreq, bnx2x_ptp_adjfreq, 
ravb_ptp_adjfreq)
- due to network interface being down (e.g. bnx2x_ptp_adjfreq)
- due to adjustment value being too large (e.g. lan743x_ptpci_adjreq)

> 
> --
> Miroslav Lichvar



_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel