Re: [PowerPC] [PASEMI] Issue with the identification of ATA drives after the of/irq updates 2024-05-29

Marc Zyngier
2024-07-02T21:42:54Z

On Tue, 02 Jul 2024 21:48:23 +0100,
Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Jul 2, 2024 at 10:54 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Sun, 30 Jun 2024 11:21:55 +0100,
> > Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
> > >
> > > Hello,
> > >
> > > There is an issue with the identification of ATA drives with our
> > > P.A. Semi Nemo boards [1] after the
> > > commit "of/irq: Factor out parsing of interrupt-map parent
> > > phandle+args from of_irq_parse_raw()" [2].
> >
> > [snip]
> >
> > My earlier request for valuable debug information still stands. But
> > while you're at it, can you please give the following hack a go?
> >
> > M.
> >
> > --- a/drivers/of/irq.c
> > +++ b/drivers/of/irq.c
> > @@ -282,8 +282,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
> >
> > oldimap = imap;
> > imap = of_irq_parse_imap_parent(oldimap, imaplen, out_irq);
> > - if (!imap)
> > - goto fail;
> > + if (!imap) {
> > + match = 0;
> > + break;
> > + }
>
> AFAICT reading the DT, I don't think this would fix it. imap should
> only be null if malformed. This case to me looks like interrupt-map
> has the correct cell sizes, but just never matches to do the mapping.
> So maybe imaplen is off and that causes us to end up here, but if
> there's an error I don't see it. A boot with DEBUG enabled in
> drivers/of/irq.c would help.
>
> >
> > match &= of_device_is_available(out_irq->np);
> > if (match)
> >
> > This may not be the final workaround even if it solves your boot
> > problem, but will at least give us a hint at what is going wrong.
> >
> > I have the fuzzy feeling that we may be able to lob this broken system
> > as part of the of_irq_imap_abusers[] array, which would solve things
> > pretty "neatly".
>
> I think this would work and would consolidate the work-arounds. It
> would need either "pasemi,rootbus" or "pa-pxp" added to the list.

I'd be all for this.

M.

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 462375b293e47..c94203ce65bb3 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -81,7 +81,8 @@ EXPORT_SYMBOL_GPL(of_irq_find_parent);
/*
* These interrupt controllers abuse interrupt-map for unspeakable
* reasons and rely on the core code to *ignore* it (the drivers do
- * their own parsing of the property).
+ * their own parsing of the property). The PAsemi entry covers a
+ * non-sensical interrupt-map that is better left ignored.
*
* If you think of adding to the list for something *new*, think
* again. There is a high chance that you will be sent back to the
@@ -95,6 +96,7 @@ static const char * const of_irq_imap_abusers[] = {
"fsl,ls1043a-extirq",
"fsl,ls1088a-extirq",
"renesas,rza1-irqc",
+ "pasemi,rootbus",
NULL,
};

@@ -293,20 +295,8 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
imaplen -= imap - oldimap;
pr_debug(" -> imaplen=%d\n", imaplen);
}
- if (!match) {
- if (intc) {
- /*
- * The PASEMI Nemo is a known offender, so
- * let's only warn for anyone else.
- */
- WARN(!IS_ENABLED(CONFIG_PPC_PASEMI),
- "%pOF interrupt-map failed, using interrupt-controller\n",
- ipar);
- return 0;
- }
-
+ if (!match)
goto fail;
- }

/*
* Successfully parsed an interrupt-map translation; copy new

--
Without deviation from the norm, progress is not possible.