* [PATCH net] ptp: ocp: have adjtime handle negative delta_ns correctly
@ 2022-05-05 23:40 Jonathan Lemon
2022-05-07 0:19 ` Vadim Fedorenko
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Lemon @ 2022-05-05 23:40 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, kernel-team, davem, kuba, pabeni
delta_ns is a s64, but it was being passed ptp_ocp_adjtime_coarse
as an u64. Also, it turns out that timespec64_add_ns() only handles
positive values, so the math needs to be updated.
Fix by passing in the correct signed value, then adding to a
nanosecond version of the timespec.
Fixes: '90f8f4c0e3ce ("ptp: ocp: Add ptp_ocp_adjtime_coarse for large adjustments")'
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
drivers/ptp/ptp_ocp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index dd45471f6780..65e592ec272e 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -841,16 +841,18 @@ __ptp_ocp_adjtime_locked(struct ptp_ocp *bp, u32 adj_val)
}
static void
-ptp_ocp_adjtime_coarse(struct ptp_ocp *bp, u64 delta_ns)
+ptp_ocp_adjtime_coarse(struct ptp_ocp *bp, s64 delta_ns)
{
struct timespec64 ts;
unsigned long flags;
int err;
+ s64 ns;
spin_lock_irqsave(&bp->lock, flags);
err = __ptp_ocp_gettime_locked(bp, &ts, NULL);
if (likely(!err)) {
- timespec64_add_ns(&ts, delta_ns);
+ ns = timespec64_to_ns(&ts) + delta_ns;
+ ts = ns_to_timespec64(ns);
__ptp_ocp_settime_locked(bp, &ts);
}
spin_unlock_irqrestore(&bp->lock, flags);
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] ptp: ocp: have adjtime handle negative delta_ns correctly
2022-05-05 23:40 [PATCH net] ptp: ocp: have adjtime handle negative delta_ns correctly Jonathan Lemon
@ 2022-05-07 0:19 ` Vadim Fedorenko
2022-05-08 4:55 ` Jonathan Lemon
0 siblings, 1 reply; 4+ messages in thread
From: Vadim Fedorenko @ 2022-05-07 0:19 UTC (permalink / raw)
To: Jonathan Lemon, netdev; +Cc: richardcochran, kernel-team, davem, kuba, pabeni
On 06.05.2022 00:40, Jonathan Lemon wrote:
> delta_ns is a s64, but it was being passed ptp_ocp_adjtime_coarse
> as an u64. Also, it turns out that timespec64_add_ns() only handles
> positive values, so the math needs to be updated.
>
> Fix by passing in the correct signed value, then adding to a
> nanosecond version of the timespec.
>
> Fixes: '90f8f4c0e3ce ("ptp: ocp: Add ptp_ocp_adjtime_coarse for large adjustments")'
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> ---
> drivers/ptp/ptp_ocp.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index dd45471f6780..65e592ec272e 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -841,16 +841,18 @@ __ptp_ocp_adjtime_locked(struct ptp_ocp *bp, u32 adj_val)
> }
>
> static void
> -ptp_ocp_adjtime_coarse(struct ptp_ocp *bp, u64 delta_ns)
> +ptp_ocp_adjtime_coarse(struct ptp_ocp *bp, s64 delta_ns)
> {
> struct timespec64 ts;
> unsigned long flags;
> int err;
> + s64 ns;
>
> spin_lock_irqsave(&bp->lock, flags);
> err = __ptp_ocp_gettime_locked(bp, &ts, NULL);
> if (likely(!err)) {
> - timespec64_add_ns(&ts, delta_ns);
> + ns = timespec64_to_ns(&ts) + delta_ns;
> + ts = ns_to_timespec64(ns);
Maybe use set_normalized_timespec64 instead of this ugly transformations and
additional variable?
> __ptp_ocp_settime_locked(bp, &ts);
> }
> spin_unlock_irqrestore(&bp->lock, flags);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] ptp: ocp: have adjtime handle negative delta_ns correctly
2022-05-07 0:19 ` Vadim Fedorenko
@ 2022-05-08 4:55 ` Jonathan Lemon
2022-05-09 1:22 ` Vadim Fedorenko
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Lemon @ 2022-05-08 4:55 UTC (permalink / raw)
To: Vadim Fedorenko; +Cc: netdev, richardcochran, kernel-team, davem, kuba, pabeni
On Sat, May 07, 2022 at 01:19:54AM +0100, Vadim Fedorenko wrote:
> On 06.05.2022 00:40, Jonathan Lemon wrote:
> > delta_ns is a s64, but it was being passed ptp_ocp_adjtime_coarse
> > as an u64. Also, it turns out that timespec64_add_ns() only handles
> > positive values, so the math needs to be updated.
> >
> > Fix by passing in the correct signed value, then adding to a
> > nanosecond version of the timespec.
> >
> > Fixes: '90f8f4c0e3ce ("ptp: ocp: Add ptp_ocp_adjtime_coarse for large adjustments")'
> > Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> > ---
> > drivers/ptp/ptp_ocp.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> > index dd45471f6780..65e592ec272e 100644
> > --- a/drivers/ptp/ptp_ocp.c
> > +++ b/drivers/ptp/ptp_ocp.c
> > @@ -841,16 +841,18 @@ __ptp_ocp_adjtime_locked(struct ptp_ocp *bp, u32 adj_val)
> > }
> > static void
> > -ptp_ocp_adjtime_coarse(struct ptp_ocp *bp, u64 delta_ns)
> > +ptp_ocp_adjtime_coarse(struct ptp_ocp *bp, s64 delta_ns)
> > {
> > struct timespec64 ts;
> > unsigned long flags;
> > int err;
> > + s64 ns;
> > spin_lock_irqsave(&bp->lock, flags);
> > err = __ptp_ocp_gettime_locked(bp, &ts, NULL);
> > if (likely(!err)) {
> > - timespec64_add_ns(&ts, delta_ns);
> > + ns = timespec64_to_ns(&ts) + delta_ns;
> > + ts = ns_to_timespec64(ns);
>
> Maybe use set_normalized_timespec64 instead of this ugly transformations and
> additional variable?
I don't see how that would work - set_normalized_timespec64 just sets
the ts from a <sec>.<nsec> value. In this case, delta_ns need to be
added in to the ts value, but timespec64_add_ns() doeesn't handle
negative values, hence the conversion / add / reconversion.
--
Jonathan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] ptp: ocp: have adjtime handle negative delta_ns correctly
2022-05-08 4:55 ` Jonathan Lemon
@ 2022-05-09 1:22 ` Vadim Fedorenko
0 siblings, 0 replies; 4+ messages in thread
From: Vadim Fedorenko @ 2022-05-09 1:22 UTC (permalink / raw)
To: Jonathan Lemon; +Cc: netdev, richardcochran, kernel-team, davem, kuba, pabeni
On 08.05.2022 05:55, Jonathan Lemon wrote:
> On Sat, May 07, 2022 at 01:19:54AM +0100, Vadim Fedorenko wrote:
>> On 06.05.2022 00:40, Jonathan Lemon wrote:
>>> delta_ns is a s64, but it was being passed ptp_ocp_adjtime_coarse
>>> as an u64. Also, it turns out that timespec64_add_ns() only handles
>>> positive values, so the math needs to be updated.
>>>
>>> Fix by passing in the correct signed value, then adding to a
>>> nanosecond version of the timespec.
>>>
>>> Fixes: '90f8f4c0e3ce ("ptp: ocp: Add ptp_ocp_adjtime_coarse for large adjustments")'
>>> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
>>> ---
>>> drivers/ptp/ptp_ocp.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
>>> index dd45471f6780..65e592ec272e 100644
>>> --- a/drivers/ptp/ptp_ocp.c
>>> +++ b/drivers/ptp/ptp_ocp.c
>>> @@ -841,16 +841,18 @@ __ptp_ocp_adjtime_locked(struct ptp_ocp *bp, u32 adj_val)
>>> }
>>> static void
>>> -ptp_ocp_adjtime_coarse(struct ptp_ocp *bp, u64 delta_ns)
>>> +ptp_ocp_adjtime_coarse(struct ptp_ocp *bp, s64 delta_ns)
>>> {
>>> struct timespec64 ts;
>>> unsigned long flags;
>>> int err;
>>> + s64 ns;
>>> spin_lock_irqsave(&bp->lock, flags);
>>> err = __ptp_ocp_gettime_locked(bp, &ts, NULL);
>>> if (likely(!err)) {
>>> - timespec64_add_ns(&ts, delta_ns);
>>> + ns = timespec64_to_ns(&ts) + delta_ns;
>>> + ts = ns_to_timespec64(ns);
>>
>> Maybe use set_normalized_timespec64 instead of this ugly transformations and
>> additional variable?
>
> I don't see how that would work - set_normalized_timespec64 just sets
> the ts from a <sec>.<nsec> value. In this case, delta_ns need to be
> added in to the ts value, but timespec64_add_ns() doeesn't handle
> negative values, hence the conversion / add / reconversion.
Could be like:
- timespec64_add_ns(&ts, delta_ns);
+ set_normalized_timespec64(&ts, ts.tv_sec, ts.tv_nsec + delta_ns);
That's actually without multiplication and division caused by two conversions.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-09 1:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05 23:40 [PATCH net] ptp: ocp: have adjtime handle negative delta_ns correctly Jonathan Lemon
2022-05-07 0:19 ` Vadim Fedorenko
2022-05-08 4:55 ` Jonathan Lemon
2022-05-09 1:22 ` Vadim Fedorenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).