Hi Linus, Sorry to pester you again... On Sat, 17 Oct 2020 at 01:56, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > + gpiochip->to_irq = msc313_gpio_to_irq; > > + gpiochip->base = -1; > > + gpiochip->ngpio = gpio->gpio_data->num; > > + gpiochip->names = gpio->gpio_data->names; > > + > > + for (i = 0; i < gpiochip->ngpio; i++) > > + gpio->irqs[i] = of_irq_get_byname(pdev->dev.of_node, gpio->gpio_data->names[i]); > > Use hierarchical generic GPIO IRQs for these. > > Assign ->fwnode, ->parent_domain, ->child_to_parent_hwirq, > and probably also ->handler on the struct gpio_irq_chip *. > > Skip assigning gpiochip->to_irq, the generic code will > handle that. > > Again see gpio-ixp4xx.c for an example. I sent a v2 with this conversion already and it looks a lot better. Based on Andy Shevchenko's comments[0] I'll be sending a v3 that fixes up all style and other issues he found. Before I do that I have a question that maybe you could help me with: Andy noted a few times that I have this driver as a built in driver and not a module. The gpio-ixp4xx.c driver is also a built in driver. Is there a reason why it's ok there but not this driver? I've actually changed it to allow building as a module already but I don't want to push a v3 if something like the interrupt handling means it should actually be a built in and I'm just missing something. Thanks, Daniel 0 - https://lore.kernel.org/linux-gpio/CAHp75Vf5iUzKp32CqBbv_5MRo8q8CyBPsBcgzKsww6BFtGJwUA@xxxxxxxxxxxxxx/
