> -----Original Message-----
> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Sent: Thursday, April 11, 2024 3:59 AM
> To: D, Lakshmi Sowjanya <lakshmi.sowjanya.d@xxxxxxxxx>;
> jstultz@xxxxxxxxxx; giometti@xxxxxxxxxxxx; corbet@xxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Cc: x86@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx; intel-
> wired-lan@xxxxxxxxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; Dong, Eddie
> <eddie.dong@xxxxxxxxx>; Hall, Christopher S <christopher.s.hall@xxxxxxxxx>;
> Brandeburg, Jesse <jesse.brandeburg@xxxxxxxxx>; davem@xxxxxxxxxxxxx;
> alexandre.torgue@xxxxxxxxxxx; joabreu@xxxxxxxxxxxx;
> mcoquelin.stm32@xxxxxxxxx; perex@xxxxxxxx; linux-sound@xxxxxxxxxxxxxxx;
> Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>;
> peter.hilber@xxxxxxxxxxxxxxx; N, Pandith <pandith.n@xxxxxxxxx>; Mohan,
> Subramanian <subramanian.mohan@xxxxxxxxx>; T R, Thejesh Reddy
> <thejesh.reddy.t.r@xxxxxxxxx>; D, Lakshmi Sowjanya
> <lakshmi.sowjanya.d@xxxxxxxxx>
> Subject: Re: [PATCH v6 09/11] pps: generators: Add PPS Generator TIO Driver
>
> On Wed, Apr 10 2024 at 17:18, lakshmi.sowjanya.d@xxxxxxxxx wrote:
> > +static bool pps_generate_next_pulse(struct pps_tio *tio, ktime_t
> > +expires) {
> > + u64 art;
> > +
> > + if (!ktime_real_to_base_clock(expires, CSID_X86_ART, &art)) {
> > + pps_tio_disable(tio);
> > + return false;
> > + }
> > +
> > + pps_compv_write(tio, art - ART_HW_DELAY_CYCLES);
> > + return true;
> > +}
> > +
> > +static enum hrtimer_restart hrtimer_callback(struct hrtimer *timer) {
> > + struct pps_tio *tio = container_of(timer, struct pps_tio, timer);
> > + ktime_t expires, now;
> > + u32 event_count;
> > +
> > + guard(spinlock)(&tio->lock);
> > +
> > + /* Check if any event is missed. If an event is missed, TIO will be
> disabled*/
> > + event_count = pps_tio_read(tio, TIOEC);
> > + if (!tio->prev_count && tio->prev_count == event_count)
> > + goto err;
> > + tio->prev_count = event_count;
> > + expires = hrtimer_get_expires(timer);
> > + now = ktime_get_real();
> > +
> > + if (now - expires < SAFE_TIME_NS) {
> > + if (!pps_generate_next_pulse(tio, expires + SAFE_TIME_NS))
> > + goto err;
> > + }
>
> If the hrtimer callback is invoked late so that now - expires is >= SAFE_TIME_NS
> then this just forwards the timer and tries again.
Yes we will introduce a return HRTIMER_NORESTART if the time is expired.
>
> This lacks any form of explanation why this is correct as obviously there will be a
> missing pulse, no?
We had added an event count check to detect the missed pulse(i.e if we had programmed an expired time).
Timed I/O hardware has an event count register to log the number of pulses generated.
>
> > + hrtimer_forward(timer, now, NSEC_PER_SEC / 2);
> > + return HRTIMER_RESTART;
> > +err:
> > + dev_err(tio->dev, "Event missed, Disabling Timed I/O");
> > + pps_tio_disable(tio);
>
> Why does this disable it again? The failure path in
> pps_generate_next_pulse() does so already, no?
will remove disabling twice in the next version of patchset.
>
> > + return HRTIMER_NORESTART;
> > +}
> > +
>
> Thanks,
>
> tglx
