LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Dipen Patel <dipenp@nvidia.com>
Cc: Jon Hunter <jonathanh@nvidia.com>,
	thierry.reding@gmail.com, linux-kernel@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-gpio@vger.kernel.org,
	linus.walleij@linaro.org, bgolaszewski@baylibre.com,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	robh+dt@kernel.org
Subject: Re: [RFC 08/11] gpiolib: cdev: Add hardware timestamp clock type
Date: Wed, 4 Aug 2021 07:09:02 +0800	[thread overview]
Message-ID: <20210803230902.GA7730@sol> (raw)
In-Reply-To: <f0d8da75-6e31-5432-6909-8eab6960b787@nvidia.com>

On Tue, Aug 03, 2021 at 03:51:31PM -0700, Dipen Patel wrote:
> 
> On 8/3/21 9:42 AM, Jon Hunter wrote:
> >
> > On 30/07/2021 03:33, Dipen Patel wrote:
> >>
> >> On 7/9/21 1:30 AM, Jon Hunter wrote:
> >>> On 26/06/2021 00:55, Dipen Patel wrote:
> >>>> This patch adds new clock type for the GPIO controller which can
> >>>> timestamp gpio lines using hardware means. To expose such
> >>>> functionalities to the userspace, code has been added in this patch
> >>>> where during line create call, it checks for new clock type and if
> >>>> requested, calls hardware timestamp related API from gpiolib.c.
> >>>> During line change event, it retrieves timestamp in nano seconds by
> >>>> calling gpiod_get_hw_timestamp API from gpiolib.c. At the line release,
> >>>> it disables this functionality by calling gpiod_hw_timestamp_control.
> >>>>
> >>>> Signed-off-by: Dipen Patel <dipenp@nvidia.com>
> >>>> ---
> >>>>   drivers/gpio/gpiolib-cdev.c | 65 +++++++++++++++++++++++++++++++++++--
> >>>>   include/uapi/linux/gpio.h   |  1 +
> >>>>   2 files changed, 64 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> >>>> index 1631727bf0da..9f98c727e937 100644
> >>>> --- a/drivers/gpio/gpiolib-cdev.c
> >>>> +++ b/drivers/gpio/gpiolib-cdev.c
> >>>> @@ -518,6 +518,7 @@ struct linereq {
> >>>>        GPIO_V2_LINE_DRIVE_FLAGS | \
> >>>>        GPIO_V2_LINE_EDGE_FLAGS | \
> >>>>        GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME | \
> >>>> +     GPIO_V2_LINE_FLAG_EVENT_CLOCK_HARDWARE | \
> >>>>        GPIO_V2_LINE_BIAS_FLAGS)
> >>>>     static void linereq_put_event(struct linereq *lr,
> >>>> @@ -540,9 +541,20 @@ static void linereq_put_event(struct linereq *lr,
> >>>>     static u64 line_event_timestamp(struct line *line)
> >>>>   {
> >>>> +    bool block;
> >>>> +
> >>>>       if (test_bit(FLAG_EVENT_CLOCK_REALTIME, &line->desc->flags))
> >>>>           return ktime_get_real_ns();
> >>>>   +    if (test_bit(FLAG_EVENT_CLOCK_HARDWARE, &line->desc->flags)) {
> >>>> +        if (irq_count())
> >>>> +            block = false;
> >>>> +        else
> >>>> +            block = true;
> >>>> +
> >>>> +        return gpiod_get_hw_timestamp(line->desc, block);
> >>>> +    }
> >>>> +
> >>>>       return ktime_get_ns();
> >>>>   }
> >>>
> >>> Looking at line_event_timestamp() and the callers of this function, it
> >>> appears that this should always return nanoseconds. Does
> >>> gpiod_get_hw_timestamp() return nanoseconds?
> >> Yes, it returns in ns to align with line_event_timestamp.
> >
> >
> > It might be worth updating the function name to gpiod_get_hw_timestamp_ns() so that this is clear.
> Wouldn't be sufficient to right into its description rather embed in API name?
> >

Adding a suffix identifying the timestamp resolution to variable and
function names is pretty standard in the kernel.
It makes the code easier to read as you don't have to keep checking the
documentation.

Cheers,
Kent.

  reply	other threads:[~2021-08-03 23:09 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-25 23:55 [RFC 00/11] Intro to Hardware timestamping engine Dipen Patel
2021-06-25 23:55 ` [RFC 01/11] Documentation: Add HTE subsystem guide Dipen Patel
2021-07-04 18:55   ` Jonathan Cameron
2021-07-27 23:44     ` Dipen Patel
2021-08-01 15:24       ` Jonathan Cameron
2021-06-25 23:55 ` [RFC 02/11] drivers: Add HTE subsystem Dipen Patel
2021-06-27 17:40   ` Randy Dunlap
2021-07-04 20:15   ` Jonathan Cameron
2021-07-04 20:45     ` Jonathan Cameron
2021-07-28  5:12       ` Dipen Patel
2021-08-01 16:48         ` Jonathan Cameron
2021-07-28  4:38     ` Dipen Patel
2021-08-01 16:13       ` Jonathan Cameron
2021-09-14  5:43         ` Dipen Patel
2021-09-26 15:42           ` Jonathan Cameron
2021-07-05  7:30   ` Greg KH
2021-07-28  0:34     ` Dipen Patel
2021-06-25 23:55 ` [RFC 03/11] hte: Add tegra194 HTE kernel provider Dipen Patel
2021-07-01 14:21   ` Kent Gibson
2021-07-28 23:59     ` Dipen Patel
2021-07-30  7:01       ` Dipen Patel
2021-07-31 15:43       ` Kent Gibson
2021-08-03 22:40         ` Dipen Patel
2021-08-03 23:02           ` Kent Gibson
2021-08-07  2:41         ` Dipen Patel
2021-08-07  3:07           ` Kent Gibson
2021-08-07  4:52             ` Dipen Patel
2021-08-07  4:51               ` Kent Gibson
2021-08-07  5:35                 ` Dipen Patel
2021-08-07  5:42                   ` Kent Gibson
2021-08-07  5:47                   ` Dipen Patel
2021-07-04 20:27   ` Jonathan Cameron
2021-07-29  2:42     ` Dipen Patel
2021-07-08 23:33   ` Michał Mirosław
2021-07-29  2:43     ` Dipen Patel
2021-06-25 23:55 ` [RFC 04/11] dt-bindings: Add HTE bindings Dipen Patel
2021-06-27 10:56   ` Linus Walleij
2021-07-30  1:32     ` Dipen Patel
2021-07-01 14:02   ` Rob Herring
2021-07-30  1:56     ` Dipen Patel
2021-07-01 15:54   ` Rob Herring
2021-07-30  1:58     ` Dipen Patel
2021-06-25 23:55 ` [RFC 05/11] hte: Add Tegra194 IRQ HTE test driver Dipen Patel
2021-06-27 17:42   ` Randy Dunlap
2021-06-25 23:55 ` [RFC 06/11] gpiolib: Add HTE support Dipen Patel
2021-06-27 11:41   ` Linus Walleij
2021-07-01 14:24   ` Kent Gibson
2021-07-30  2:25     ` Dipen Patel
2021-07-31  5:13       ` Kent Gibson
2021-06-25 23:55 ` [RFC 07/11] gpio: tegra186: Add HTE in gpio-tegra186 driver Dipen Patel
2021-06-25 23:55 ` [RFC 08/11] gpiolib: cdev: Add hardware timestamp clock type Dipen Patel
2021-06-27 11:38   ` Linus Walleij
2021-06-27 11:49   ` Linus Walleij
2021-07-30  3:16     ` Dipen Patel
2021-07-01 14:24   ` Kent Gibson
2021-07-30  3:07     ` Dipen Patel
2021-07-31  6:05       ` Kent Gibson
2021-08-03 22:41         ` Dipen Patel
2021-08-03 22:38           ` Kent Gibson
2021-07-09  8:30   ` Jon Hunter
2021-07-30  2:33     ` Dipen Patel
2021-08-03 16:42       ` Jon Hunter
2021-08-03 22:51         ` Dipen Patel
2021-08-03 23:09           ` Kent Gibson [this message]
2021-06-25 23:55 ` [RFC 09/11] tools: gpio: Add new hardware " Dipen Patel
2021-06-27 11:36   ` Linus Walleij
2021-07-30  3:17     ` Dipen Patel
2021-07-31  6:16       ` Kent Gibson
2021-08-11  9:11         ` Linus Walleij
2021-06-25 23:55 ` [RFC 10/11] hte: Add tegra GPIO HTE test driver Dipen Patel
2021-06-27 17:43   ` Randy Dunlap
2021-06-25 23:55 ` [RFC 11/11] MAINTAINERS: Added HTE Subsystem Dipen Patel
2021-06-27 13:07 ` [RFC 00/11] Intro to Hardware timestamping engine Andy Shevchenko
2021-06-27 14:40   ` Linus Walleij
2021-06-28 12:02     ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210803230902.GA7730@sol \
    --to=warthog618@gmail.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dipenp@nvidia.com \
    --cc=jonathanh@nvidia.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    --subject='Re: [RFC 08/11] gpiolib: cdev: Add hardware timestamp clock type' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).