Re: [PATCH v2 10/11] PCI: mvebu: Implement support for legacy INTx interrupts

From: Marc Zyngier <maz@kernel.org>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: "Pali Rohár" <pali@kernel.org>,
	robh+dt@kernel.org, "Bjorn Helgaas" <bhelgaas@google.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Marek Behún" <kabel@kernel.org>,
	"Russell King" <rmk+kernel@armlinux.org.uk>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 10/11] PCI: mvebu: Implement support for legacy INTx interrupts
Date: Sat, 12 Feb 2022 10:59:37 +0000	[thread overview]
Message-ID: <87a6ewl59i.wl-maz@kernel.org> (raw)
In-Reply-To: <20220211182137.GA2492@lpieralisi>

On Fri, 11 Feb 2022 18:21:37 +0000,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> 
> On Fri, Feb 11, 2022 at 06:52:02PM +0100, Pali Rohár wrote:
> 
> [...]
> 
> > > > @@ -1121,6 +1247,21 @@ static int mvebu_pcie_parse_port(struct mvebu_pcie *pcie,
> > > >  		port->io_attr = -1;
> > > >  	}
> > > >  
> > > > +	/*
> > > > +	 * Old DT bindings do not contain "intx" interrupt
> > > > +	 * so do not fail probing driver when interrupt does not exist.
> > > > +	 */
> > > > +	port->intx_irq = of_irq_get_byname(child, "intx");
> > > > +	if (port->intx_irq == -EPROBE_DEFER) {
> > > > +		ret = port->intx_irq;
> > > > +		goto err;
> > > > +	}
> > > > +	if (port->intx_irq <= 0) {
> > > > +		dev_warn(dev, "%s: legacy INTx interrupts cannot be masked individually, "
> > > > +			      "%pOF does not contain intx interrupt\n",
> > > > +			 port->name, child);
> > > 
> > > Here you end up with a new warning on existing firmware. Is it
> > > legitimate ? I would remove the dev_warn().
> > 
> > I added this warning in v2 because Marc wanted it.
> > 
> > Should I (again) remove it in v3?
> 
> No, I asked a question and gave an opinion, I appreciate Marc's concern
> so leave it (ie not everyone running a new kernel with new warnings on
> existing firmware would be happy - maybe it is a good way of forcing a
> firmware upgrade, you will tell me).

My concern is that short of being able to mask these interrupts, it is
possible for a device to assert an interrupt that no driver handles,
and the kernel spurious interrupt detector won't be able to shut it
up. At this stage, the machine is totally dead (screaming *level*
interrupt).

The dev_warn() could toned down to a dev_warn_once() though.

	M.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel