Re: [PATCH 19/19] PCI: Set bridge map_irq and swizzle_irq to default functions

From: Bjorn Helgaas <helgaas@kernel.org>
To: Rob Herring <robh@kernel.org>
Cc: "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Tiezhu Yang" <yangtiezhu@loongson.cn>,
	"Jiaxun Yang" <jiaxun.yang@flygoat.com>,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"Ray Jui" <rjui@broadcom.com>,
	"Scott Branden" <sbranden@broadcom.com>,
	"maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE"
	<bcm-kernel-feedback-list@broadcom.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	PCI <linux-pci@vger.kernel.org>,
	linux-tegra <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH 19/19] PCI: Set bridge map_irq and swizzle_irq to default functions
Date: Wed, 12 Jan 2022 09:32:40 -0600	[thread overview]
Message-ID: <20220112153240.GA258585@bhelgaas> (raw)
In-Reply-To: <CAL_JsqKoK6Vewwn4X0L+TKAz-UdK4n7CQ+OKpiEwGe2HqR5L8g@mail.gmail.com>

On Wed, Jan 12, 2022 at 09:09:31AM -0600, Rob Herring wrote:
> On Tue, Jan 11, 2022 at 3:46 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Tue, Jul 21, 2020 at 08:25:14PM -0600, Rob Herring wrote:
> > > The majority of DT based host drivers use the default .map_irq() and
> > > .swizzle_irq() functions, so let's initialize the function pointers to
> > > the default and drop setting them in the host drivers.
> > >
> > > Drivers like iProc which don't support legacy interrupts need to set
> > > .map_irq() back to NULL.
> >
> > Probably a dumb question...
> >
> > This patch removed all the ->swizzle_irq users in drivers/pci/, which
> > is great -- IIUC swizzling is specified by the PCI-to-PCI Bridge Spec,
> > r1.2, sec 9.1, and should not be device-specific.  I assume the few
> > remaining arch/ users (arm and alpha) are either bugs or workarounds
> > for broken devices.
> >
> > My question is why we still have a few users of ->map_irq: loongson,
> > tegra, iproc.  Shouldn't this mapping be described somehow via DT?
> 
> Tegra could perhaps be written another way. The mapping is standard,
> but it's disabling an idle state when PCI interrupts are used. It just
> needs some way to know if legacy interrupts are being used.
>
> iproc looks pretty special with its bcma bus.
> 
> Adding something to DT doesn't really help because we'd still have to
> support the old way.

I guess my underlying question is whether new drivers should ever use
->swizzle_irq() and ->map_irq().  I'm hoping the answer is "no".

If we need them for pre-DT systems or things with incomplete DTs in
the field, I guess we have to live with that.

All ACPI really gives you is the _PRT, and I'm not sure that's
expressive enough to describe platforms that need these things, so
maybe this will eventually solve itself.

Bjorn