On Fri, Jun 25, 2021 at 04:55:27PM -0700, Dipen Patel wrote:
> Some GPIO chip can provide hardware timestamp support on its GPIO lines
> , in order to support that additional functions needs to be added which
> can talk to both GPIO chip and HTE (hardware timestamping engine)
> subsystem. This patch introduces functions which gpio consumer can use
> to request hardware assisted timestamping. Below is the list of the APIs
> that are added in gpiolib subsystem.
>
> - gpiod_hw_timestamp_control - to enable/disable HTE on specified GPIO
> line. This API will return HTE specific descriptor for the specified
> GPIO line during the enable call, it will be stored as pointer in the
> gpio_desc structure as hw_ts_data.
> - gpiod_is_hw_timestamp_enabled - to query if HTE is enabled on
> specified GPIO line.
> - gpiod_get_hw_timestamp - to retrieve hardware timestamps.
>
> Signed-off-by: Dipen Patel <dipenp@xxxxxxxxxx>
> ---
> drivers/gpio/gpiolib.c | 92 +++++++++++++++++++++++++++++++++++
> drivers/gpio/gpiolib.h | 11 +++++
> include/linux/gpio/consumer.h | 21 +++++++-
> include/linux/gpio/driver.h | 13 +++++
> 4 files changed, 135 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 220a9d8dd4e3..335eaddfde98 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2361,6 +2361,98 @@ int gpiod_direction_output(struct gpio_desc *desc, int value)
> }
> EXPORT_SYMBOL_GPL(gpiod_direction_output);
>
> +/**
> + * gpiod_hw_timestamp_control - set the hardware assisted timestamp control.
> + * @desc: GPIO to set
> + * @enable: Set true to enable the hardware timestamp, false otherwise.
> + *
> + * Certain GPIO chip can rely on hardware assisted timestamp engines which can
> + * record timestamp at the occurance of the configured events on selected GPIO
> + * lines. This is helper API to control such engine.
> + *
> + * Return 0 in case of success, else an error code.
> + */
> +int gpiod_hw_timestamp_control(struct gpio_desc *desc, bool enable)
> +{
> + struct gpio_chip *gc;
> + int ret = 0;
> +
> + VALIDATE_DESC(desc);
> + gc = desc->gdev->chip;
> +
> + if (!gc->timestamp_control) {
> + gpiod_warn(desc,
> + "%s: Hardware assisted ts not supported\n",
> + __func__);
> + return -ENOTSUPP;
> + }
> +
> + ret = gc->timestamp_control(gc, gpio_chip_hwgpio(desc),
> + &desc->hdesc, enable);
> +
> + if (ret) {
> + gpiod_warn(desc,
> + "%s: ts control operation failed\n", __func__);
> + return ret;
> + }
> +
> + if (!enable)
> + desc->hdesc = NULL;
> +
> + return ret;
> +}
Last I checked, pointer accesses are not guaranteed atomic, so how is
hdesc protected from concurrent access?
Here is it modified unprotected.
Below it is read unprotected.
> +EXPORT_SYMBOL_GPL(gpiod_hw_timestamp_control);
> +
> +/**
> + * gpiod_is_hw_timestamp_enabled - check if hardware assisted timestamp is
> + * enabled.
> + * @desc: GPIO to check
> + *
> + * Return true in case of success, false otherwise.
> + */
> +bool gpiod_is_hw_timestamp_enabled(const struct gpio_desc *desc)
> +{
> + if (!desc)
> + return false;
> +
> + return (desc->hdesc) ? true : false;
> +}
> +EXPORT_SYMBOL_GPL(gpiod_is_hw_timestamp_enabled);
> +
> +/**
> + * gpiod_get_hw_timestamp - Get hardware timestamp in nano seconds.
> + * @desc: GPIO to get the timestamp.
> + * @block: Set true to block until data is available.
> + *
> + * Return non-zero on success, else 0.
> + */
> +u64 gpiod_get_hw_timestamp(struct gpio_desc *desc, bool block)
> +{
> + struct gpio_chip *gc;
> + int ret = 0;
> + u64 ts;
> +
> + VALIDATE_DESC(desc);
> + gc = desc->gdev->chip;
> +
> + if (!gc->get_hw_timestamp) {
> + gpiod_warn(desc,
> + "%s: Hardware assisted ts not supported\n",
> + __func__);
> + return -ENOTSUPP;
> + }
> +
Can't return an error code here. Return value is u64, so this will look
like a valid ts.
Just return 0 on error, as you do immediately below...
> + ret = gc->get_hw_timestamp(gc, block, desc->hdesc, &ts);
> + if (ret) {
> + gpiod_warn(desc,
> + "%s: get timestamp operation failed\n", __func__);
> + return 0;
> + }
> +
> + return ts;
> +}
> +EXPORT_SYMBOL_GPL(gpiod_get_hw_timestamp);
> +
> /**
> * gpiod_set_config - sets @config for a GPIO
> * @desc: descriptor of the GPIO for which to set the configuration
> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> index 30bc3f80f83e..5393e1d90f61 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -15,6 +15,7 @@
> #include <linux/device.h>
> #include <linux/module.h>
> #include <linux/cdev.h>
> +#include <linux/hte.h>
>
> #define GPIOCHIP_NAME "gpiochip"
>
> @@ -117,6 +118,7 @@ struct gpio_desc {
> #define FLAG_EDGE_RISING 16 /* GPIO CDEV detects rising edge events */
> #define FLAG_EDGE_FALLING 17 /* GPIO CDEV detects falling edge events */
> #define FLAG_EVENT_CLOCK_REALTIME 18 /* GPIO CDEV reports REALTIME timestamps in events */
> +#define FLAG_EVENT_CLOCK_HARDWARE 19 /* GPIO CDEV reports hardware timestamps in events */
>
> /* Connection label */
> const char *label;
> @@ -129,6 +131,15 @@ struct gpio_desc {
> /* debounce period in microseconds */
> unsigned int debounce_period_us;
> #endif
> + /*
> + * Hardware timestamp engine related internal data structure.
> + * This gets set when the consumer calls gpiod_hw_timestamp_control to enable
> + * hardware timestamping on the specified GPIO line. The API calls into HTE
> + * subsystem, in turns HTE subsystem return the HTE descriptor for the GPIO
> + * line. The hdesc will be later used with gpiod_is_hw_timestamp_enabled
> + * and gpiod_get_hw_timestamp API calls.
> + */
> + struct hte_ts_desc *hdesc;
> };
>
> #define gpiod_not_found(desc) (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index c73b25bc9213..476ee04de7d0 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -112,6 +112,9 @@ int gpiod_get_direction(struct gpio_desc *desc);
> int gpiod_direction_input(struct gpio_desc *desc);
> int gpiod_direction_output(struct gpio_desc *desc, int value);
> int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
> +int gpiod_hw_timestamp_control(struct gpio_desc *desc, bool enable);
> +bool gpiod_is_hw_timestamp_enabled(const struct gpio_desc *desc);
> +u64 gpiod_get_hw_timestamp(struct gpio_desc *desc, bool block);
>
> /* Value get/set from non-sleeping context */
> int gpiod_get_value(const struct gpio_desc *desc);
> @@ -353,8 +356,22 @@ static inline int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
> WARN_ON(desc);
> return -ENOSYS;
> }
> -
> -
> +static inline int gpiod_hw_timestamp_control(struct gpio_desc *desc,
> + bool enable)
> +{
> + WARN_ON(desc);
> + return -ENOSYS;
> +}
> +static inline bool gpiod_is_hw_timestamp_enabled(const struct gpio_desc *desc)
> +{
> + WARN_ON(desc);
> + return false;
> +}
> +static inline u64 gpiod_get_hw_timestamp(struct gpio_desc *desc, bool block)
> +{
> + WARN_ON(desc);
> + return 0;
> +}
> static inline int gpiod_get_value(const struct gpio_desc *desc)
> {
> /* GPIO can never have been requested */
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index 3a268781fcec..f343e8f54b08 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -10,6 +10,7 @@
> #include <linux/lockdep.h>
> #include <linux/pinctrl/pinctrl.h>
> #include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/hte.h> /* For hardware timestamping */
>
> struct gpio_desc;
> struct of_phandle_args;
> @@ -304,6 +305,10 @@ struct gpio_irq_chip {
> * @add_pin_ranges: optional routine to initialize pin ranges, to be used when
> * requires special mapping of the pins that provides GPIO functionality.
> * It is called after adding GPIO chip and before adding IRQ chip.
> + * @timestamp_control: Dependent on GPIO chip, an optional routine to
> + * enable/disable hardware assisted timestamp.
> + * @get_hw_timestamp: Retrieves hardware timestamp. The consumer can specify
> + * block parameter if it wishes to block till timestamp is available.
> * @base: identifies the first GPIO number handled by this chip;
> * or, if negative during registration, requests dynamic ID allocation.
> * DEPRECATION: providing anything non-negative and nailing the base
> @@ -396,6 +401,14 @@ struct gpio_chip {
>
> int (*add_pin_ranges)(struct gpio_chip *gc);
>
> + int (*timestamp_control)(struct gpio_chip *gc,
> + unsigned int offset,
> + struct hte_ts_desc **hdesc,
> + bool enable);
> + int (*get_hw_timestamp)(struct gpio_chip *gc,
> + bool block,
> + struct hte_ts_desc *hdesc,
> + u64 *ts);
> int base;
> u16 ngpio;
> const char *const *names;
> --
> 2.17.1
>
Cheers,
Kent.
