Hi,
Linus Walleij <linus.walleij@xxxxxxxxxx> writes:
> Add a hardirq handler to the GPIO userspace event loop, making
> sure to pick up the timestamp there, as close as possible in time
> relative to the actual event causing the interrupt.
>
> Tested with a simple pushbutton GPIO on ux500 and seems to work
> fine.
>
> Cc: Bartosz Golaszewski <brgl@xxxxxxxx>
> Cc: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> Reported-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> drivers/gpio/gpiolib.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index aad84a6306c4..6e006e6df95a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -587,6 +587,9 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
> * @events: KFIFO for the GPIO events
> * @read_lock: mutex lock to protect reads from colliding with adding
> * new events to the FIFO
> + * @timestamp: cache for the timestamp storing it between hardirq
> + * and IRQ thread, used to bring the timestamp close to the actual
> + * event
> */
> struct lineevent_state {
> struct gpio_device *gdev;
> @@ -597,6 +600,7 @@ struct lineevent_state {
> wait_queue_head_t wait;
> DECLARE_KFIFO(events, struct gpioevent_data, 16);
> struct mutex read_lock;
> + s64 timestamp;
> };
>
> #define GPIOEVENT_REQUEST_VALID_FLAGS \
> @@ -731,7 +735,7 @@ static irqreturn_t lineevent_irq_thread(int irq, void *p)
> struct gpioevent_data ge;
> int ret, level;
>
> - ge.timestamp = ktime_get_real_ns();
> + ge.timestamp = le->timestamp;
> level = gpiod_get_value_cansleep(le->desc);
>
> if (le->eflags & GPIOEVENT_REQUEST_RISING_EDGE
> @@ -759,6 +763,19 @@ static irqreturn_t lineevent_irq_thread(int irq, void *p)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t lineevent_irq_handler(int irq, void *p)
> +{
> + struct lineevent_state *le = p;
> +
> + /*
> + * Just store the timestamp in hardirq context so we get it as
> + * close in time as possible to the actual event.
> + */
> + le->timestamp = ktime_get_real_ns();
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> static int lineevent_create(struct gpio_device *gdev, void __user *ip)
> {
> struct gpioevent_request eventreq;
> @@ -851,7 +868,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
>
> /* Request a thread to read the events */
> ret = request_threaded_irq(le->irq,
> - NULL,
> + lineevent_irq_handler,
now that you have a hardirq handler, do you even need IRQF_ONESHOT? How
about using the hardirq handler to mask $this gpio's IRQ, then run
thread without IRQF_ONESHOT? This would help a in cases where the IRQ
line is shared.
Perhaps that should be done in a separate patch, though?
FWIW:
Reviewed-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
--
balbi
Attachment:
signature.asc
Description: PGP signature
