by Dipen Patel

[permalink] [raw]

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

Hi,

On 10/8/21 3:16 PM, Rob Herring wrote:
> 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 <[email protected]>
>> ---
>> .../bindings/gpio/nvidia,tegra186-gpio.txt | 7 ++
> Bindings should be a separate patch. Consider converting this to schema
> first too.

I will create separate patch. For the schema, I guess converting is out of the

scope in this patch series. I believe its better to create separate patch for that

task.

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

The mapping is done in the GPIO HTE provider if its not one-to-one. An example is

hte-tegra194.c (tegra194_aon_gpio_map) in this patch series.

>> + 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
>>
>>