irqchip: qcom: pdc: Introduce irq_set_wake call

PATCH In review Maulik Shah 4 participants 9 messages May 22, 2020 – May 25, 2020

Subsystems (6)

Arm Qualcomm mailing list GPIO Pin control irqchip drivers +2 more subsystems

lore

Share & tools

lore

Maulik Shah

author

2020-05-22 13:20 UTC #0 lore raw

cover irqchip: qcom: pdc: Introduce irq_set_wake call

This series adds support to lazy disable pdc interrupt.

Some drivers using gpio interrupts want to configure gpio for wakeup using enable_irq_wake() but during suspend entry disables irq and expects system to resume when interrupt occurs. In the driver resume call interrupt is re-enabled and removes wakeup capability using disable_irq_wake() one such example is cros ec driver.

With [1] in documentation saying "An irq can be disabled with disable_irq() and still wake the system as long as the irq has wake enabled".

The PDC IRQs are currently "unlazy disabled" (disable here means that it will be masked in PDC & GIC HW GICD_ISENABLER, the moment driver invokes disable_irq()) such IRQs can not wakeup from low power modes like suspend to RAM since the driver chosen to disable this.

During suspend entry, no one re-enable/unmask in HW, even if its marked for wakeup.

One solutions thought to address this problem was...During suspend entry at last point, irq chip driver re-enable/unmask IRQs in HW that are marked for wakeup. This was attemped in [2].

This series adds alternate solution to [2] by "lazy disable" IRQs in HW. The genirq takes care of lazy disable in case if irqchip did not implement irq_disable callback. Below is high level steps on how this works out..

a. During driver's disable_irq() call, IRQ will be marked disabled in SW b. IRQ will still be enabled(read unmasked in HW)
c. The device then enters low power mode like suspend to RAM d. The HW detects unmasked IRQs and wakesup the CPU
e. During resume after local_irq_enable() CPU goes to handle the wake IRQ f. Generic handler comes to know that IRQ is disabled in SW
g. Generic handler marks IRQ as pending and now invokes mask callback h. IRQ gets disabled/masked in HW now
i. When driver invokes enable_irq() the SW pending IRQ leads IRQ's handler j. enable_irq() will again enable/unmask in HW

[1] https://www.spinics.net/lists/kernel/msg3398294.html
[2] https://patchwork.kernel.org/patch/11466021/

Maulik Shah (4):
gpio: gpiolib: Allow GPIO IRQs to lazy disable
pinctrl: qcom: Remove irq_disable callback from msmgpio irqchip
pinctrl: qcom: Add msmgpio irqchip flags
irqchip: qcom-pdc: Introduce irq_set_wake call

 drivers/gpio/gpiolib.c             | 59 ++++++++++++++++++++++++--------------
 drivers/irqchip/qcom-pdc.c         | 33 ++++++++++-----------
 drivers/pinctrl/qcom/pinctrl-msm.c | 15 ++--------
 include/linux/gpio/driver.h        | 13 +++++++++
 4 files changed, 70 insertions(+), 50 deletions(-)

Signature · 2 lines

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Maulik Shah

author

2020-05-22 13:20 UTC #1 lore raw

PATCH 1/4 gpio: gpiolib: Allow GPIO IRQs to lazy disable

With 'commit 461c1a7d4733 ("gpiolib: override irq_enable/disable")' gpiolib overrides irqchip's irq_enable and irq_disable callbacks. If irq_disable callback is implemented then genirq takes unlazy path to disable irq.

Underlying irqchip may not want to implement irq_disable callback to lazy disable irq when client drivers invokes disable_irq(). By overriding irq_disable callback, gpiolib ends up always unlazy disabling IRQ.

Allow gpiolib to lazy disable IRQs by overriding irq_disable callback only if irqchip implemented irq_disable. In cases where irq_disable is not implemented irq_mask is overridden. Similarly override irq_enable callback only if irqchip implemented irq_enable otherwise irq_unmask is overridden.

Fixes 461c1a7d47 (gpiolib: override irq_enable/disable)

Signed-off-by Maulik Shah <[email protected]>

---
 drivers/gpio/gpiolib.c      | 59 +++++++++++++++++++++++++++++----------------
 include/linux/gpio/driver.h | 13 ++++++++++
 2 files changed, 51 insertions(+), 21 deletions(-)

