Quoting Brian Masney (2019-01-06 18:11:44) > @@ -762,11 +763,14 @@ static int pmic_gpio_of_xlate(struct gpio_chip *chip, > static int pmic_gpio_to_irq(struct gpio_chip *chip, unsigned pin) > { > struct pmic_gpio_state *state = gpiochip_get_data(chip); > - struct pmic_gpio_pad *pad; > + struct irq_fwspec fwspec; > > - pad = state->ctrl->desc->pins[pin].drv_data; > + fwspec.fwnode = state->fwnode; > + fwspec.param_count = 2; > + fwspec.param[0] = pin + 1; There seems to be PMIC_GPIO_PHYSICAL_OFFSET for these purposes. > + fwspec.param[1] = IRQ_TYPE_NONE; > > - return pad->irq; > + return irq_create_fwspec_mapping(&fwspec); > } > > static void pmic_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) > @@ -936,6 +940,86 @@ static int pmic_gpio_populate(struct pmic_gpio_state *state, > return 0; > } > > +static struct irq_chip pmic_gpio_irq_chip = { > + .name = "spmi-gpio", > + .irq_ack = irq_chip_ack_parent, > + .irq_mask = irq_chip_mask_parent, > + .irq_unmask = irq_chip_unmask_parent, > + .irq_set_type = irq_chip_set_type_parent, > + .irq_set_wake = irq_chip_set_wake_parent, > + .flags = IRQCHIP_MASK_ON_SUSPEND, > +}; > + > +static int pmic_gpio_irq_activate(struct irq_domain *domain, > + struct irq_data *data, bool reserve) > +{ > + struct pmic_gpio_state *state = domain->host_data; > + > + return gpiochip_lock_as_irq(&state->chip, data->hwirq); > +} > + > +static void pmic_gpio_irq_deactivate(struct irq_domain *domain, > + struct irq_data *data) > +{ > + struct pmic_gpio_state *state = domain->host_data; > + > + gpiochip_unlock_as_irq(&state->chip, data->hwirq); > +} I meant these sorts of wrappers that could be used by other drivers possibly, if all they need to store in their domain->host_data is the gpiochip structure (they can probably indirect through the gpiochip again to get their driver specific structure). int gpiochip_irq_domain_activate(struct irq_domain *domain, struct irq_data *data, bool reserve) { struct gpiochip *chip = domain->host_data; return gpiochip_lock_as_irq(chip, data->hwirq); } void gpiochip_irq_domain_deactivate(struct irq_domain *domain, struct irq_data *data) { struct gpiochip *chip = domain->host_data; return gpiochip_unlock_as_irq(chip, data->hwirq); } > + > +static int pmic_gpio_domain_translate(struct irq_domain *domain, > + struct irq_fwspec *fwspec, > + unsigned long *hwirq, > + unsigned int *type) > +{ > + struct pmic_gpio_state *state = domain->host_data; > + > + if (fwspec->param_count != 2 || fwspec->param[0] >= state->chip.ngpio) > + return -EINVAL; > + > + *hwirq = fwspec->param[0] - 1; There seems to be PMIC_GPIO_PHYSICAL_OFFSET for these purposes. > + *type = fwspec->param[1]; > + > + return 0; > +} > + > +static int pmic_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq, > + unsigned int nr_irqs, void *data) > +{ > + struct pmic_gpio_state *state = domain->host_data; > + struct irq_fwspec *fwspec = data; > + struct irq_fwspec parent_fwspec; > + irq_hw_number_t hwirq; > + unsigned int type; > + int ret, i; > + > + ret = pmic_gpio_domain_translate(domain, fwspec, &hwirq, &type); > + if (ret) > + return ret; > + > + for (i = 0; i < nr_irqs; i++) > + irq_domain_set_info(domain, virq + i, hwirq + i, > + &pmic_gpio_irq_chip, state, > + handle_level_irq, NULL, NULL); > + > + parent_fwspec.fwnode = domain->parent->fwnode; > + parent_fwspec.param_count = 4; > + parent_fwspec.param[0] = 0; > + parent_fwspec.param[1] = fwspec->param[0] + 0xc0 - 1; Is the -1 there twice, once in pmic_gpio_domain_translate() and then here? > + parent_fwspec.param[2] = 0; > + parent_fwspec.param[3] = fwspec->param[1]; > + > + return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, > + &parent_fwspec); > +} > +