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

[Dipen Patel <dipenp@xxxxxxxxxx>]

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 <dipenp@xxxxxxxxxx>
>> ---
>>  .../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
>>
>>