On Mon, Mar 11 2019 at 04:55 -0600, Marc Zyngier wrote:
On 22/02/2019 22:18, Lina Iyer wrote:
To allow GPIOs to wakeup the system from suspend or deep idle, the
wakeup capable GPIOs are setup in hierarchy with interrupts from the
wakeup-parent irqchip.
In older SoC's, the TLMM will handover detection to the parent irqchip
and in newer SoC's, the parent irqchip may also be active as well as the
TLMM and therefore the GPIOs need to be masked at TLMM to avoid
duplicate interrupts. To enable both these configurations to exist,
allow the parent irqchip to dictate the TLMM irqchip's behavior when
masking/unmasking the interrupt.
Co-developed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx>
[...]
@@ -986,6 +1093,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
chip->need_valid_mask = msm_gpio_needs_valid_mask(pctrl);
pctrl->irq_chip.name = "msmgpio";
+ pctrl->irq_chip.irq_eoi = irq_chip_eoi_parent;
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;
@@ -994,6 +1102,22 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
pctrl->irq_chip.irq_request_resources = msm_gpio_irq_reqres;
pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres;
+ chip->irq.chip = &pctrl->irq_chip;
+ chip->irq.domain_ops = &msm_gpio_domain_ops;
+ chip->irq.handler = handle_edge_irq;
+ chip->irq.default_type = IRQ_TYPE_NONE;
I know you're only moving this around, but can you explain why you're
setting IRQ_TYPE_NONE in combination with handle_edge_irq? The two
really should go together. If this doesn't work for some reason, please
document it.
Yes, it was a carry-over from the existing code. I am not sure why that
seems to be a common practice among GPIO drivers.
IRQ_TYPE_EDGE_RISING would be a better option ?
+
+ dn = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
+ if (dn) {
+ chip->irq.parent_domain = irq_find_matching_host(dn,
+ DOMAIN_BUS_WAKEUP);
+ of_node_put(dn);
+ if (!chip->irq.parent_domain)
+ return -EPROBE_DEFER;
+
+ chip->to_irq = msm_gpio_to_irq;
+ }
+
ret = gpiochip_add_data(&pctrl->chip, pctrl);
if (ret) {
dev_err(pctrl->dev, "Failed register gpiochip\n");
@@ -1015,26 +1139,17 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
dev_name(pctrl->dev), 0, 0, chip->ngpio);
if (ret) {
dev_err(pctrl->dev, "Failed to add pin range\n");
- gpiochip_remove(&pctrl->chip);
- return ret;
+ goto fail;
}
}
- ret = gpiochip_irqchip_add(chip,
- &pctrl->irq_chip,
- 0,
- handle_edge_irq,
- IRQ_TYPE_NONE);
- if (ret) {
- dev_err(pctrl->dev, "Failed to add irqchip to gpiochip\n");
- gpiochip_remove(&pctrl->chip);
- return -ENOSYS;
- }
-
gpiochip_set_chained_irqchip(chip, &pctrl->irq_chip, pctrl->irq,
msm_gpio_irq_handler);
return 0;
+fail:
+ gpiochip_remove(&pctrl->chip);
+ return ret;
}
Thanks,
Lina