drivers/gpio/gpiolib.c +38 ~~-21~~

index eaa0e20..a8fdc74 100644
@@ -2465,33 +2465,38 @@ static void gpiochip_irq_relres(struct irq_data *d)
 	gpiochip_relres_irq(gc, d->hwirq);
 }
 
+static void gpiochip_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	if (chip->irq.irq_mask)
+		chip->irq.irq_mask(d);
+	gpiochip_disable_irq(chip, d->hwirq);
+}
+
+static void gpiochip_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	gpiochip_enable_irq(chip, d->hwirq);
+	if (chip->irq.irq_unmask)
+		chip->irq.irq_unmask(d);
+}
+
 static void gpiochip_irq_enable(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 
-	gpiochip_enable_irq(gc, d->hwirq);
-	if (gc->irq.irq_enable)
-		gc->irq.irq_enable(d);
-	else
-		gc->irq.chip->irq_unmask(d);
+	gpiochip_enable_irq(chip, d->hwirq);
+	chip->irq.irq_enable(d);
 }
 
 static void gpiochip_irq_disable(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 
-	/*
-	 * Since we override .irq_disable() we need to mimic the
-	 * behaviour of __irq_disable() in irq/chip.c.
-	 * First call .irq_disable() if it exists, else mimic the
-	 * behaviour of mask_irq() which calls .irq_mask() if
-	 * it exists.
-	 */
-	if (gc->irq.irq_disable)
-		gc->irq.irq_disable(d);
-	else if (gc->irq.chip->irq_mask)
-		gc->irq.chip->irq_mask(d);
-	gpiochip_disable_irq(gc, d->hwirq);
+	chip->irq.irq_disable(d);
+	gpiochip_disable_irq(chip, d->hwirq);
 }
 
 static void gpiochip_set_irq_hooks(struct gpio_chip *gc)

@@ -2515,10 +2520,22 @@ static void gpiochip_set_irq_hooks(struct gpio_chip *gc)
 			  "detected irqchip that is shared with multiple gpiochips: please fix the driver.\n");
 		return;
 	}
-	gc->irq.irq_enable = irqchip->irq_enable;
-	gc->irq.irq_disable = irqchip->irq_disable;
-	irqchip->irq_enable = gpiochip_irq_enable;
-	irqchip->irq_disable = gpiochip_irq_disable;
+
+	if (irqchip->irq_disable) {
+		gpiochip->irq.irq_disable = irqchip->irq_disable;
+		irqchip->irq_disable = gpiochip_irq_disable;
+	} else {
+		gpiochip->irq.irq_mask = irqchip->irq_mask;
+		irqchip->irq_mask = gpiochip_irq_mask;
+	}
+
+	if (irqchip->irq_enable) {
+		gpiochip->irq.irq_enable = irqchip->irq_enable;
+		irqchip->irq_enable = gpiochip_irq_enable;
+	} else {
+		gpiochip->irq.irq_unmask = irqchip->irq_unmask;
+		irqchip->irq_unmask = gpiochip_irq_unmask;
+	}
 }
 
 /**

include/linux/gpio/driver.h +13 -0

index 8c41ae4..c8bcaf3 100644
@@ -253,6 +253,19 @@ struct gpio_irq_chip {
 	 * Store old irq_chip irq_disable callback
 	 */
 	void		(*irq_disable)(struct irq_data *data);
