Received-SPF: pass (google.com: domain of linux-kernel+bounces-117155-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <12417336.O9o76ZdvQC@kreacher> <CACRpkdayWiVskunN-YeYzS7dd6ZGHmqiTbCyWQKvOG91S04Dzw@mail.gmail.com>
In-Reply-To: <CACRpkdayWiVskunN-YeYzS7dd6ZGHmqiTbCyWQKvOG91S04Dzw@mail.gmail.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Mon, 25 Mar 2024 14:36:23 +0100
Message-ID: <CAJZ5v0j5qFR-8iLUh+o+k6rTF3xEAXu0iBdcRK6p-64=w2wchA@mail.gmail.com>
Subject: Re: [PATCH v1] irq: Introduce IRQF_COND_ONESHOT and use it in pinctrl-amd
To: Linus Walleij <linus.walleij@linaro.org>, Thomas Gleixner <tglx@linutronix.de>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>, "Rafael J. Wysocki" <rafael@kernel.org>, 
	LKML <linux-kernel@vger.kernel.org>, Hans de Goede <hdegoede@redhat.com>, 
	Linux ACPI <linux-acpi@vger.kernel.org>, 
	Francisco Ayala Le Brun <francisco@videowindow.eu>, Mario Limonciello <mario.limonciello@amd.com>, 
	Adrian Hunter <adrian.hunter@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Mar 25, 2024 at 2:32=E2=80=AFPM Linus Walleij <linus.walleij@linaro=
org> wrote:
>
> On Mon, Mar 25, 2024 at 1:58=E2=80=AFPM Rafael J. Wysocki <rjw@rjwysocki.=
net> wrote:
>
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > There is a problem when a driver requests a shared IRQ line to run
> > a threaded handler on it without IRQF_ONESHOT set if that flag has
> > been set already for the IRQ in question by somebody else.  Namely,
> > the request fails which usually leads to a probe failure even though
> > the driver might have worked just fine with IRQF_ONESHOT, but it does
> > not want to use it by default.  Currently, the only way to handle this
> > is to try to request the IRQ without IRQF_ONESHOT, but with
> > IRQF_PROBE_SHARED set and if this fails, try again with IRQF_ONESHOT
> > set.  However, this is a bit cumbersome and not very clean.
> >
> > When commit 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler
> > for SCI") switched over the ACPI subsystem to using a threaded interrup=
t
> > handler for the SCI, it had to use IRQF_ONESHOT for it because that's
> > required due to the way the SCI handler works (it needs to walk all of
> > the enabled GPEs before IRQ line can be unmasked).  The SCI IRQ line is
> > not shared with other users very often due to the SCI handling overhead=
,
> > but on sone systems it is shared and when the other user of it attempts
> > to install a threaded handler, a flags mismatch related to IRQF_ONESHOT
> > may occur.  As it turned out, that happened to the pinctrl-amd driver
> > and so commit 4451e8e8415e ("pinctrl: amd: Add IRQF_ONESHOT to the
> > interrupt request") attempted to address the issue by adding
> > IRQF_ONESHOT to the interrupt flags in that driver, but this is now
> > causing an IRQF_ONESHOT-related mismatch to occur on another system
> > which cannot boot as a result of it.
> >
> > Clearly, pinctrl-amd can work with IRQF_ONESHOT if need be, but it
> > should not set that flag by default, so it needs a way to indicate that
> > to the IRQ subsystem.
> >
> > To that end, introdcuce a new interrupt flag, IRQF_COND_ONESHOT, which
> > will only have effect when the IRQ line is shared and IRQF_ONESHOT has
> > been set for it already, in which case it will be promoted to the
> > latter.
> >
> > This is sufficient for drivers sharing the IRQ line with the SCI as it
> > is requested by the ACPI subsystem before any drivers are probed, so
> > they will always see IRQF_ONESHOT set for the IRQ in question.
> >
> > Closes: https://lore.kernel.org/lkml/CAN-StX1HqWqi+YW=3Dt+V52-38Mfp5fAz=
7YHx4aH-CQjgyNiKx3g@mail.gmail.com/
> > Fixes: 4451e8e8415e ("pinctrl: amd: Add IRQF_ONESHOT to the interrupt r=
equest")
> > Cc: 6.8+ <stable@vger.kernel.org> # 6.8+
> > Reported-by: Francisco Ayala Le Brun <francisco@videowindow.eu>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> > -#if !defined(CONFIG_GENERIC_IRQ_PROBE)
> > +#if !defined(CONFIG_GENERIC_IRQ_PROBE)
>
> Is that some whitespace fix? Not that it matters to me, so:

Well, incidental, but yes (trailing whitespace).  I actually forgot to
remove this change from the patch before sending it.

> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> I expect that Thomas want to apply this one.

Thank you!