Hi Linus, On Mon, 4 Oct 2021 at 07:16, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > If Marc says this is the way to go I think it is the way to go! To be completely transparent, moving irq_domain_set_info() is from Marc. Not setting the handler is from me. > > > kfree(parent_arg); > > + > > + if (!ret) { > > Please just exit on error so invert this. Ok. > if (ret) > return ret; > > and just de-indent the below code (easier to follow) > > > + /* If there is a parent domain leave the flow handler alone */ > > + if (d->parent) > > Should we print an error if girq->handler is not NULL and we find > a parent domain, like > if (d->parent && girq->handler) > dev_err(dev, "parent domain and flow handler both specified\n"); I think that would help catch situations where these changes would break stuff. I want to avoid breaking other people's stuff for my hobby project. However, I've noticed we can't get to the "if (d->parent)" if there is no parent as irq_domain_alloc_irqs_parent() will return -ENOSYS if d->parent is null. So the logic isn't right there. I think the idea is right but now I can't figure out what we should actually check to know whether we need to set the handler or not. I'm hoping Marc will chip in when he has some time. :) Cheers, Daniel