+	/**
+	 * @irq_unmask:
+	 *
+	 * Store old irq_chip irq_unmask callback
+	 */
+	void		(*irq_unmask)(struct irq_data *data);
+
+	/**
+	 * @irq_mask:
+	 *
+	 * Store old irq_chip irq_mask callback
+	 */
+	void		(*irq_mask)(struct irq_data *data);
 };
 
 /**

Signature · 2 lines

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Marc Zyngier

2020-05-23 09:43 UTC #2 lore raw

Re: [PATCH 1/4] gpio: gpiolib: Allow GPIO IRQs to lazy disable

Quoted 48 lines · Maulik Shah

With 'commit 461c1a7d4733 ("gpiolib: override irq_enable/disable")' gpiolib overrides irqchip's irq_enable and irq_disable callbacks. If irq_disable callback is implemented then genirq takes unlazy path to disable irq.
Underlying irqchip may not want to implement irq_disable callback to lazy disable irq when client drivers invokes disable_irq(). By overriding irq_disable callback, gpiolib ends up always unlazy disabling IRQ.
Allow gpiolib to lazy disable IRQs by overriding irq_disable callback only if irqchip implemented irq_disable. In cases where irq_disable is not implemented irq_mask is overridden. Similarly override irq_enable callback only if irqchip implemented irq_enable otherwise irq_unmask is overridden.
Fixes 461c1a7d47 (gpiolib: override irq_enable/disable)
Signed-off-by Maulik Shah <[email protected]>
---
 drivers/gpio/gpiolib.c      | 59 
+++++++++++++++++++++++++++++----------------
 include/linux/gpio/driver.h | 13 ++++++++++
 2 files changed, 51 insertions(+), 21 deletions(-)
drivers/gpio/gpiolib.c +18 -0
index eaa0e20..a8fdc74 100644
@@ -2465,33 +2465,38 @@ static void gpiochip_irq_relres(struct irq_data 
 *d)
 	gpiochip_relres_irq(gc, d->hwirq);
 }
 
+static void gpiochip_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	if (chip->irq.irq_mask)
+		chip->irq.irq_mask(d);
+	gpiochip_disable_irq(chip, d->hwirq);
+}
+
+static void gpiochip_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	gpiochip_enable_irq(chip, d->hwirq);
+	if (chip->irq.irq_unmask)
+		chip->irq.irq_unmask(d);
+}
+

drivers/gpio/gpiolib.c static void gpiochip_irq_relres(struct irq_data · line 2491

Quoted 10 lines

drivers/gpio/gpiolib.c +1 -5

 static void gpiochip_irq_enable(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 
-	gpiochip_enable_irq(gc, d->hwirq);
-	if (gc->irq.irq_enable)
-		gc->irq.irq_enable(d);
-	else
-		gc->irq.chip->irq_unmask(d);
+	gpiochip_enable_irq(chip, d->hwirq);

You really never compiled this, did you?

I've stopped looking at this. Please send something that you will have actually tested.

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Maulik Shah

author

2020-05-23 17:07 UTC #3 lore raw

Re: [PATCH 1/4] gpio: gpiolib: Allow GPIO IRQs to lazy disable

Hi,

Quoted 62 lines · Marc Zyngier

Quoted 48 lines · Maulik Shah
With 'commit 461c1a7d4733 ("gpiolib: override irq_enable/disable")' gpiolib overrides irqchip's irq_enable and irq_disable callbacks. If irq_disable callback is implemented then genirq takes unlazy path to disable irq.
Underlying irqchip may not want to implement irq_disable callback to lazy disable irq when client drivers invokes disable_irq(). By overriding irq_disable callback, gpiolib ends up always unlazy disabling IRQ.
Allow gpiolib to lazy disable IRQs by overriding irq_disable callback only if irqchip implemented irq_disable. In cases where irq_disable is not implemented irq_mask is overridden. Similarly override irq_enable callback only if irqchip implemented irq_enable otherwise irq_unmask is overridden.
Fixes 461c1a7d47 (gpiolib: override irq_enable/disable)
Signed-off-by Maulik Shah <[email protected]>
---
drivers/gpio/gpiolib.c | 59
+++++++++++++++++++++++++++++----------------
include/linux/gpio/driver.h | 13 ++++++++++
 2 files changed, 51 insertions(+), 21 deletions(-)
drivers/gpio/gpiolib.c +18 -0
index eaa0e20..a8fdc74 100644
@@ -2465,33 +2465,38 @@ static void gpiochip_irq_relres(struct 
 irq_data *d)
      gpiochip_relres_irq(gc, d->hwirq);
  }
 
+static void gpiochip_irq_mask(struct irq_data *d)
+{
+    struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+    if (chip->irq.irq_mask)
+        chip->irq.irq_mask(d);
+    gpiochip_disable_irq(chip, d->hwirq);
+}
+
+static void gpiochip_irq_unmask(struct irq_data *d)
+{
+    struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+    gpiochip_enable_irq(chip, d->hwirq);
+    if (chip->irq.irq_unmask)
+        chip->irq.irq_unmask(d);
+}
+
drivers/gpio/gpiolib.c static void gpiochip_irq_relres(struct · line 2491
Quoted 10 lines
drivers/gpio/gpiolib.c +1 -5
  static void gpiochip_irq_enable(struct irq_data *d)
  {
      struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 
-    gpiochip_enable_irq(gc, d->hwirq);
-    if (gc->irq.irq_enable)
-        gc->irq.irq_enable(d);
-    else
-        gc->irq.chip->irq_unmask(d);
+    gpiochip_enable_irq(chip, d->hwirq);
You really never compiled this, did you?

Quoted 3 lines

I've stopped looking at this. Please send something that you will have actually tested.
M.

Apologies.

I did tested out on internal devices based on kernel 5.4 as well as linux-next with sc7180.

While posting i somehow taken patch from kernel 5.4 and messed up this patch during merge conflicts.

I fixed this in v2 and posted out changes that cleanly applies on latest linux-next tag (next-20200521).

Thanks,
Maulik

Signature · 1 line

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

kbuild test robot

build bot

2020-05-23 12:30 UTC #4 lore raw

Re: [PATCH 1/4] gpio: gpiolib: Allow GPIO IRQs to lazy disable

In reply to Maulik Shah

Thank you for the patch! Yet something to improve:

Hi Maulik,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on pinctrl/devel v5.7-rc6 next-20200522] [cannot apply to tip/irq/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Maulik-Shah/irqchip-qcom-pdc-Introduce-irq_set_wake-call/20200522-212226
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-a013-20200521 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 3393cc4cebf9969db94dc424b7a2b6195589c33b)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate

Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/gpio/gpiolib.c:2490:22: error: use of undeclared identifier 'chip'

gpiochip_enable_irq(chip, d->hwirq);
^
drivers/gpio/gpiolib.c:2491:2: error: use of undeclared identifier 'chip' chip->irq.irq_enable(d);
^
drivers/gpio/gpiolib.c:2498:2: error: use of undeclared identifier 'chip' chip->irq.irq_disable(d);
^
drivers/gpio/gpiolib.c:2499:23: error: use of undeclared identifier 'chip' gpiochip_disable_irq(chip, d->hwirq);
^

>> drivers/gpio/gpiolib.c:2525:3: error: use of undeclared identifier 'gpiochip'

gpiochip->irq.irq_disable = irqchip->irq_disable;
^
drivers/gpio/gpiolib.c:2528:3: error: use of undeclared identifier 'gpiochip' gpiochip->irq.irq_mask = irqchip->irq_mask;
^
drivers/gpio/gpiolib.c:2533:3: error: use of undeclared identifier 'gpiochip' gpiochip->irq.irq_enable = irqchip->irq_enable;
^
drivers/gpio/gpiolib.c:2536:3: error: use of undeclared identifier 'gpiochip' gpiochip->irq.irq_unmask = irqchip->irq_unmask;
^
8 errors generated.

vim +/chip +2490 drivers/gpio/gpiolib.c

2485
2486 static void gpiochip_irq_enable(struct irq_data *d)
2487 {
2488 struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
2489

> 2490		gpiochip_enable_irq(chip, d->hwirq);
  2491		chip->irq.irq_enable(d);
  2492	}
  2493	
  2494	static void gpiochip_irq_disable(struct irq_data *d)
  2495	{
  2496		struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
  2497	
  2498		chip->irq.irq_disable(d);
  2499		gpiochip_disable_irq(chip, d->hwirq);
  2500	}
  2501	
  2502	static void gpiochip_set_irq_hooks(struct gpio_chip *gc)
  2503	{
  2504		struct irq_chip *irqchip = gc->irq.chip;
  2505	
  2506		if (!irqchip->irq_request_resources &&
  2507		    !irqchip->irq_release_resources) {
  2508			irqchip->irq_request_resources = gpiochip_irq_reqres;
  2509			irqchip->irq_release_resources = gpiochip_irq_relres;
  2510		}
  2511		if (WARN_ON(gc->irq.irq_enable))
  2512			return;
  2513		/* Check if the irqchip already has this hook... */
  2514		if (irqchip->irq_enable == gpiochip_irq_enable) {
  2515			/*
  2516			 * ...and if so, give a gentle warning that this is bad
  2517			 * practice.
  2518			 */
  2519			chip_info(gc,
  2520				  "detected irqchip that is shared with multiple gpiochips: please fix the driver.\n");
  2521			return;
  2522		}
  2523	
  2524		if (irqchip->irq_disable) {
> 2525			gpiochip->irq.irq_disable = irqchip->irq_disable;

2526 irqchip->irq_disable = gpiochip_irq_disable;
2527 } else {
2528 gpiochip->irq.irq_mask = irqchip->irq_mask;
2529 irqchip->irq_mask = gpiochip_irq_mask;
2530 }
2531
2532 if (irqchip->irq_enable) {
2533 gpiochip->irq.irq_enable = irqchip->irq_enable;
2534 irqchip->irq_enable = gpiochip_irq_enable;
2535 } else {
2536 gpiochip->irq.irq_unmask = irqchip->irq_unmask;
2537 irqchip->irq_unmask = gpiochip_irq_unmask;
2538 }
2539 }
2540

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Maulik Shah

