Quoting Lina Iyer (2018-12-19 14:11:03) > 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. > > Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxx> I don't think I gave a signed-off-by, so you need to ask to forge my sign off here. Please change it to be: Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx> and I'm not sure how much I wrote vs. you wrote anymore so perhaps also add a Co-developed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx> And finally, please just email my chromium.org email for this series because I apparently messed up the address once and now it's all going to the wrong inbox. Thanks! > Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx> Can you Cc Linus Walleij and Bjorn Andersson on the whole patch series next time? Would be good to have their review on major pinctrl driver changes. > @@ -967,11 +994,86 @@ static bool msm_gpio_needs_valid_mask(struct msm_pinctrl *pctrl) > return device_property_read_u16_array(pctrl->dev, "gpios", NULL, 0) > 0; > } > > +static int msm_gpio_domain_translate(struct irq_domain *d, > + struct irq_fwspec *fwspec, > + unsigned long *hwirq, unsigned int *type) > +{ > + if (is_of_node(fwspec->fwnode)) { > + if (fwspec->param_count < 2) > + return -EINVAL; > + *hwirq = fwspec->param[0]; > + *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK; > + return 0; > + } > + > + return -EINVAL; > +} Maybe this can be a generic function in gpiolib? > + > +static int msm_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq, > + unsigned int nr_irqs, void *arg) > +{ > + int ret; > + irq_hw_number_t hwirq; > + struct gpio_chip *gc = domain->host_data; > + struct msm_pinctrl *pctrl = gpiochip_get_data(gc); > + struct irq_fwspec *fwspec = arg; > + struct qcom_irq_fwspec parent = { }; > + unsigned int type; > + > + ret = msm_gpio_domain_translate(domain, fwspec, &hwirq, &type); > + if (ret) > + return ret; > + > + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, > + &pctrl->irq_chip, gc); > + if (ret < 0) > + return ret; > + > + if (!domain->parent) > + return 0; > + > + parent.fwspec.fwnode = domain->parent->fwnode; > + parent.fwspec.param_count = 2; > + parent.fwspec.param[0] = hwirq; > + parent.fwspec.param[1] = type; > + > + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &parent); > + if (ret) > + return ret; > + > + if (parent.mask) > + set_bit(hwirq, pctrl->wakeup_masked_irqs); > + > + return 0; > +} > + > +/* > + * TODO: Get rid of this and push it into gpiochip_to_irq() Hmm.. yeah we need to do this still. I think we can have a generic two cell function similar to irq_domain_xlate_twocell() that does the fwspec creation and uses some of the things that we pass to gpiochip_irqchip_add(), like the default level type. This existing function is not good to have, so there's work to do to get rid of this. I was also thinking that maybe we can make the alloc function above take a struct gpio_irq_fwspec structure that tells the alloc function what gpiochip the irq is for. That would mean that we need to change the gpio_to_irq() function below to be generic and stuff the chip inside the fwspec wrapper structure: struct gpio_irq_fwspec { struct irq_fwspec fwspec; struct gpio_chip *chip; unsigned int offset; }; but I seem to recall that was not working for some reason. > + */ > +static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned int offset) > +{ > + struct irq_fwspec fwspec; > + > + fwspec.fwnode = of_node_to_fwnode(chip->of_node); > + fwspec.param[0] = offset; > + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH; > + fwspec.param_count = 2; > + > + return irq_create_fwspec_mapping(&fwspec); > +} > + > +static const struct irq_domain_ops msm_gpio_domain_ops = { > + .translate = msm_gpio_domain_translate, > + .alloc = msm_gpio_domain_alloc, > + .free = irq_domain_free_irqs_top, > +}; > +