diff mbox

[5/9] gpio: vf610: Extend with wakeup support

Message ID 704b6bb97147b3d0ee1557b43d85ab9376c28666.1411404079.git.stefan@agner.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Agner Sept. 22, 2014, 5:09 p.m. UTC
Support Vybrid GPIO's as wakeup source by requesting the parent
IRQ as wakeup IRQ.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpio/gpio-vf610.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Linus Walleij Sept. 24, 2014, 9:19 a.m. UTC | #1
On Mon, Sep 22, 2014 at 7:09 PM, Stefan Agner <stefan@agner.ch> wrote:

> Support Vybrid GPIO's as wakeup source by requesting the parent
> IRQ as wakeup IRQ.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpio/gpio-vf610.c | 16 ++++++++++++++++

This driver is not yet upstream. Make wakeup support part
of the initial driver submission instead, it doesn't hurt.

Yours,
Linus Walleij
Lucas Stach Sept. 24, 2014, 10:06 a.m. UTC | #2
Hi Stefan,

Am Montag, den 22.09.2014, 19:09 +0200 schrieb Stefan Agner:
> Support Vybrid GPIO's as wakeup source by requesting the parent
> IRQ as wakeup IRQ.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpio/gpio-vf610.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
> index 5f59424..50326af 100644
> --- a/drivers/gpio/gpio-vf610.c
> +++ b/drivers/gpio/gpio-vf610.c
> @@ -196,12 +196,28 @@ static void vf610_gpio_irq_unmask(struct irq_data *d)
>  			  pcr_base);
>  }
>  
> +static int vf610_gpio_irq_set_wake(struct irq_data *d, u32 enable)
> +{
> +	struct vf610_gpio_port *port = irq_data_get_irq_chip_data(d);
> +
> +	if (enable)
> +		enable_irq_wake(port->irq);
> +	else
> +		disable_irq_wake(port->irq);
> +
> +	return 0;
> +}
> +
> +
>  static struct irq_chip vf610_gpio_irq_chip = {
>  	.name		= "gpio-vf610",
>  	.irq_ack	= vf610_gpio_irq_ack,
>  	.irq_mask	= vf610_gpio_irq_mask,
>  	.irq_unmask	= vf610_gpio_irq_unmask,
>  	.irq_set_type	= vf610_gpio_irq_set_type,
> +#ifdef CONFIG_PM_SLEEP
> +	.irq_set_wake	= vf610_gpio_irq_set_wake,
> +#endif

Either you need to get rid of this #ifdef and always assign this
function or you need to wrap the function itself into the same #ifdef.
Otherwise you provoke a unused function warning with !CONFIG_PM_SLEEP.

Given that this function isn't really that big I would argue to just
drop the #ifdef.

Regards,
Lucas
Stefan Agner Sept. 24, 2014, 4:33 p.m. UTC | #3
Am 2014-09-24 11:19, schrieb Linus Walleij:
> On Mon, Sep 22, 2014 at 7:09 PM, Stefan Agner <stefan@agner.ch> wrote:
> 
>> Support Vybrid GPIO's as wakeup source by requesting the parent
>> IRQ as wakeup IRQ.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpio/gpio-vf610.c | 16 ++++++++++++++++
> 
> This driver is not yet upstream. Make wakeup support part
> of the initial driver submission instead, it doesn't hurt.

Ok, will do. 

Just realized that this patch even leads to compile errors when applying
on top of v2 of the GPIO driver; we need the irq in drivers struct here,
and I removed that in v2...  :-)

--
Stefan
Stefan Agner Sept. 24, 2014, 4:51 p.m. UTC | #4
Am 2014-09-24 12:06, schrieb Lucas Stach:
> Hi Stefan,
> 
> Am Montag, den 22.09.2014, 19:09 +0200 schrieb Stefan Agner:
>> Support Vybrid GPIO's as wakeup source by requesting the parent
>> IRQ as wakeup IRQ.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpio/gpio-vf610.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
>> index 5f59424..50326af 100644
>> --- a/drivers/gpio/gpio-vf610.c
>> +++ b/drivers/gpio/gpio-vf610.c
>> @@ -196,12 +196,28 @@ static void vf610_gpio_irq_unmask(struct irq_data *d)
>>  			  pcr_base);
>>  }
>>
>> +static int vf610_gpio_irq_set_wake(struct irq_data *d, u32 enable)
>> +{
>> +	struct vf610_gpio_port *port = irq_data_get_irq_chip_data(d);
>> +
>> +	if (enable)
>> +		enable_irq_wake(port->irq);
>> +	else
>> +		disable_irq_wake(port->irq);
>> +
>> +	return 0;
>> +}
>> +
>> +
>>  static struct irq_chip vf610_gpio_irq_chip = {
>>  	.name		= "gpio-vf610",
>>  	.irq_ack	= vf610_gpio_irq_ack,
>>  	.irq_mask	= vf610_gpio_irq_mask,
>>  	.irq_unmask	= vf610_gpio_irq_unmask,
>>  	.irq_set_type	= vf610_gpio_irq_set_type,
>> +#ifdef CONFIG_PM_SLEEP
>> +	.irq_set_wake	= vf610_gpio_irq_set_wake,
>> +#endif
> 
> Either you need to get rid of this #ifdef and always assign this
> function or you need to wrap the function itself into the same #ifdef.
> Otherwise you provoke a unused function warning with !CONFIG_PM_SLEEP.
> 
> Given that this function isn't really that big I would argue to just
> drop the #ifdef.
> 

Good point, thx! I will drop the #ifdef when including that patch to the
GPIO driver (as Linus Walleij suggested).

--
Stefan
diff mbox

Patch

diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index 5f59424..50326af 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -196,12 +196,28 @@  static void vf610_gpio_irq_unmask(struct irq_data *d)
 			  pcr_base);
 }
 
+static int vf610_gpio_irq_set_wake(struct irq_data *d, u32 enable)
+{
+	struct vf610_gpio_port *port = irq_data_get_irq_chip_data(d);
+
+	if (enable)
+		enable_irq_wake(port->irq);
+	else
+		disable_irq_wake(port->irq);
+
+	return 0;
+}
+
+
 static struct irq_chip vf610_gpio_irq_chip = {
 	.name		= "gpio-vf610",
 	.irq_ack	= vf610_gpio_irq_ack,
 	.irq_mask	= vf610_gpio_irq_mask,
 	.irq_unmask	= vf610_gpio_irq_unmask,
 	.irq_set_type	= vf610_gpio_irq_set_type,
+#ifdef CONFIG_PM_SLEEP
+	.irq_set_wake	= vf610_gpio_irq_set_wake,
+#endif
 };
 
 static int vf610_gpio_probe(struct platform_device *pdev)