linux-kernel - Re: [PATCH v2 1/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 17 Oct 2020 07:27:53 +0800
From:   Kent Gibson <warthog618@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Jack Winch <sunt.un.morcov@...il.com>
Subject: Re: [PATCH v2 1/3] gpiolib: cdev: allow edge event timestamps to be
 configured as REALTIME

On Fri, Oct 16, 2020 at 05:13:22PM +0300, Andy Shevchenko wrote:
> On Thu, Oct 15, 2020 at 6:53 AM Kent Gibson <warthog618@...il.com> wrote:
> >
> > Using CLOCK_REALTIME as the source for event timestamps is crucial for
> > some specific applications, particularly those requiring timetamps
> > relative to a PTP clock, so provide an option to switch the event
> > timestamp source from the default CLOCK_MONOTONIC to CLOCK_REALTIME.
> >
> > Note that CLOCK_REALTIME was the default source clock for GPIO until
> > Linux 5.7 when it was changed to CLOCK_MONOTONIC due to issues with the
> > shifting of the realtime clock.
> > Providing this option maintains the CLOCK_MONOTONIC as the default,
> > while also providing a path forward for those dependent on the pre-5.7
> > behaviour.
> 
> ...
> 
> >          GPIO_V2_LINE_DIRECTION_FLAGS | \
> >          GPIO_V2_LINE_DRIVE_FLAGS | \
> >          GPIO_V2_LINE_EDGE_FLAGS | \
> > +        GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME | \
> 
> Wondering if we would have something like
> 
>           GPIO_V2_LINE_CLOCK_FLAGS | \
> 
> here for the sake of consistency.
> 

I considered it, but thought the chance of there ever being another
CLOCK flag to be near zero - the remaining clocks relate to CPU or
thread time which don't have any relevance for external events like
GPIO.  If there ever is one we can split it out then.

And it is consistent with the GPIO_V2_LINE_FLAG_ACTIVE_LOW flag that you
pruned out in your response. i.e. lone flags don't get grouped.

> >          GPIO_V2_LINE_BIAS_FLAGS)
> 
> ...
> 
> > +static u64 line_event_timestamp(struct line *line)
> > +{
> 
> > +       if (test_bit(FLAG_EVENT_CLOCK_REALTIME, &line->desc->flags))
> 
> I dunno if we can actually drop the word EVENT from these definitions.
> I don't think we would have in the near future something similar for
> the non-event data.
> 

I considered this too, as another clock flag seems unlikely.
But if we ever add a non-event clock flag then this one would become
confusing, and the overhead of the long name seemed minor compared to
the clarity it brings with it.

Cheers,
Kent.