[PATCH 1/2] gpio: tegra186: Implement system suspend/resume support

[PATCH 1/2] gpio: tegra186: Implement system suspend/resume support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Backup GPIO control registers on suspend and restore them on resume to
ensure that the GPIOs' configuration remains the same across suspend and
resume.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpio/gpio-tegra186.c | 51 ++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index a9058fda187e..3ded6ba2f997 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -64,6 +64,12 @@ struct tegra_gpio {
 	const struct tegra_gpio_soc *soc;
 
 	void __iomem *base;
+
+	struct tegra_gpio_context {
+		u32 value;
+		u32 control;
+		u32 config;
+	} *context;
 };
 
 static const struct tegra_gpio_port *
@@ -455,6 +461,11 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	for (i = 0; i < gpio->soc->num_ports; i++)
 		gpio->gpio.ngpio += gpio->soc->ports[i].pins;
 
+	gpio->context = devm_kmalloc_array(gpio->gpio.parent, gpio->gpio.ngpio,
+					   sizeof(*gpio->context), GFP_KERNEL);
+	if (!gpio->context)
+		return -ENOMEM;
+
 	names = devm_kcalloc(gpio->gpio.parent, gpio->gpio.ngpio,
 			     sizeof(*names), GFP_KERNEL);
 	if (!names)