author

2020-05-22 13:20 UTC #5 lore raw

PATCH 2/4 pinctrl: qcom: Remove irq_disable callback from msmgpio irqchip

In reply to Maulik Shah

The gpio can be marked for wakeup and drivers can invoke disable_irq() during suspend, in such cases unlazy approach will also disable at HW and such gpios will not wakeup device from suspend to RAM.

Remove irq_disable callback to allow gpio interrupts to lazy disabled. The gpio interrupts will get disabled during irq_mask callback.

Signed-off-by Maulik Shah <[email protected]>

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 13 -------------
 1 file changed, 13 deletions(-)

drivers/pinctrl/qcom/pinctrl-msm.c +0 ~~-13~~

index 83b7d64..2419023 100644
@@ -815,18 +815,6 @@ static void msm_gpio_irq_enable(struct irq_data *d)
 	msm_gpio_irq_clear_unmask(d, true);
 }
 
-static void msm_gpio_irq_disable(struct irq_data *d)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
-
-	if (d->parent_data)
-		irq_chip_disable_parent(d);
-
-	if (!test_bit(d->hwirq, pctrl->skip_wake_irqs))
-		msm_gpio_irq_mask(d);
-}
-
 static void msm_gpio_irq_unmask(struct irq_data *d)
 {
 	msm_gpio_irq_clear_unmask(d, false);

@@ -1146,7 +1134,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 
 	pctrl->irq_chip.name = "msmgpio";
 	pctrl->irq_chip.irq_enable = msm_gpio_irq_enable;
-	pctrl->irq_chip.irq_disable = msm_gpio_irq_disable;
 	pctrl->irq_chip.irq_mask = msm_gpio_irq_mask;
 	pctrl->irq_chip.irq_unmask = msm_gpio_irq_unmask;
 	pctrl->irq_chip.irq_ack = msm_gpio_irq_ack;

Signature · 2 lines

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Maulik Shah

author

2020-05-22 13:20 UTC #6 lore raw

PATCH 3/4 pinctrl: qcom: Add msmgpio irqchip flags

In reply to Maulik Shah

Add irqchip specific flags for msmgpio irqchip to mask non wakeirqs during suspend and mask before setting irq type.

Signed-off-by Maulik Shah <[email protected]>

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/pinctrl/qcom/pinctrl-msm.c +2 -0

index 2419023..b909ffe 100644
@@ -1143,6 +1143,8 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres;
 	pctrl->irq_chip.irq_set_affinity = msm_gpio_irq_set_affinity;
 	pctrl->irq_chip.irq_set_vcpu_affinity = msm_gpio_irq_set_vcpu_affinity;
+	pctrl->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND
+				| IRQCHIP_SET_TYPE_MASKED;
 
 	np = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
 	if (np) {

Signature · 2 lines

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Maulik Shah

author

2020-05-22 13:20 UTC #7 lore raw

PATCH 4/4 irqchip: qcom-pdc: Introduce irq_set_wake call

In reply to Maulik Shah

Remove irq_disable callback to allow lazy disable for pdc interrupts.

Add irq_set_wake callback that unmask interrupt in HW when drivers mark interrupt for wakeup. Interrupt will be cleared in HW during lazy disable if its not marked for wakeup.

Signed-off-by Maulik Shah <[email protected]>

---
 drivers/irqchip/qcom-pdc.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

drivers/irqchip/qcom-pdc.c +17 ~~-16~~

index 6ae9e1f..f7c0662 100644
@@ -36,6 +36,7 @@ struct pdc_pin_region {
 	u32 cnt;
 };
 
+DECLARE_BITMAP(pdc_wake_irqs, PDC_MAX_IRQS);
 static DEFINE_RAW_SPINLOCK(pdc_lock);
 static void __iomem *pdc_base;
 static struct pdc_pin_region *pdc_region;

@@ -87,22 +88,20 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
 	raw_spin_unlock(&pdc_lock);
 }
 
-static void qcom_pdc_gic_disable(struct irq_data *d)
+static int qcom_pdc_gic_set_wake(struct irq_data *d, unsigned int on)
 {
 	if (d->hwirq == GPIO_NO_WAKE_IRQ)
-		return;
-
-	pdc_enable_intr(d, false);
-	irq_chip_disable_parent(d);
-}
+		return 0;
 
-static void qcom_pdc_gic_enable(struct irq_data *d)
-{
-	if (d->hwirq == GPIO_NO_WAKE_IRQ)
-		return;
+	if (on) {
+		pdc_enable_intr(d, true);
+		irq_chip_enable_parent(d);
+		set_bit(d->hwirq, pdc_wake_irqs);
+	} else {
+		clear_bit(d->hwirq, pdc_wake_irqs);
+	}
 
-	pdc_enable_intr(d, true);
-	irq_chip_enable_parent(d);
+	return irq_chip_set_wake_parent(d, on);
 }
 
 static void qcom_pdc_gic_mask(struct irq_data *d)

@@ -110,6 +109,9 @@ static void qcom_pdc_gic_mask(struct irq_data *d)
 	if (d->hwirq == GPIO_NO_WAKE_IRQ)
 		return;
 
+	if (!test_bit(d->hwirq, pdc_wake_irqs))
+		pdc_enable_intr(d, false);
+
 	irq_chip_mask_parent(d);
 }

