Hi Marc, On Thu, 5 Nov 2020 at 21:08, Marc Zyngier <maz@xxxxxxxxxx> wrote: > > On 2020-11-05 09:40, Linus Walleij wrote: > > On Mon, Oct 19, 2020 at 4:10 PM Daniel Palmer <daniel@xxxxxxxx> wrote: > > [...] > > >> +/* The parent interrupt controller needs the GIC interrupt type set > >> to GIC_SPI > >> + * so we need to provide the fwspec. Essentially > >> gpiochip_populate_parent_fwspec_twocell > >> + * that puts GIC_SPI into the first cell. > >> + */ > > nit: comment style. I've fixed these and some other bits for the v3. I've held off on pushing that until the rest of it seemed right. > >> +static void *msc313_gpio_populate_parent_fwspec(struct gpio_chip *gc, > >> + unsigned int > >> parent_hwirq, > >> + unsigned int parent_type) > >> +{ > >> + struct irq_fwspec *fwspec; > >> + > >> + fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL); > >> + if (!fwspec) > >> + return NULL; > >> + > >> + fwspec->fwnode = gc->irq.parent_domain->fwnode; > >> + fwspec->param_count = 3; > >> + fwspec->param[0] = GIC_SPI; > >> + fwspec->param[1] = parent_hwirq; > >> + fwspec->param[2] = parent_type; > >> + > >> + return fwspec; > >> +} > > > > Clever. Looping in Marc Z so he can say if this looks allright to him. > > Yup, this looks correct. However, looking at the bit of the patch that > isn't quoted here, I see that msc313_gpio_irqchip doesn't have a > .irq_set_affinity callback. Is this system UP only? What is in mainline right now is UP only but there are chips with a second cortex A7 that I have working in my tree. So I will add that in for v3 if I can work out what I should actually do there. :) Thanks, Daniel