All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: evgreen@chromium.org, marc.zyngier@arm.com
Cc: linux-kernel@vger.kernel.org, rplsssn@codeaurora.org,
	linux-arm-msm@vger.kernel.org, thierry.reding@gmail.com,
	bjorn.andersson@linaro.org, dianders@chromium.org,
	linus.walleij@linaro.org, Lina Iyer <ilina@codeaurora.org>
Subject: Re: [PATCH v3 6/9] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
Date: Thu, 07 Mar 2019 16:59:34 -0800	[thread overview]
Message-ID: <155200677430.20095.14337964669558394653@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <20190222221850.26939-7-ilina@codeaurora.org>

Quoting Lina Iyer (2019-02-22 14:18:47)
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index ee8119879c4c..83053b45982e 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -890,6 +918,9 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
>  
>         raw_spin_unlock_irqrestore(&pctrl->lock, flags);

Can we remove the above hunk of code in this function? From what I can
tell we're trying to set wake on the GIC SPI line and that won't ever
work because it doesn't support wakeup support. Furthermore, I get a
lockdep splat when I try to set wake on a GPIO irq with this patchset
applied. It's probably a false positive though because it's warning
about an AA deadlock with the irq descriptor lock, which is aliased
between the two controllers.

>  
> +       if (d->parent_data)
> +               irq_chip_set_wake_parent(d, on);
> +
>         return 0;
>  }
>  

Here's a patch on top. If you need my signed-off-by please feel free to
add it:

Signed-off-by: Stephen Boyd <swboyd@chromium.org>

-----8<-------
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index cdbc06ec5e96..4e81dad80a4d 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -852,20 +852,10 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 
 static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 {
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&pctrl->lock, flags);
-
-	irq_set_irq_wake(pctrl->irq, on);
-
-	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
-
 	if (d->parent_data)
-		irq_chip_set_wake_parent(d, on);
+		return irq_chip_set_wake_parent(d, on);
 
-	return 0;
+	return -ENOSYS;
 }
 
 static int msm_gpio_irq_reqres(struct irq_data *d)

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <swboyd@chromium.org>
To: Lina Iyer <ilina@codeaurora.org>,
	evgreen@chromium.org, marc.zyngier@arm.com
Cc: linux-kernel@vger.kernel.org, rplsssn@codeaurora.org,
	linux-arm-msm@vger.kernel.org, thierry.reding@gmail.com,
	bjorn.andersson@linaro.org, dianders@chromium.org,
	linus.walleij@linaro.org, Lina Iyer <ilina@codeaurora.org>
Subject: Re: [PATCH v3 6/9] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
Date: Thu, 07 Mar 2019 16:59:34 -0800	[thread overview]
Message-ID: <155200677430.20095.14337964669558394653@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <20190222221850.26939-7-ilina@codeaurora.org>

Quoting Lina Iyer (2019-02-22 14:18:47)
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index ee8119879c4c..83053b45982e 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -890,6 +918,9 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
>  
>         raw_spin_unlock_irqrestore(&pctrl->lock, flags);

Can we remove the above hunk of code in this function? From what I can
tell we're trying to set wake on the GIC SPI line and that won't ever
work because it doesn't support wakeup support. Furthermore, I get a
lockdep splat when I try to set wake on a GPIO irq with this patchset
applied. It's probably a false positive though because it's warning
about an AA deadlock with the irq descriptor lock, which is aliased
between the two controllers.

>  
> +       if (d->parent_data)
> +               irq_chip_set_wake_parent(d, on);
> +
>         return 0;
>  }
>  

Here's a patch on top. If you need my signed-off-by please feel free to
add it:

Signed-off-by: Stephen Boyd <swboyd@chromium.org>

-----8<-------
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index cdbc06ec5e96..4e81dad80a4d 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -852,20 +852,10 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 
 static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 {
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&pctrl->lock, flags);
-
-	irq_set_irq_wake(pctrl->irq, on);
-
-	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
-
 	if (d->parent_data)
-		irq_chip_set_wake_parent(d, on);
+		return irq_chip_set_wake_parent(d, on);
 
-	return 0;
+	return -ENOSYS;
 }
 
 static int msm_gpio_irq_reqres(struct irq_data *d)

  reply	other threads:[~2019-03-08  0:59 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-22 22:18 [PATCH v3 0/9] qcom: support wakeup capable GPIOs Lina Iyer
2019-02-22 22:18 ` [PATCH v3 1/9] gpio: Add support for hierarchical IRQ domains Lina Iyer
2019-02-22 22:18 ` [PATCH v3 2/9] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
2019-03-11 10:07   ` Marc Zyngier
2019-02-22 22:18 ` [PATCH v3 3/9] of: irq: add helper to remap interrupts to another irqdomain Lina Iyer
2019-03-11 10:51   ` Marc Zyngier
2019-02-22 22:18 ` [PATCH v3 4/9] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Lina Iyer
2019-03-11 10:41   ` Marc Zyngier
2019-03-12 16:38     ` Lina Iyer
2019-02-22 22:18 ` [PATCH v3 5/9] dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO Lina Iyer
2019-02-22 22:18 ` [PATCH v3 6/9] drivers: pinctrl: msm: setup GPIO irqchip hierarchy Lina Iyer
2019-03-08  0:59   ` Stephen Boyd [this message]
2019-03-08  0:59     ` Stephen Boyd
2019-03-08 22:30     ` Lina Iyer
2019-03-11 10:54   ` Marc Zyngier
2019-03-12 16:35     ` Lina Iyer
2019-02-22 22:18 ` [PATCH v3 7/9] arm64: dts: qcom: add PDC interrupt controller for SDM845 Lina Iyer
2019-02-22 22:18 ` [PATCH v3 8/9] arm64: dts: qcom: setup PDC as wakeup parent for GPIOs " Lina Iyer
2019-02-22 22:18 ` [PATCH v3 9/9] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845 Lina Iyer
2019-03-11 11:09 ` [PATCH v3 0/9] qcom: support wakeup capable GPIOs Marc Zyngier
2019-03-11 22:26   ` Lina Iyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=155200677430.20095.14337964669558394653@swboyd.mtv.corp.google.com \
    --to=swboyd@chromium.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=dianders@chromium.org \
    --cc=evgreen@chromium.org \
    --cc=ilina@codeaurora.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=rplsssn@codeaurora.org \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.