@@ -526,6 +537,45 @@ static int tegra186_gpio_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int tegra186_gpio_suspend(struct device *dev)
+{
+	struct tegra_gpio *gpio = dev_get_drvdata(dev);
+	unsigned int i;
+
+	for (i = 0; i < gpio->gpio.ngpio; i++) {
+		struct tegra_gpio_context *context = &gpio->context[i];
+		void __iomem *base = tegra186_gpio_get_base(gpio, i);
+
+		context->config = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
+		context->control = readl(base + TEGRA186_GPIO_OUTPUT_CONTROL);
+		context->value = readl(base + TEGRA186_GPIO_OUTPUT_VALUE);
+	}
+
+	return 0;
+}
+
+static int tegra186_gpio_resume(struct device *dev)
+{
+	struct tegra_gpio *gpio = dev_get_drvdata(dev);
+	unsigned int i;
+
+	for (i = 0; i < gpio->gpio.ngpio; i++) {
+		struct tegra_gpio_context *context = &gpio->context[i];
+		void __iomem *base = tegra186_gpio_get_base(gpio, i);
+
+		writel(context->value, base + TEGRA186_GPIO_OUTPUT_VALUE);
+		writel(context->control, base + TEGRA186_GPIO_OUTPUT_CONTROL);
+		writel(context->config, base + TEGRA186_GPIO_ENABLE_CONFIG);
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops tegra186_gpio_pm = {
+	.suspend_late = tegra186_gpio_suspend,
+	.resume_early = tegra186_gpio_resume,
+};
+
 #define TEGRA186_MAIN_GPIO_PORT(port, base, count, controller)	\
 	[TEGRA186_MAIN_GPIO_PORT_##port] = {			\
 		.name = #port,					\
@@ -680,6 +730,7 @@ static struct platform_driver tegra186_gpio_driver = {
 	.driver = {
 		.name = "tegra186-gpio",
 		.of_match_table = tegra186_gpio_of_match,
+		.pm = &tegra186_gpio_pm,
 	},
 	.probe = tegra186_gpio_probe,
 	.remove = tegra186_gpio_remove,
-- 
2.23.0

[PATCH v5 2/2] gpio: tegra186: Implement wake event support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The GPIO controller doesn't have any controls to enable the system to
wake up from low power states based on activity on GPIO pins. An extra
hardware block that is part of the power management controller (PMC)
contains these controls. In order for the GPIO controller to be able
to cooperate with the PMC, obtain a reference to the PMC's IRQ domain
and make it a parent to the GPIO controller's IRQ domain. This way the
PMC gets an opportunity to program the additional registers required
to enable wakeup sources on suspend.

Based on additional work by Bitan Biswas <bbiswas@nvidia.com>.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- rebase on top of new hierarchical IRQ domain support

Changes in v4:
- implement gpio_irq_chip.to_fwspec() instead of gpio_chip.to_irq()
- initialize missing irq_domain_ops.free()

Changes in v3:
- use irq_create_fwspec_mapping() instead of irq_domain_alloc_irqs()
- initialize tegra_gpio_soc.instance field on Tegra186

Changes in v2:
- don't use wakeup-parent property but look up PMC by compatible

 drivers/gpio/Kconfig         |  1 +
 drivers/gpio/gpio-tegra186.c | 97 +++++++++++++++++++++++++++++-------
 2 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 38e096e6925f..1d1c79795db2 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -531,6 +531,7 @@ config GPIO_TEGRA186
 	depends on ARCH_TEGRA_186_SOC || COMPILE_TEST
 	depends on OF_GPIO
 	select GPIOLIB_IRQCHIP
+	select IRQ_DOMAIN_HIERARCHY
 	help
 	  Say yes here to support GPIO pins on NVIDIA Tegra186 SoCs.
 
diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 3ded6ba2f997..bf1968b77a7b 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -53,6 +53,7 @@ struct tegra_gpio_soc {
 	const struct tegra_gpio_port *ports;
 	unsigned int num_ports;
 	const char *name;
+	unsigned int instance;
 };
 
 struct tegra_gpio {
@@ -333,7 +334,7 @@ static int tegra186_irq_set_type(struct irq_data *data, unsigned int type)
 	else
 		irq_set_handler_locked(data, handle_edge_irq);
 
-	return 0;
+	return irq_chip_set_type_parent(data, type);
 }
 
 static void tegra186_gpio_irq(struct irq_desc *desc)
@@ -373,39 +374,80 @@ static void tegra186_gpio_irq(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
-static int tegra186_gpio_irq_domain_xlate(struct irq_domain *domain,
-					  struct device_node *np,
-					  const u32 *spec, unsigned int size,
-					  unsigned long *hwirq,
-					  unsigned int *type)
+static int tegra186_gpio_irq_domain_translate(struct irq_domain *domain,
+					      struct irq_fwspec *fwspec,
+					      unsigned long *hwirq,
+					      unsigned int *type)
 {
 	struct tegra_gpio *gpio = gpiochip_get_data(domain->host_data);
 	unsigned int port, pin, i, offset = 0;
 
-	if (size < 2)
+	if (WARN_ON(gpio->gpio.of_gpio_n_cells < 2))
+		return -EINVAL;
+
+	if (WARN_ON(fwspec->param_count < gpio->gpio.of_gpio_n_cells))
 		return -EINVAL;
 
-	port = spec[0] / 8;
-	pin = spec[0] % 8;
+	port = fwspec->param[0] / 8;
+	pin = fwspec->param[0] % 8;
 
-	if (port >= gpio->soc->num_ports) {
-		dev_err(gpio->gpio.parent, "invalid port number: %u\n", port);
+	if (port >= gpio->soc->num_ports)
 		return -EINVAL;
-	}
 
 	for (i = 0; i < port; i++)
 		offset += gpio->soc->ports[i].pins;
 
-	*type = spec[1] & IRQ_TYPE_SENSE_MASK;
+	*type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
 	*hwirq = offset + pin;
 
 	return 0;
 }
 
-static const struct irq_domain_ops tegra186_gpio_irq_domain_ops = {
-	.map = gpiochip_irq_map,
-	.unmap = gpiochip_irq_unmap,
-	.xlate = tegra186_gpio_irq_domain_xlate,
+static void tegra186_gpio_populate_parent_fwspec(struct gpio_chip *chip,
+						 struct irq_fwspec *fwspec,
+						 unsigned int parent_hwirq,
+						 unsigned int parent_type)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+
+	fwspec->param_count = 3;
+	fwspec->param[0] = gpio->soc->instance;
+	fwspec->param[1] = parent_hwirq;
+	fwspec->param[2] = parent_type;
+}
+
+static int tegra186_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
+					       unsigned int hwirq,
+					       unsigned int type,
+					       unsigned int *parent_hwirq,
+					       unsigned int *parent_type)
+{
+	*parent_hwirq = chip->irq.child_offset_to_irq(chip, hwirq);
+	*parent_type = type;
+
+	return 0;
+}
+
+static unsigned int tegra186_gpio_child_offset_to_irq(struct gpio_chip *chip,
+						      unsigned int offset)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	unsigned int i;
+
+	for (i = 0; i < gpio->soc->num_ports; i++) {
+		if (offset < gpio->soc->ports[i].pins)
+			break;
+
+		offset -= gpio->soc->ports[i].pins;
+	}
+
+	return offset + i * 8;
+}
+
+static const struct of_device_id tegra186_pmc_of_match[] = {
+	{ .compatible = "nvidia,tegra186-pmc" },
+	{ .compatible = "nvidia,tegra194-pmc" },
+	{ /* sentinel */ }
 };
 
 static int tegra186_gpio_probe(struct platform_device *pdev)
@@ -413,6 +455,7 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	unsigned int i, j, offset;
 	struct gpio_irq_chip *irq;
 	struct tegra_gpio *gpio;
+	struct device_node *np;
 	struct resource *res;
 	char **names;
 	int err;
@@ -498,10 +541,15 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	gpio->intc.irq_mask = tegra186_irq_mask;
 	gpio->intc.irq_unmask = tegra186_irq_unmask;
 	gpio->intc.irq_set_type = tegra186_irq_set_type;
+	gpio->intc.irq_set_wake = irq_chip_set_wake_parent;
 
 	irq = &gpio->gpio.irq;
 	irq->chip = &gpio->intc;
-	irq->domain_ops = &tegra186_gpio_irq_domain_ops;
+	irq->fwnode = of_node_to_fwnode(pdev->dev.of_node);
+	irq->child_to_parent_hwirq = tegra186_gpio_child_to_parent_hwirq;
+	irq->populate_parent_fwspec = tegra186_gpio_populate_parent_fwspec;
+	irq->child_offset_to_irq = tegra186_gpio_child_offset_to_irq;
+	irq->child_irq_domain_ops.translate = tegra186_gpio_irq_domain_translate;
 	irq->handler = handle_simple_irq;
 	irq->default_type = IRQ_TYPE_NONE;
 	irq->parent_handler = tegra186_gpio_irq;
@@ -509,6 +557,15 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	irq->num_parents = gpio->num_irq;
 	irq->parents = gpio->irq;
 
+	np = of_find_matching_node(NULL, tegra186_pmc_of_match);
+	if (np) {
+		irq->parent_domain = irq_find_host(np);
+		of_node_put(np);
+
+		if (!irq->parent_domain)
+			return -EPROBE_DEFER;
+	}
+
 	irq->map = devm_kcalloc(&pdev->dev, gpio->gpio.ngpio,
 				sizeof(*irq->map), GFP_KERNEL);
 	if (!irq->map)
@@ -614,6 +671,7 @@ static const struct tegra_gpio_soc tegra186_main_soc = {
 	.num_ports = ARRAY_SIZE(tegra186_main_ports),
 	.ports = tegra186_main_ports,
 	.name = "tegra186-gpio",
+	.instance = 0,
 };
 
 #define TEGRA186_AON_GPIO_PORT(port, base, count, controller)	\
@@ -639,6 +697,7 @@ static const struct tegra_gpio_soc tegra186_aon_soc = {
 	.num_ports = ARRAY_SIZE(tegra186_aon_ports),
 	.ports = tegra186_aon_ports,
 	.name = "tegra186-gpio-aon",
+	.instance = 1,
 };
 
 #define TEGRA194_MAIN_GPIO_PORT(port, base, count, controller)	\
@@ -684,6 +743,7 @@ static const struct tegra_gpio_soc tegra194_main_soc = {
 	.num_ports = ARRAY_SIZE(tegra194_main_ports),
 	.ports = tegra194_main_ports,
 	.name = "tegra194-gpio",
+	.instance = 0,
 };
 
 #define TEGRA194_AON_GPIO_PORT(port, base, count, controller)	\
@@ -706,6 +766,7 @@ static const struct tegra_gpio_soc tegra194_aon_soc = {
 	.num_ports = ARRAY_SIZE(tegra194_aon_ports),
 	.ports = tegra194_aon_ports,
 	.name = "tegra194-gpio-aon",
+	.instance = 1,
 };
 
 static const struct of_device_id tegra186_gpio_of_match[] = {
-- 
2.23.0

Re: [PATCH 1/2] gpio: tegra186: Implement system suspend/resume support

[Bitan Biswas]

On 10/2/19 7:45 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Backup GPIO control registers on suspend and restore them on resume to
> ensure that the GPIOs' configuration remains the same across suspend and
> resume.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>   drivers/gpio/gpio-tegra186.c | 51 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
> index a9058fda187e..3ded6ba2f997 100644
> --- a/drivers/gpio/gpio-tegra186.c
> +++ b/drivers/gpio/gpio-tegra186.c
> @@ -64,6 +64,12 @@ struct tegra_gpio {
>   	const struct tegra_gpio_soc *soc;
>   
>   	void __iomem *base;
> +
> +	struct tegra_gpio_context {
> +		u32 value;
> +		u32 control;
> +		u32 config;
> +	} *context;
>   };
>   
>   static const struct tegra_gpio_port *
> @@ -455,6 +461,11 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
>   	for (i = 0; i < gpio->soc->num_ports; i++)
>   		gpio->gpio.ngpio += gpio->soc->ports[i].pins;
>   
> +	gpio->context = devm_kmalloc_array(gpio->gpio.parent, gpio->gpio.ngpio,
> +					   sizeof(*gpio->context), GFP_KERNEL);
> +	if (!gpio->context)
> +		return -ENOMEM;
> +
>   	names = devm_kcalloc(gpio->gpio.parent, gpio->gpio.ngpio,
>   			     sizeof(*names), GFP_KERNEL);
>   	if (!names)
> @@ -526,6 +537,45 @@ static int tegra186_gpio_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +static int tegra186_gpio_suspend(struct device *dev)
> +{
> +	struct tegra_gpio *gpio = dev_get_drvdata(dev);
> +	unsigned int i;
> +
> +	for (i = 0; i < gpio->gpio.ngpio; i++) {
> +		struct tegra_gpio_context *context = &gpio->context[i];
> +		void __iomem *base = tegra186_gpio_get_base(gpio, i);
> +
> +		context->config = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
> +		context->control = readl(base + TEGRA186_GPIO_OUTPUT_CONTROL);
> +		context->value = readl(base + TEGRA186_GPIO_OUTPUT_VALUE);
> +	}
> +
> +	return 0;
> +}
> +
> +static int tegra186_gpio_resume(struct device *dev)
> +{
> +	struct tegra_gpio *gpio = dev_get_drvdata(dev);
> +	unsigned int i;
> +
> +	for (i = 0; i < gpio->gpio.ngpio; i++) {
> +		struct tegra_gpio_context *context = &gpio->context[i];
> +		void __iomem *base = tegra186_gpio_get_base(gpio, i);
> +
> +		writel(context->value, base + TEGRA186_GPIO_OUTPUT_VALUE);
> +		writel(context->control, base + TEGRA186_GPIO_OUTPUT_CONTROL);
> +		writel(context->config, base + TEGRA186_GPIO_ENABLE_CONFIG);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops tegra186_gpio_pm = {
> +	.suspend_late = tegra186_gpio_suspend,
> +	.resume_early = tegra186_gpio_resume,
> +};
> +
>   #define TEGRA186_MAIN_GPIO_PORT(port, base, count, controller)	\
>   	[TEGRA186_MAIN_GPIO_PORT_##port] = {			\
>   		.name = #port,					\
> @@ -680,6 +730,7 @@ static struct platform_driver tegra186_gpio_driver = {
>   	.driver = {
>   		.name = "tegra186-gpio",
>   		.of_match_table = tegra186_gpio_of_match,
> +		.pm = &tegra186_gpio_pm,
>   	},
>   	.probe = tegra186_gpio_probe,
>   	.remove = tegra186_gpio_remove,
> 

I see jetson Xavier RTC wakeup test fail with this patch. It no longer 
reaches the UART shell after suspend exit. Jetson-TX2 works fine with 
this patch. There seems to be some hang on Xavier.


-regards,
  Bitan

Re: [PATCH 1/2] gpio: tegra186: Implement system suspend/resume support

[Linus Walleij]

On Thu, Oct 3, 2019 at 1:59 PM Bitan Biswas <bbiswas@nvidia.com> wrote:

> I see jetson Xavier RTC wakeup test fail with this patch. It no longer
> reaches the UART shell after suspend exit. Jetson-TX2 works fine with
> this patch. There seems to be some hang on Xavier.

That sounds like some very typical situation where you need proper
wakeup handling, so that the chip does not suspend if any irq lines
are flagged as wakeup-capable and in use.

Yours,
Linus Walleij

Re: [PATCH v5 2/2] gpio: tegra186: Implement wake event support

[Linus Walleij]

On Wed, Oct 2, 2019 at 4:45 PM Thierry Reding <thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> The GPIO controller doesn't have any controls to enable the system to
> wake up from low power states based on activity on GPIO pins. An extra
> hardware block that is part of the power management controller (PMC)
> contains these controls. In order for the GPIO controller to be able
> to cooperate with the PMC, obtain a reference to the PMC's IRQ domain
> and make it a parent to the GPIO controller's IRQ domain. This way the
> PMC gets an opportunity to program the additional registers required
> to enable wakeup sources on suspend.
>
> Based on additional work by Bitan Biswas <bbiswas@nvidia.com>.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v5:
> - rebase on top of new hierarchical IRQ domain support

Patch applied!

Also a special thanks for standing out to my sometimes flimsy
and handwavy engineering, and I'm happy we landed this the
way I imagined it. Sort of.

Yours,
Linus Walleij