linux-kernel - RE: [PATCH v2] ACPI: AGDI: Add interrupt signaling mode support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <TY3PR01MB99834EEA2514239975DBA3BDD501A@TY3PR01MB9983.jpnprd01.prod.outlook.com>
Date: Wed, 3 Sep 2025 02:42:46 +0000
From: "Kazuhiro Abe (Fujitsu)" <fj1078ii@...itsu.com>
To: 'Ilkka Koskinen' <ilkka@...amperecomputing.com>
CC: Lorenzo Pieralisi <lpieralisi@...nel.org>, Hanjun Guo
	<guohanjun@...wei.com>, Sudeep Holla <sudeep.holla@....com>, "Rafael J.
 Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] ACPI: AGDI: Add interrupt signaling mode support

Hi Ilkka
Thank you for your comment.

> Hi Kazuhiro,
> 
> I have a couple of minor comments below
> 
> On Fri, 29 Aug 2025, Kazuhiro Abe wrote:
> > AGDI has two types of signaling modes: SDEI and interrupt.
> > Currently, the AGDI driver only supports SDEI.
> > Therefore, add support for interrupt singaling mode The interrupt
> > vector is retrieved from the AGDI table, and call panic function when
> > an interrupt occurs.
> >
> > ---
> > I keep normal IRQ code when NMI cannot be used.
> > If there is any concern, please let me know.
> >
> > v1->v2
> > - Remove acpica update since there is no need to update define value
> >   for this patch.
> >
> > Signed-off-by: Kazuhiro Abe <fj1078ii@...jp.fujitsu.com>
> > ---
> > drivers/acpi/arm64/agdi.c | 98
> ++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 91 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> > index e0df3daa4abf..e887aab6b448 100644
> > --- a/drivers/acpi/arm64/agdi.c
> > +++ b/drivers/acpi/arm64/agdi.c
> 
> ...
> 
> > static int agdi_probe(struct platform_device *pdev) {
> > 	struct agdi_data *adata = dev_get_platdata(&pdev->dev); @@ -55,12
> > +108,17 @@ static int agdi_probe(struct platform_device *pdev)
> > 	if (!adata)
> > 		return -EINVAL;
> >
> > -	return agdi_sdei_probe(pdev, adata);
> > +	if (adata->flags & ACPI_AGDI_SIGNALING_MODE)
> > +		agdi_interrupt_probe(pdev, adata);
> > +	else
> > +		agdi_sdei_probe(pdev, adata);
> > +
> > +	return 0;
> 
> Is there a reason why you always return zero instead of a possible error code from
> either of the probe functions?

There was no particular reason; it was a bug.
I will fix it.

> 
> > }
> >
> 
> ...
> 
> > +static void agdi_remove(struct platform_device *pdev) {
> > +	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
> > +
> > +	if (adata->flags & ACPI_AGDI_SIGNALING_MODE) {
> > +		agdi_interrupt_remove(pdev, adata);
> > +	} else {
> > +		agdi_sdei_remove(pdev, adata);
> > +	}
> 
> You don't need curly braces in this if/else block.

Understood, I will do that.

Best Regards,
Kazuhiro Abe

> 
> Cheers, Ilkka