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

[Marc Zyngier <maz@kernel.org>]

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.