lkml.org 
[lkml]   [2019]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V2 10/12] gpio: tegra: implement wake event support for Tegra210 and prior GPIO
From
Date

On 5/29/19 7:03 AM, Thierry Reding wrote:
> On Tue, May 28, 2019 at 04:08:54PM -0700, Sowjanya Komatineni wrote:
>> 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.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/gpio/gpio-tegra.c | 116 +++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 110 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
>> index 6d9b6906b9d0..5190129668d3 100644
>> --- a/drivers/gpio/gpio-tegra.c
>> +++ b/drivers/gpio/gpio-tegra.c
>> @@ -32,6 +32,8 @@
>> #include <linux/pinctrl/consumer.h>
>> #include <linux/pm.h>
>>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> #define GPIO_BANK(x) ((x) >> 5)
>> #define GPIO_PORT(x) (((x) >> 3) & 0x3)
>> #define GPIO_BIT(x) ((x) & 0x7)
>> @@ -275,8 +277,22 @@ static int tegra_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
>> static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
>> {
>> struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
>> + struct irq_domain *domain = tgi->irq_domain;
>> +
>> + if (!gpiochip_irqchip_irq_valid(chip, offset))
>> + return -ENXIO;
>> +
>> + if (irq_domain_is_hierarchy(domain)) {
>> + struct irq_fwspec spec;
>> +
>> + spec.fwnode = domain->fwnode;
>> + spec.param_count = 2;
>> + spec.param[0] = offset;
>> + spec.param[1] = IRQ_TYPE_NONE;
>> + return irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, &spec);
> This looks like it was copied from the equivalent Tegra186 patch. I have
> since then changed the implementation, based on feedback by Linus, to
> not call irq_domain_alloc_irqs() here and instead call
> irq_create_fwspec_mapping(). This has the advantage of not requiring the
> irq_domain_alloc_irqs() function to be exported. It ends up calling that
> function internally, but as discussed with Linus it's also a nicer way
> to create these mappings.
>
existing gpio-tegra driver maps hwirq to virtual interrupt number using
irq_create_mapping during gpio probe

and irq_create_fwspec_mapping() will always return virq number as virq
number exists already and irq_domain_alloc_irqs doesn't happen.

So I was using irq_domain_alloc_irqs().


>> + }
>>
>> - return irq_find_mapping(tgi->irq_domain, offset);
>> + return irq_find_mapping(domain, offset);
>> }
>>
>> static void tegra_gpio_irq_ack(struct irq_data *d)
>> @@ -365,7 +381,10 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>> else if (type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
>> irq_set_handler_locked(d, handle_edge_irq);
>>
>> - return 0;
>> + if (d->parent_data)
>> + return irq_chip_set_type_parent(d, type);
>> + else
>> + return 0;
> There's no need for this final else. Just make it a regular "return 0;"
> at the end of the function, without the extra else branch.
>
>> }
>>
>> static void tegra_gpio_irq_shutdown(struct irq_data *d)
>> @@ -503,6 +522,7 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
>> struct tegra_gpio_bank *bank = irq_data_get_irq_chip_data(d);
>> unsigned int gpio = d->hwirq;
>> u32 port, bit, mask;
>> + int ret;
>>
>> port = GPIO_PORT(gpio);
>> bit = GPIO_BIT(gpio);
>> @@ -513,7 +533,14 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
>> else
>> bank->wake_enb[port] &= ~mask;
>>
>> - return irq_set_irq_wake(bank->irq, enable);
>> + ret = irq_set_irq_wake(bank->irq, enable);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (d->parent_data)
>> + return irq_chip_set_wake_parent(d, enable);
>> + else
>> + return 0;
> Same here.
>
> Thierry

\
 
 \ /
  Last update: 2019-06-01 10:29    [W:0.086 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site