Re: [RFC v2 07/11] gpio: tegra186: Add HTE in gpio-tegra186 driver

From: Rob Herring
Date: Fri Oct 08 2021 - 18:16:12 EST


On Thu, Sep 30, 2021 at 04:26:13PM -0700, Dipen Patel wrote:
> Tegra194 AON GPIO controller with the use of its internal hardware
> timestamping engine (HTE) also known as GTE can timestamp GPIO
> lines through system counter. This patch implements two callbacks
> which are essential for the gpio consumers that want such HTE
> functionality. The callbacks details can be found at
> include/gpio/driver.h.
>
> Since AON GPIO controller depends on HTE engine, it creates hardware
> dependency between controller and AON HTE provider. To express that,
> the optional devicetree property is introduced for AON GPIO controller.
>
> Signed-off-by: Dipen Patel <dipenp@xxxxxxxxxx>
> ---
> .../bindings/gpio/nvidia,tegra186-gpio.txt | 7 ++

Bindings should be a separate patch. Consider converting this to schema
first too.

> drivers/gpio/gpio-tegra186.c | 89 +++++++++++++++++++
> 2 files changed, 96 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpio/nvidia,tegra186-gpio.txt b/Documentation/devicetree/bindings/gpio/nvidia,tegra186-gpio.txt
> index adff16c71d21..00a3e47ab560 100644
> --- a/Documentation/devicetree/bindings/gpio/nvidia,tegra186-gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/nvidia,tegra186-gpio.txt
> @@ -127,6 +127,12 @@ Required properties:
> - 8: Active low level-sensitive.
> Valid combinations are 1, 2, 3, 4, 8.
>
> +Optional properties:
> +- timestamp-engine

Should be in the common binding.

But then when do you use hardware-timestamps? This property seems to
assume GPIO hand the timestamp engine have the same numbering of
signals. I think you need to be able to map GPIOx to timestamp y. If you
want a short cut for a 1-1 case, then that's another discussion.

> + AON GPIO controller has timestamp engine which can hardware timestamp
> + GPIO configured as input and IRQ. This property specifies hardware
> + timestamp engine (HTE) device-tree node.
> +
> Example:
>
> #include <dt-bindings/interrupt-controller/irq.h>
> @@ -162,4 +168,5 @@ gpio@c2f0000 {
> #gpio-cells = <2>;
> interrupt-controller;
> #interrupt-cells = <2>;
> + timestamp-engine = <&tegra_hte_aon>;
> };
> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
> index c026e7141e4e..6d1f15167529 100644
> --- a/drivers/gpio/gpio-tegra186.c
> +++ b/drivers/gpio/gpio-tegra186.c
> @@ -11,6 +11,7 @@
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/hte.h>
>
> #include <dt-bindings/gpio/tegra186-gpio.h>
> #include <dt-bindings/gpio/tegra194-gpio.h>
> @@ -34,6 +35,7 @@
> #define TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_LEVEL BIT(4)
> #define TEGRA186_GPIO_ENABLE_CONFIG_DEBOUNCE BIT(5)
> #define TEGRA186_GPIO_ENABLE_CONFIG_INTERRUPT BIT(6)
> +#define TEGRA186_GPIO_ENABLE_CONFIG_TIMESTAMP_FUNC BIT(7)
>
> #define TEGRA186_GPIO_DEBOUNCE_CONTROL 0x04
> #define TEGRA186_GPIO_DEBOUNCE_CONTROL_THRESHOLD(x) ((x) & 0xff)
> @@ -81,6 +83,7 @@ struct tegra_gpio {
> struct irq_chip intc;
> unsigned int num_irq;
> unsigned int *irq;
> + struct device *dev;
>
> const struct tegra_gpio_soc *soc;
> unsigned int num_irqs_per_bank;
> @@ -192,6 +195,86 @@ static int tegra186_gpio_direction_output(struct gpio_chip *chip,
> return 0;
> }
>
> +static int tegra186_gpio_req_hw_ts(struct gpio_chip *chip, unsigned int offset,
> + hte_ts_cb_t cb, hte_ts_threaded_cb_t tcb,
> + struct hte_ts_desc *hdesc, void *data)
> +{
> + struct tegra_gpio *gpio;
> + void __iomem *base;
> + int value, ret;
> +
> + if (!chip || !hdesc)
> + return -EINVAL;
> +
> + gpio = gpiochip_get_data(chip);
> + if (!gpio)
> + return -ENODEV;
> +
> + base = tegra186_gpio_get_base(gpio, offset);
> + if (WARN_ON(base == NULL))
> + return -EINVAL;
> +
> + /*
> + * HTE provider of this gpio controller does not support below gpio
> + * configs:
> + * 1. gpio as output
> + * 2. gpio as input
> + *
> + * HTE provider supports below gpio config:
> + * a. gpio as input with irq enabled
> + */
> +
> + if (tegra186_gpio_get_direction(chip, offset) ==
> + GPIO_LINE_DIRECTION_OUT)
> + return -ENOTSUPP;
> +
> + if (!gpiochip_line_is_irq(chip, offset))
> + return -ENOTSUPP;
> +
> + hdesc->con_id = offset;
> +
> + ret = hte_req_ts_by_hte_name(gpio->dev, "timestamp-engine", hdesc, cb,
> + tcb, data);
> + if (ret)
> + return ret;
> +
> + value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
> + value |= TEGRA186_GPIO_ENABLE_CONFIG_TIMESTAMP_FUNC;
> + writel(value, base + TEGRA186_GPIO_ENABLE_CONFIG);
> +
> + return 0;
> +}
> +
> +static int tegra186_gpio_rel_hw_ts(struct gpio_chip *chip,
> + unsigned int offset,
> + struct hte_ts_desc *hdesc)
> +{
> + struct tegra_gpio *gpio;
> + void __iomem *base;
> + int value, ret;
> +
> + if (!hdesc || !chip)
> + return -EINVAL;
> +
> + gpio = gpiochip_get_data(chip);
> + if (!gpio)
> + return -ENODEV;
> +
> + base = tegra186_gpio_get_base(gpio, offset);
> + if (WARN_ON(base == NULL))
> + return -EINVAL;
> +
> + ret = hte_release_ts(hdesc);
> + if (ret)
> + return ret;
> +
> + value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
> + value &= ~TEGRA186_GPIO_ENABLE_CONFIG_TIMESTAMP_FUNC;
> + writel(value, base + TEGRA186_GPIO_ENABLE_CONFIG);
> +
> + return 0;
> +}
> +
> static int tegra186_gpio_get(struct gpio_chip *chip, unsigned int offset)
> {
> struct tegra_gpio *gpio = gpiochip_get_data(chip);
> @@ -821,6 +904,12 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
> offset += port->pins;
> }
>
> + gpio->dev = &pdev->dev;
> + if (device_property_present(gpio->dev, "timestamp-engine")) {
> + gpio->gpio.req_hw_timestamp = tegra186_gpio_req_hw_ts;
> + gpio->gpio.rel_hw_timestamp = tegra186_gpio_rel_hw_ts;
> + }
> +
> return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio);
> }
>
> --
> 2.17.1
>
>