Messages in this thread | | | From | Linus Walleij <> | Date | Mon, 4 Oct 2021 00:16:08 +0200 | Subject | Re: [RFC PATCH] gpiolib: Move setting the flow handler and don't set it at all if there is a parent domain |
| |
Hi Daniel,
thanks for your patch!
On Sat, Oct 2, 2021 at 6:20 PM Daniel Palmer <daniel@0x0f.com> wrote:
> Calling irq_domain_set_info() before irq_domain_alloc_irqs_parent() > can cause a null pointer dereference as the parent domain isn't > ready yet. > > Move irq_domain_set_info() to after irq_domain_alloc_irqs_parent(). > A side effect of this is that irq_domain_set_info() will now overwrite > the flow handler from the parent domain. So if there is a parent > domain do not set the flow handler anymore. > > This allows gpio-msc313.c to level it's irq domain on top of the > new irq controller in later SigmaStar SoCs without crashing. > > Link: https://lore.kernel.org/linux-arm-kernel/20210914100415.1549208-1-daniel@0x0f.com/ > Signed-off-by: Daniel Palmer <daniel@0x0f.com> > Suggested-by: Marc Zyngier <maz@kernel.org>
If Marc says this is the way to go I think it is the way to go!
> kfree(parent_arg); > + > + if (!ret) {
Please just exit on error so invert this.
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) > + irq_domain_set_hwirq_and_chip(d, > + irq, > + hwirq, > + gc->irq.chip, > + gc); > + /* Otherwise set the flow handler supplied by the gpio driver */ > + else > + irq_domain_set_info(d, > + irq, > + hwirq, > + gc->irq.chip, > + gc, > + girq->handler, > + NULL, NULL); > + irq_set_probe(irq); > + }
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");
Yours, Linus Walleij
| |