Re: [PATCH v5 05/20] Input: axp20x-pek: Bail out if AXP has no interrupt line connected

From: Mark Brown <broonie@kernel.org>
To: Andre Przywara <andre.przywara@arm.com>
Cc: "Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Chen-Yu Tsai" <wens@csie.org>,
	"Jernej Skrabec" <jernej.skrabec@siol.net>,
	"Samuel Holland" <samuel@sholland.org>,
	"Icenowy Zheng" <icenowy@aosc.io>,
	"Rob Herring" <robh@kernel.org>,
	"Clément Péron" <peron.clem@gmail.com>,
	"Shuosheng Huang" <huangshuosheng@allwinnertech.com>,
	"Yangtao Li" <tiny.windzz@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com,
	"Lee Jones" <lee.jones@linaro.org>,
	linux-input@vger.kernel.org
Subject: Re: [PATCH v5 05/20] Input: axp20x-pek: Bail out if AXP has no interrupt line connected
Date: Thu, 28 Jan 2021 15:05:55 +0000	[thread overview]
Message-ID: <20210128150555.GD4537@sirena.org.uk> (raw)
In-Reply-To: <20210128123136.417eea7c@slackpad.fritz.box>

[-- Attachment #1: Type: text/plain, Size: 1916 bytes --]

On Thu, Jan 28, 2021 at 12:31:36PM +0000, Andre Przywara wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > It sounds like you're trying to register an IRQ chip with a somehow
> > bogus configuration?

> I patched the AXP MFD driver [1] to skip the regmap-irq creation when no
> interrupts DT property was found, but this NULL pointer now
> understandably confuses the -pek driver, and leads to this crash:

> http://lists.infradead.org/pipermail/linux-arm-kernel/2021-January/634969.html

> Hence I wanted to plug this hole, which seems useful regardless of this
> particular issue.

The driver code here looks pretty confused.  It appears to be looking up
the interrupt to use from a resource (which is what I'd expect for a MFD
child) then for reasons I can't fathom trying to pass that resource into
regmap_irq_get_virq() which is at best going to just return the value
that was passed in but may potentially end up just returning a random
interrupt other than the one that was asked for since we're passing in a
global interrupt number rather than a controller relative one.  I really
can't tell what's supposed to be going on there.  A driver should either
use resources or it should use regmap_irq_get_virq(), using both is a
bug.

The MFD for this device is also just plain buggy in that it is providing
IRQ resources to the children when there is in fact no support for the
interrupts on the device in the system.  This means that the MFD core
sees that it has no interrupt domain, assumes that those interrupt
resources are in fact absolute interrupt numbers and passes them
straight through to the children.  This means that the children will
just be requesting random interrupts in the system which may actually
exist and be requestable which probably isn't going to end well.  When
there is no interrupt controller the parent should not be trying to
supply interrupt resources to the children at all.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]