@@ -118,6 +120,7 @@ static void qcom_pdc_gic_unmask(struct irq_data *d)
 	if (d->hwirq == GPIO_NO_WAKE_IRQ)
 		return;
 
+	pdc_enable_intr(d, true);
 	irq_chip_unmask_parent(d);
 }

@@ -197,15 +200,13 @@ static struct irq_chip qcom_pdc_gic_chip = {
 	.irq_eoi		= irq_chip_eoi_parent,
 	.irq_mask		= qcom_pdc_gic_mask,
 	.irq_unmask		= qcom_pdc_gic_unmask,
-	.irq_disable		= qcom_pdc_gic_disable,
-	.irq_enable		= qcom_pdc_gic_enable,
 	.irq_get_irqchip_state	= qcom_pdc_gic_get_irqchip_state,
 	.irq_set_irqchip_state	= qcom_pdc_gic_set_irqchip_state,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_type		= qcom_pdc_gic_set_type,
+	.irq_set_wake		= qcom_pdc_gic_set_wake,
 	.flags			= IRQCHIP_MASK_ON_SUSPEND |
-				  IRQCHIP_SET_TYPE_MASKED |
-				  IRQCHIP_SKIP_SET_WAKE,
+				  IRQCHIP_SET_TYPE_MASKED,
 	.irq_set_vcpu_affinity	= irq_chip_set_vcpu_affinity_parent,
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 };

Signature · 2 lines

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Linus Walleij

Acked-by on the v1 series

2020-05-25 11:34 UTC #8 lore raw

In reply to Maulik Shah

Quoted 2 lines · Maulik Shah

pinctrl: qcom: Remove irq_disable callback from msmgpio irqchip
pinctrl: qcom: Add msmgpio irqchip flags

For these two:

Acked-by Linus Walleij <[email protected]>

so the irqchip maintainers can merge them.

But you ideally also need Björn's ACKs.

Yours,
Linus Walleij