Re: [PATCH] pps: fix poll support

[Rodolfo Giometti]

On 03/04/25 08:06, Denis OSTERLAND-HEIM wrote:
> Hi Rodolfo,
>
> Do you think that there is a chance that this patch make it in the current merge window?

Honestly, I don't know, it's up to Greg or Andrew to proceed for inclusion.

Ciao,

Rodolfo

> -----Original Message-----
> From: Rodolfo Giometti <giometti@xxxxxxxxxxxx>
> Sent: Monday, March 3, 2025 12:54 PM
> To: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; Denis OSTERLAND-HEIM <denis.osterland@xxxxxxxxx>; stable@xxxxxxxxxxxxxxx
> Subject: [EXT] Re: [PATCH] pps: fix poll support
>
> [EXTERNAL EMAIL]
>   
>
> On 03/03/25 09:02, Denis OSTERLAND-HEIM wrote:
> > [BUG]
> > A user space program that calls select/poll get always an immediate data
> > ready-to-read response. As a result the intended use to wait until next
> > data becomes ready does not work.
> >
> > User space snippet:
> >
> >       struct pollfd pollfd = {
> >         .fd = open("/dev/pps0", O_RDONLY),
> >         .events = POLLIN|POLLERR,
> >         .revents = 0 };
> >       while(1) {
> >         poll(&pollfd, 1, 2000/*ms*/); // returns immediate, but should wait
> >         if(revents & EPOLLIN) { // always true
> >           struct pps_fdata fdata;
> >           memset(&fdata, 0, sizeof(memdata));
> >           ioctl(PPS_FETCH, &fdata); // currently fetches data at max speed
> >         }
> >       }
> >
> > [CAUSE]
> > pps_cdev_poll() returns unconditionally EPOLLIN.
> >
> > [FIX]
> > Remember the last fetch event counter and compare this value in
> > pps_cdev_poll() with most recent event counter
> > and return 0 if they are equal.
> >
> > Signed-off-by: Denis OSTERLAND-HEIM <denis.osterland@xxxxxxxxx>
> > Co-developed-by: Rodolfo Giometti <giometti@xxxxxxxxxxxx>
> > Signed-off-by: Rodolfo Giometti <giometti@xxxxxxxxxxxx>
>
> Acked-by: Rodolfo Giometti <giometti@xxxxxxxxxxxx>
>
> If needed. :)
>
> > Fixes: eae9d2ba0cfc ("LinuxPPS: core support")
> > CC: stable@xxxxxxxxxxxxxx # 5.4+
> > ---
> >    drivers/pps/pps.c          | 11 +++++++++--
> >    include/linux/pps_kernel.h |  1 +
> >    2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
> > index 6a02245ea35f..9463232af8d2 100644
> > --- a/drivers/pps/pps.c
> > +++ b/drivers/pps/pps.c
> > @@ -41,6 +41,9 @@ static __poll_t pps_cdev_poll(struct file *file, poll_table *wait)
> >    
> >    	poll_wait(file, &pps->queue, wait);
> >    
> > +	if (pps->last_fetched_ev == pps->last_ev)
> > +		return 0;
> > +
> >    	return EPOLLIN | EPOLLRDNORM;
> >    }
> >    
> > @@ -186,9 +189,11 @@ static long pps_cdev_ioctl(struct file *file,
> >    		if (err)
> >    			return err;
> >    
> > -		/* Return the fetched timestamp */
> > +		/* Return the fetched timestamp and save last fetched event  */
> >    		spin_lock_irq(&pps->lock);
> >    
> > +		pps->last_fetched_ev = pps->last_ev;
> > +
> >    		fdata.info.assert_sequence = pps->assert_sequence;
> >    		fdata.info.clear_sequence = pps->clear_sequence;
> >    		fdata.info.assert_tu = pps->assert_tu;
> > @@ -272,9 +277,11 @@ static long pps_cdev_compat_ioctl(struct file *file,
> >    		if (err)
> >    			return err;
> >    
> > -		/* Return the fetched timestamp */
> > +		/* Return the fetched timestamp and save last fetched event  */
> >    		spin_lock_irq(&pps->lock);
> >    
> > +		pps->last_fetched_ev = pps->last_ev;
> > +
> >    		compat.info.assert_sequence = pps->assert_sequence;
> >    		compat.info.clear_sequence = pps->clear_sequence;
> >    		compat.info.current_mode = pps->current_mode;
> > diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
> > index c7abce28ed29..aab0aebb529e 100644
> > --- a/include/linux/pps_kernel.h
> > +++ b/include/linux/pps_kernel.h
> > @@ -52,6 +52,7 @@ struct pps_device {
> >    	int current_mode;			/* PPS mode at event time */
> >    
> >    	unsigned int last_ev;			/* last PPS event id */
> > +	unsigned int last_fetched_ev;		/* last fetched PPS event id */
> >    	wait_queue_head_t queue;		/* PPS event queue */
> >    
> >    	unsigned int id;			/* PPS source unique ID */

--
GNU/Linux Solutions                  e-mail: giometti@xxxxxxxxxxxx
Linux Device Driver                          giometti@xxxxxxxx
Embedded Systems                     phone:  +39 349 2432127
UNIX programming