[PATCH v2] gpio: amdptpci: add support for AMD PT PCI controller

[PATCH v2] gpio: amdptpci: add support for AMD PT PCI controller

[Szuying Chen]

This commit implements the new gpio-amdptpci driver to support the
GPIO functionality on AMD PT PCI platforms.

Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
Co-authored-by: YD Tseng <Yd_Tseng@asmedia.com.tw>

v1->v2:
	- Add .irq_request/release_resources to set irq_pm_held flag.
	- switch to devres in probe/remove.
	- Add synchronize_irq().
---
 drivers/gpio/Kconfig         |   6 +
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-amdptpci.c | 891 +++++++++++++++++++++++++++++++++++
 3 files changed, 898 insertions(+)
 create mode 100644 drivers/gpio/gpio-amdptpci.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a48586bb8edb..1c2f6fc164a1 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1833,6 +1833,12 @@ endmenu
 menu "PCI GPIO expanders"
 	depends on PCI
 
+config GPIO_AMDPTPCI
+	tristate "AMD Promontory PCI GPIO support"
+	depends on X86
+	help
+	  Driver for Promontory IOHub PCI GPIO functionality.
+
 config GPIO_AMD8111
 	tristate "AMD 8111 GPIO driver"
 	depends on X86 || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index dc9e6d643b5b..ca26e98fc8e8 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_GPIO_ALTERA)  		+= gpio-altera.o
 obj-$(CONFIG_GPIO_AMD8111)		+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_AMD_FCH)		+= gpio-amd-fch.o
 obj-$(CONFIG_GPIO_AMDPT)		+= gpio-amdpt.o
+obj-$(CONFIG_GPIO_AMDPTPCI)		+= gpio-amdptpci.o
 obj-$(CONFIG_GPIO_ARIZONA)		+= gpio-arizona.o
 obj-$(CONFIG_GPIO_ASPEED)		+= gpio-aspeed.o
 obj-$(CONFIG_GPIO_ASPEED_SGPIO)		+= gpio-aspeed-sgpio.o
diff --git a/drivers/gpio/gpio-amdptpci.c b/drivers/gpio/gpio-amdptpci.c
new file mode 100644
index 000000000000..ac2a7b46c3cd
--- /dev/null
+++ b/drivers/gpio/gpio-amdptpci.c
@@ -0,0 +1,891 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * gpio-amdptpci.c - AMD Promontory PCI GPIO Controller Driver
+ *
+ * Copyright (C) 2026 ASMedia Technology Inc.
+ *
+ * Author:	Chloe Chen <chloe_chen@asmedia.com.tw>
+ *		YD Tseng <yd_tseng@asmedia.com.tw>
+ *
+ */
+
+#include <linux/bits.h>
+#include <linux/compiler.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_wakeup.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#define PT27_NUM_GPIOS          24
+#define PT27_WAKE_GPIOS         12
+#define PT27_GPIO_MASK          GENMASK(PT27_NUM_GPIOS - 1, 0)
+#define PT27_WAKE_MASK          GENMASK(PT27_WAKE_GPIOS - 1, 0)
+
+#define PT27_REG_DIR            0x0000
+#define PT27_REG_IN             0x0004
+#define PT27_REG_OUT            0x0008
+#define PT27_REG_INT_EN         0x000C
+#define PT27_REG_INT_LVL        0x0010
+#define PT27_REG_INT_MODE0      0x0014
+#define PT27_REG_INT_MODE1      0x0018
+#define PT27_REG_INT_STAT       0x001C
+#define PT27_REG_INT_MASK       0x0020
+#define PT27_REG_INT_CTL        0x0024
+
+#define PT27_INT_CTL_MODE       BIT(0)
+#define PT27_INT_CTL_LVL        BIT(1)
+#define PT27_INT_CTL_PAD_EN     BIT(2)
+
+#define PT27_REG_VENDOR0        0x0028
+#define PT27_REG_VENDOR1        0x002C
+
+#define PT27_REG_WAKE_EN        0x0034
+#define PT27_REG_WAKE_LVL       0x0036
+#define PT27_REG_WAKE_MODE      0x0038
+#define PT27_REG_WAKE_STAT      0x003A
+
+#define PT27_INT_MODE_MASK	0x3U
+#define PT27_MODE_LEVEL         0x0U
+#define PT27_MODE_RISING        0x1U
+#define PT27_MODE_FALLING       0x2U
+#define PT27_MODE_BOTH          0x3U
+
+#define PT27_AUTOSUSPEND_DELAY_MS   2000
+
+struct pt27_saved_regs {
+	u32 dir;
+	u32 out;
+	u32 int_en;
+	u32 int_lvl;
+	u32 int_mode0;
+	u16 int_mode1;
+	u32 int_mask;
+	u8 int_ctl;
+	u32 vendor0;
+	u32 vendor1;
+	u16 wake_en;
+	u16 wake_lvl;
+	u16 wake_mode;
+};
+
+struct pt27_gpio {
+	struct device          *dev;
+	struct gpio_chip        gc;
+	void __iomem           *base;
+	struct pt27_saved_regs  saved;
+	bool                    saved_valid;
+	raw_spinlock_t          lock;
+	struct mutex            irq_bus_lock;
+	bool                    irq_bus_active;
+	u32                     enabled_irq_mask;
+	bool                    irq_pm_held;
+	u16                     wake_mask;
+	bool                    use_msi;
+	int                     irq;
+};
+
+static inline u32  pt27_rd32(struct pt27_gpio *c, u32 off)
+{
+	return ioread32(c->base + off);
+}
+
+static inline void pt27_wr32(struct pt27_gpio *c, u32 off, u32 v)
+{
+	iowrite32(v, c->base + off);
+}
+
+static inline u16  pt27_rd16(struct pt27_gpio *c, u32 off)
+{
+	return ioread16(c->base + off);
+}
+
+static inline void pt27_wr16(struct pt27_gpio *c, u32 off, u16 v)
+{
+	iowrite16(v, c->base + off);
+}
+
+static inline u8   pt27_rd8(struct pt27_gpio *c, u32 off)
+{
+	return ioread8(c->base + off);
+}
+
+static inline void pt27_wr8(struct pt27_gpio *c, u32 off, u8 v)
+{
+	iowrite8(v, c->base + off);
+}
+
+static void pt27_runtime_put_autosuspend(struct pt27_gpio *chip)
+{
+	pm_runtime_mark_last_busy(chip->dev);
+	pm_runtime_put_autosuspend(chip->dev);
+
+}
+
+static void pt27_save_regs(struct pt27_gpio *chip)
+{
+	chip->saved.dir       = pt27_rd32(chip, PT27_REG_DIR);
+	chip->saved.out       = pt27_rd32(chip, PT27_REG_OUT);
+	chip->saved.int_en    = pt27_rd32(chip, PT27_REG_INT_EN);
+	chip->saved.int_lvl   = pt27_rd32(chip, PT27_REG_INT_LVL);
+	chip->saved.int_mode0 = pt27_rd32(chip, PT27_REG_INT_MODE0);
+	chip->saved.int_mode1 = pt27_rd16(chip, PT27_REG_INT_MODE1);
+	chip->saved.int_mask  = pt27_rd32(chip, PT27_REG_INT_MASK);
+	chip->saved.int_ctl   = pt27_rd8(chip, PT27_REG_INT_CTL);
+	chip->saved.vendor0   = pt27_rd32(chip, PT27_REG_VENDOR0);
+	chip->saved.vendor1   = pt27_rd32(chip, PT27_REG_VENDOR1);
+	chip->saved.wake_en   = pt27_rd16(chip, PT27_REG_WAKE_EN);
+	chip->saved.wake_lvl  = pt27_rd16(chip, PT27_REG_WAKE_LVL);
+	chip->saved.wake_mode = pt27_rd16(chip, PT27_REG_WAKE_MODE);
+	chip->saved_valid = true;
+
+}
+
+static void pt27_restore_regs(struct pt27_gpio *chip)
+{
+	pt27_wr32(chip, PT27_REG_INT_EN, 0);
+	pt27_wr32(chip, PT27_REG_INT_MASK, PT27_GPIO_MASK);
+
+	pt27_wr32(chip, PT27_REG_DIR,       chip->saved.dir);
+	pt27_wr32(chip, PT27_REG_OUT,       chip->saved.out);
+	pt27_wr32(chip, PT27_REG_INT_LVL,   chip->saved.int_lvl);
+	pt27_wr32(chip, PT27_REG_INT_MODE0, chip->saved.int_mode0);
+	pt27_wr16(chip, PT27_REG_INT_MODE1, chip->saved.int_mode1);
+	pt27_wr8(chip, PT27_REG_INT_CTL,    chip->saved.int_ctl);
+	pt27_wr32(chip, PT27_REG_VENDOR0,   chip->saved.vendor0);
+	pt27_wr32(chip, PT27_REG_VENDOR1,   chip->saved.vendor1);
+
+	pt27_wr16(chip, PT27_REG_WAKE_LVL,  chip->saved.wake_lvl);
+	pt27_wr16(chip, PT27_REG_WAKE_MODE, chip->saved.wake_mode);
+	pt27_wr16(chip, PT27_REG_WAKE_EN,   chip->saved.wake_en);
+
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+
+	pt27_wr32(chip, PT27_REG_INT_EN,   chip->saved.int_en);
+	pt27_wr32(chip, PT27_REG_INT_MASK, chip->saved.int_mask);
+
+}
+
+static int pt27_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	val  = pt27_rd32(chip, PT27_REG_DIR);
+	val &= ~BIT(offset);   /* 0 = input */
+	pt27_wr32(chip, PT27_REG_DIR, val);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+	return 0;
+}
+
+static int pt27_gpio_direction_output(struct gpio_chip *gc, unsigned int offset,
+				      int value)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	val = pt27_rd32(chip, PT27_REG_OUT);
+	if (value)
+		val |= BIT(offset);
+	else
+		val &= ~BIT(offset);
+	pt27_wr32(chip, PT27_REG_OUT, val);
+
+	val  = pt27_rd32(chip, PT27_REG_DIR);
+	val |= BIT(offset);
+	pt27_wr32(chip, PT27_REG_DIR, val);
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+	return 0;
+}
+
+static int pt27_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	val = pt27_rd32(chip, PT27_REG_DIR);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+
+	return (val & BIT(offset)) ? GPIO_LINE_DIRECTION_OUT
+				   : GPIO_LINE_DIRECTION_IN;
+}
+
+static int pt27_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 dir, val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	dir = pt27_rd32(chip, PT27_REG_DIR);
+	val = pt27_rd32(chip, (dir & BIT(offset)) ? PT27_REG_OUT : PT27_REG_IN);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+
+	return !!(val & BIT(offset));
+}
+
+static int pt27_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0) {
+		dev_err_ratelimited(chip->dev,
+				    "%s: cannot resume to set GPIO%u: %d\n",
+				     __func__, offset, ret);
+		return ret;
+	}
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	val = pt27_rd32(chip, PT27_REG_OUT);
+	if (value)
+		val |= BIT(offset);
+	else
+		val &= ~BIT(offset);
+	pt27_wr32(chip, PT27_REG_OUT, val);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+	return 0;
+}
+
+static u32 pt27_pending_irqs_locked(struct pt27_gpio *chip)
+{
+	u32 stat = pt27_rd32(chip, PT27_REG_INT_STAT);
+	u32 en   = pt27_rd32(chip, PT27_REG_INT_EN);
+	u32 mask = pt27_rd32(chip, PT27_REG_INT_MASK);
+
+	return stat & en & ~mask & PT27_GPIO_MASK;
+}
+
+static bool pt27_irq_regs_accessible(struct pt27_gpio *chip)
+{
+
+	return READ_ONCE(chip->irq_bus_active) ||
+	       READ_ONCE(chip->irq_pm_held);
+}
+
+static void pt27_irq_take_pm_hold(struct pt27_gpio *chip)
+{
+	unsigned long flags;
+	bool take = false;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	if (!chip->irq_pm_held) {
+		chip->irq_pm_held = true;
+		take = true;
+	}
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (take)
+		pm_runtime_get_noresume(chip->dev);
+}
+
+static void pt27_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	u32 val;
+	bool drop_pm = false;
+
+	gpiochip_disable_irq(gc, hwirq);
+
+	if (!pt27_irq_regs_accessible(chip)) {
+		dev_err_ratelimited(chip->dev,
+				    "%s: GPIO%lu remains masked; PT27 is not in D0\n",
+				     __func__, hwirq);
+		return;
+	}
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	val  = pt27_rd32(chip, PT27_REG_INT_MASK);
+	val |= BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_MASK, val);
+
+	val  = pt27_rd32(chip, PT27_REG_INT_EN);
+	val &= ~BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_EN, val);
+
+	chip->enabled_irq_mask &= ~BIT(hwirq);
+	if (!chip->enabled_irq_mask && chip->irq_pm_held) {
+		chip->irq_pm_held = false;
+		drop_pm = true;
+	}
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (drop_pm)
+		pt27_runtime_put_autosuspend(chip);
+
+}
+
+static void pt27_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	u32 val;
+
+	if (!pt27_irq_regs_accessible(chip)) {
+		dev_err_ratelimited(chip->dev,
+				    "%s: cannot unmask GPIO%lu; PT27 is not in D0\n",
+				     __func__, hwirq);
+		return;
+	}
+
+	pt27_irq_take_pm_hold(chip);
+
+	gpiochip_enable_irq(gc, hwirq);
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	pt27_wr32(chip, PT27_REG_INT_STAT, BIT(hwirq));
+
+	val  = pt27_rd32(chip, PT27_REG_INT_EN);
+	val |= BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_EN, val);
+
+	val  = pt27_rd32(chip, PT27_REG_INT_MASK);
+	val &= ~BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_MASK, val);
+
+	chip->enabled_irq_mask |= BIT(hwirq);
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+}
+
+static void pt27_irq_ack(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+
+	pt27_wr32(chip, PT27_REG_INT_STAT, BIT(hwirq));
+}
+
+static int pt27_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	u32 lvl_reg;
+	u8  lvl_bit, mode_val;
+	unsigned int shift;
+
+	if (!pt27_irq_regs_accessible(chip))
+		return -EIO;
+
+	switch (type) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		lvl_bit  = 1; mode_val = PT27_MODE_LEVEL;   break;
+	case IRQ_TYPE_LEVEL_LOW:
+		lvl_bit  = 0; mode_val = PT27_MODE_LEVEL;   break;
+	case IRQ_TYPE_EDGE_RISING:
+		lvl_bit  = 1; mode_val = PT27_MODE_RISING;  break;
+	case IRQ_TYPE_EDGE_FALLING:
+		lvl_bit  = 0; mode_val = PT27_MODE_FALLING; break;
+	case IRQ_TYPE_EDGE_BOTH:
+		lvl_bit  = 0; mode_val = PT27_MODE_BOTH;    break;
+	default:
+		return -EINVAL;
+	}
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	lvl_reg = pt27_rd32(chip, PT27_REG_INT_LVL);
+	if (lvl_bit)
+		lvl_reg |= BIT(hwirq);
+	else
+		lvl_reg &= ~BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_LVL, lvl_reg);
+
+	if (hwirq < 16) {
+		u32 reg = pt27_rd32(chip, PT27_REG_INT_MODE0);
+
+		shift  = hwirq * 2;
+		reg   &= ~(PT27_INT_MODE_MASK << shift);
+		reg   |= ((u32)mode_val << shift);
+		pt27_wr32(chip, PT27_REG_INT_MODE0, reg);
+	} else {
+		u16 reg = pt27_rd16(chip, PT27_REG_INT_MODE1);
+
+		shift  = (hwirq - 16) * 2;
+		reg   &= ~((u16)(PT27_INT_MODE_MASK << shift));
+		reg   |= (u16)(mode_val << shift);
+		pt27_wr16(chip, PT27_REG_INT_MODE1, reg);
+	}
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (type & IRQ_TYPE_LEVEL_MASK)
+		irq_set_handler_locked(d, handle_level_irq);
+	else
+		irq_set_handler_locked(d, handle_edge_irq);
+
+	return 0;
+}
+
+static int pt27_irq_set_wake(struct irq_data *d, unsigned int on)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned int trig = irqd_get_trigger_type(d);
+	unsigned long flags;
+	u16  wake_en, wake_lvl, wake_mode;
+
+	if (hwirq >= PT27_WAKE_GPIOS) {
+		dev_warn(chip->dev,
+			 "%s: GPIO%lu does not support wake (max GPIO%u)\n",
+			 __func__, hwirq, PT27_WAKE_GPIOS - 1);
+		return -EINVAL;
+	}
+	if (!pt27_irq_regs_accessible(chip))
+		return -EIO;
+	if (on && trig == IRQ_TYPE_NONE)
+		return -EINVAL;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	wake_en   = pt27_rd16(chip, PT27_REG_WAKE_EN);
+	wake_lvl  = pt27_rd16(chip, PT27_REG_WAKE_LVL);
+	wake_mode = pt27_rd16(chip, PT27_REG_WAKE_MODE);
+
+	if (on) {
+		wake_en |= BIT(hwirq);
+		chip->wake_mask |= (u16)BIT(hwirq);
+
+		if (trig & IRQ_TYPE_LEVEL_MASK) {
+			wake_mode |= BIT(hwirq);
+			if (trig == IRQ_TYPE_LEVEL_HIGH)
+				wake_lvl |= BIT(hwirq);
+			else
+				wake_lvl &= ~BIT(hwirq);
+		} else {
+			wake_mode &= ~BIT(hwirq);
+			if (trig == IRQ_TYPE_EDGE_RISING ||
+			    trig == IRQ_TYPE_EDGE_BOTH)
+				wake_lvl |= BIT(hwirq);
+			else
+				wake_lvl &= ~BIT(hwirq);
+		}
+	} else {
+		wake_en  &= ~BIT(hwirq);
+		chip->wake_mask &= ~(u16)BIT(hwirq);
+	}
+
+	pt27_wr16(chip, PT27_REG_WAKE_EN,   wake_en);
+	pt27_wr16(chip, PT27_REG_WAKE_LVL,  wake_lvl);
+	pt27_wr16(chip, PT27_REG_WAKE_MODE, wake_mode);
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static void pt27_irq_bus_lock(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	int ret;
+
+	mutex_lock(&chip->irq_bus_lock);
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0) {
+		WRITE_ONCE(chip->irq_bus_active, false);
+		dev_err_ratelimited(chip->dev,
+				    "%s: cannot resume for IRQ configuration: %d\n",
+				     __func__, ret);
+		return;
+	}
+
+	WRITE_ONCE(chip->irq_bus_active, true);
+}
+
+static void pt27_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+
+	if (READ_ONCE(chip->irq_bus_active)) {
+		WRITE_ONCE(chip->irq_bus_active, false);
+		pt27_runtime_put_autosuspend(chip);
+	}
+
+	mutex_unlock(&chip->irq_bus_lock);
+}
+
+static const struct irq_chip pt27_gpio_irq_chip = {
+	.name                = "pt27_gpio",
+	.irq_ack             = pt27_irq_ack,
+	.irq_mask            = pt27_irq_mask,
+	.irq_unmask          = pt27_irq_unmask,
+	.irq_set_type        = pt27_irq_set_type,
+	.irq_set_wake        = pt27_irq_set_wake,
+	.irq_bus_lock        = pt27_irq_bus_lock,
+	.irq_bus_sync_unlock = pt27_irq_bus_sync_unlock,
+	.flags               = IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static irqreturn_t pt27_gpio_isr(int irq, void *data)
+{
+	struct pt27_gpio *chip = data;
+	unsigned long pending;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	pending = pt27_pending_irqs_locked(chip);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (!pending)
+		return IRQ_NONE;
+
+	while (pending) {
+		unsigned int hwirq = __ffs(pending);
+
+		pending &= ~BIT(hwirq);
+		generic_handle_domain_irq(chip->gc.irq.domain, hwirq);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void pt27_hw_init(struct pt27_gpio *chip)
+{
+	u8 int_ctl;
+
+	int_ctl  = pt27_rd8(chip, PT27_REG_INT_CTL);
+	int_ctl |=  PT27_INT_CTL_MODE;
+	int_ctl &= ~PT27_INT_CTL_PAD_EN;
+	pt27_wr8(chip, PT27_REG_INT_CTL, int_ctl);
+
+	pt27_wr32(chip, PT27_REG_INT_EN,    0);
+	pt27_wr32(chip, PT27_REG_INT_MASK,  PT27_GPIO_MASK);
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_EN,   0);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+}
+
+static int pt27_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct device    *dev = &pdev->dev;
+	struct pt27_gpio *chip;
+	struct gpio_irq_chip *girq;
+	unsigned long irq_flags;
+	int ret;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = dev;
+	raw_spin_lock_init(&chip->lock);
+	mutex_init(&chip->irq_bus_lock);
+	pci_set_drvdata(pdev, chip);
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable PCI device\n");
+
+	chip->base = pcim_iomap_region(pdev, 0, "pt27_gpio");
+	if (IS_ERR(chip->base))
+		return dev_err_probe(dev, PTR_ERR(chip->base),
+				     "failed to claim and map BAR0\n");
+
+	pci_set_master(pdev);
+
+	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX | PCI_IRQ_MSI);
+	if (ret < 0) {
+		ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX);
+		if (ret < 0) {
+			ret = dev_err_probe(dev, ret,
+					    "failed to allocate IRQ vector\n");
+			goto err_clear_master;
+		}
+		chip->use_msi = false;
+		irq_flags     = IRQF_SHARED;
+		dev_dbg(dev, "Using shared INTx interrupt\n");
+	} else {
+		chip->use_msi = true;
+		irq_flags     = 0;
+		dev_dbg(dev, "Using %s interrupt vector\n",
+			pdev->msix_enabled ? "MSI-X" : "MSI");
+	}
+	chip->irq = pci_irq_vector(pdev, 0);
+
+	pt27_hw_init(chip);
+
+	pm_runtime_set_autosuspend_delay(dev, PT27_AUTOSUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
+
+	chip->gc.label            = dev_name(dev);
+	chip->gc.parent           = dev;
+	chip->gc.owner            = THIS_MODULE;
+	chip->gc.base             = -1;             /* Dynamic base assignment */
+	chip->gc.ngpio            = PT27_NUM_GPIOS;
+	chip->gc.direction_input  = pt27_gpio_direction_input;
+	chip->gc.direction_output = pt27_gpio_direction_output;
+	chip->gc.get_direction    = pt27_gpio_get_direction;
+	chip->gc.get              = pt27_gpio_get;
+	chip->gc.set              = pt27_gpio_set;
+	chip->gc.can_sleep        = true;
+
+	girq = &chip->gc.irq;
+	gpio_irq_chip_set_chip(girq, &pt27_gpio_irq_chip);
+	girq->parent_handler = NULL;
+	girq->num_parents    = 0;
+	girq->parents        = NULL;
+	girq->default_type   = IRQ_TYPE_NONE;
+	girq->handler        = handle_bad_irq;
+
+	ret = device_init_wakeup(dev, true);
+	if (ret) {
+		ret = dev_err_probe(dev, ret, "failed to enable wake capability\n");
+		goto err_free_vectors;
+	}
+
+	dev_pm_set_driver_flags(dev, DPM_FLAG_SMART_SUSPEND);
+
+	ret = devm_request_irq(dev, chip->irq, pt27_gpio_isr,
+			       irq_flags | IRQF_NO_THREAD, dev_name(dev), chip);
+	if (ret) {
+		ret = dev_err_probe(dev, ret,
+				    "failed to request IRQ %d\n", chip->irq);
+		goto err_wakeup;
+	}
+
+	ret = devm_gpiochip_add_data(dev, &chip->gc, chip);
+	if (ret) {
+		ret = dev_err_probe(dev, ret, "failed to register GPIO chip\n");
+		goto err_free_irq;
+	}
+
+	dev_info(dev,
+		 "PT27 GPIO controller: %u GPIOs (%u wake-capable), IRQ=%d (%s)\n",
+		 PT27_NUM_GPIOS, PT27_WAKE_GPIOS, chip->irq,
+		 chip->use_msi ? (pdev->msix_enabled ? "MSI-X" : "MSI") : "INTx");
+
+	pm_runtime_put_autosuspend(dev);
+	pm_runtime_allow(dev);
+
+	return 0;
+
+err_free_irq:
+	devm_free_irq(dev, chip->irq, chip);
+err_wakeup:
+	device_init_wakeup(dev, false);
+err_free_vectors:
+	pci_free_irq_vectors(pdev);
+err_clear_master:
+	pci_clear_master(pdev);
+	return ret;
+}
+
+static void pt27_gpio_remove(struct pci_dev *pdev)
+{
+	struct device     *dev  = &pdev->dev;
+	struct pt27_gpio  *chip = pci_get_drvdata(pdev);
+	unsigned long flags;
+	bool drop_pm;
+
+	/* Restore the canonical "unbound PCI device" Runtime-PM state. */
+	pm_runtime_forbid(dev);
+	pm_runtime_get_noresume(dev);
+	pm_runtime_dont_use_autosuspend(dev);
+
+	mutex_lock(&chip->irq_bus_lock);
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	pt27_wr32(chip, PT27_REG_INT_EN,    0);
+	pt27_wr32(chip, PT27_REG_INT_MASK,  PT27_GPIO_MASK);
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_EN,   0);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+
+	chip->enabled_irq_mask = 0;
+	chip->wake_mask        = 0;
+	drop_pm = chip->irq_pm_held;
+	chip->irq_pm_held = false;
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (drop_pm)
+		pm_runtime_put_noidle(dev);
+
+	device_init_wakeup(dev, false);
+
+	devm_free_irq(dev, chip->irq, chip);
+	pci_free_irq_vectors(pdev);
+	pci_clear_master(pdev);
+
+	mutex_unlock(&chip->irq_bus_lock);
+}
+
+static int __maybe_unused pt27_gpio_suspend(struct device *dev)
+{
+	struct pt27_gpio *chip = dev_get_drvdata(dev);
+	unsigned long flags;
+	u32 int_mask;
+
+	if (pm_runtime_status_suspended(dev)) {
+		dev_dbg(dev, "%s: already runtime-suspended, skipping save\n",
+			__func__);
+		return 0;
+	}
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	pt27_save_regs(chip);
+	pt27_wr32(chip, PT27_REG_INT_EN,    0);
+	int_mask = PT27_REG_INT_MASK & ~chip->wake_mask;
+	pt27_wr32(chip, PT27_REG_INT_MASK,  int_mask);
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static int __maybe_unused pt27_gpio_resume(struct device *dev)
+{
+	struct pt27_gpio *chip = dev_get_drvdata(dev);
+	unsigned long flags;
+	u16 wake_status;
+
+	if (!chip->saved_valid)
+		return 0;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	wake_status = pt27_rd16(chip, PT27_REG_WAKE_STAT) & PT27_WAKE_MASK;
+	pt27_restore_regs(chip);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (wake_status)
+		pm_wakeup_event(dev, 0);
+
+	return 0;
+}
+
+static int __maybe_unused pt27_gpio_runtime_suspend(struct device *dev)
+{
+	struct pt27_gpio *chip = dev_get_drvdata(dev);
+	unsigned long flags;
+	int ret = 0;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	if (chip->irq_pm_held || chip->enabled_irq_mask) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+	if (chip->wake_mask && !device_may_wakeup(dev)) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+	if (pt27_pending_irqs_locked(chip)) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+	pt27_save_regs(chip);
+
+	pt27_wr32(chip, PT27_REG_INT_EN,    0);
+	pt27_wr32(chip, PT27_REG_INT_MASK,  PT27_GPIO_MASK);
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+
+out_unlock:
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+	return ret;
+}
+
+static int __maybe_unused pt27_gpio_runtime_resume(struct device *dev)
+{
+
+	return pt27_gpio_resume(dev);
+}
+
+static const struct dev_pm_ops pt27_gpio_pm_ops __maybe_unused = {
+	SYSTEM_SLEEP_PM_OPS(pt27_gpio_suspend, pt27_gpio_resume)
+	RUNTIME_PM_OPS(pt27_gpio_runtime_suspend, pt27_gpio_runtime_resume, NULL)
+};
+
+#define PCI_DEVICE_ID_PT27	0x444b
+
+static const struct pci_device_id pt27_gpio_pci_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_PT27) },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, pt27_gpio_pci_ids);
+
+static struct pci_driver pt27_gpio_pci_driver = {
+	.name     = "ptpci-gpio",
+	.id_table = pt27_gpio_pci_ids,
+	.probe    = pt27_gpio_probe,
+	.remove   = pt27_gpio_remove,
+	.driver   = {
+		.pm = pm_ptr(&pt27_gpio_pm_ops),
+	},
+};
+
+module_pci_driver(pt27_gpio_pci_driver);
+
+MODULE_DESCRIPTION("AMD PT PCI GPIO Controller");
+MODULE_LICENSE("GPL");
-- 
2.48.1

==================================================================================================================
This email and any attachments to it contain confidential information and are intended solely for the use of the individual to whom it 
is addressed.If you are not the intended recipient or receive it accidentally, please immediately notify the sender by e-mail and delete 
the message and any attachments from your computer system, and destroy all hard copies. If any, please be advised that any unauthorized 
disclosure, copying, distribution or any action taken or omitted in reliance on this, is illegal and prohibited. Furthermore, any views 
or opinions expressed are solely those of the author and do not represent those of ASMedia Technology Inc. Thank you for your cooperation.
==================================================================================================================

Re: [PATCH v2] gpio: amdptpci: add support for AMD PT PCI controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] pm: irq handler accesses hardware registers without runtime PM check
- [High] pm: missing synchronize_irq() in runtime suspend callback
- [High] gpio: system suspend erroneously unmasks all interrupts due to incorrect mask calculation
- [High] gpio: PREEMPT_RT lock nesting violation in pt27_irq_mask()
- [Medium] gpio: level-triggered interrupts permanently masked due to PM state drop
- [Medium] gpio: TOCTOU race in PM reference tracking between mask and unmask
- [Medium] gpio: teardown ordering violation due to mixed devm and manual cleanup
- [Medium] pci: runtime PM state leak in probe error path
--

commit 0c4bfe0b69469a3b6087533b773e271439cdda28
Author: Szuying Chen <Chloe_Chen@asmedia.com.tw>

gpio: amdptpci: add support for AMD PT PCI controller

This commit implements the new gpio-amdptpci driver to support the
GPIO functionality on AMD PT PCI platforms.

> diff --git a/drivers/gpio/gpio-amdptpci.c b/drivers/gpio/gpio-amdptpci.c
> new file mode 100644
> index 0000000000000..ac2a7b46c3cde
> --- /dev/null
> +++ b/drivers/gpio/gpio-amdptpci.c
> @@ -0,0 +1,891 @@

[ ... ]

> +static void pt27_irq_mask(struct irq_data *d)
> +{
> +	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
> +	struct pt27_gpio *chip = gpiochip_get_data(gc);
> +	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
> +	unsigned long flags;
> +	u32 val;
> +	bool drop_pm = false;
> +
> +	gpiochip_disable_irq(gc, hwirq);
> +
> +	if (!pt27_irq_regs_accessible(chip)) {
> +		dev_err_ratelimited(chip->dev,
> +				    "%s: GPIO%lu remains masked; PT27 is not in D0\n",
> +				     __func__, hwirq);
> +		return;
> +	}
> +
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +
> +	val  = pt27_rd32(chip, PT27_REG_INT_MASK);
> +	val |= BIT(hwirq);
> +	pt27_wr32(chip, PT27_REG_INT_MASK, val);
> +
> +	val  = pt27_rd32(chip, PT27_REG_INT_EN);
> +	val &= ~BIT(hwirq);
> +	pt27_wr32(chip, PT27_REG_INT_EN, val);
> +
> +	chip->enabled_irq_mask &= ~BIT(hwirq);
> +	if (!chip->enabled_irq_mask && chip->irq_pm_held) {
> +		chip->irq_pm_held = false;
> +		drop_pm = true;
> +	}
> +
> +	raw_spin_unlock_irqrestore(&chip->lock, flags);

[Severity: Medium]
Will this logic permanently mask level-triggered interrupts?

handle_level_irq() temporarily masks the interrupt by calling .irq_mask() and
unmasks it afterward with .irq_unmask(). If this is the only active interrupt,
this clears enabled_irq_mask to 0, setting chip->irq_pm_held to
false and dropping the PM reference. 

When handle_level_irq() subsequently calls pt27_irq_unmask(), it will fail the
pt27_irq_regs_accessible(chip) check because irq_pm_held is false and
irq_bus_active is false, leaving the interrupt permanently masked.

> +
> +	if (drop_pm)
> +		pt27_runtime_put_autosuspend(chip);

[Severity: High]
Does this code call a sleepable PM runtime function while running in atomic
context under the IRQ core's raw spinlock?

pt27_irq_mask() is called by the IRQ core with the descriptor lock held, which
is a raw_spinlock_t. If drop_pm is true, the driver calls
pt27_runtime_put_autosuspend(chip), which internally acquires dev->power.lock
(a spinlock_t).

On PREEMPT_RT, spinlock_t is a sleepable rt_mutex. Acquiring a sleepable lock
inside a raw_spinlock_t critical section is a strict lock nesting violation
and triggers a sleeping in atomic context BUG.

> +
> +}
> +
> +static void pt27_irq_unmask(struct irq_data *d)
> +{
> +	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
> +	struct pt27_gpio *chip = gpiochip_get_data(gc);
> +	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
> +	unsigned long flags;
> +	u32 val;
> +
> +	if (!pt27_irq_regs_accessible(chip)) {
> +		dev_err_ratelimited(chip->dev,
> +				    "%s: cannot unmask GPIO%lu; PT27 is not in D0\n",
> +				     __func__, hwirq);
> +		return;
> +	}
> +
> +	pt27_irq_take_pm_hold(chip);
> +
> +	gpiochip_enable_irq(gc, hwirq);
> +
> +	raw_spin_lock_irqsave(&chip->lock, flags);

[Severity: Medium]
Can a TOCTOU race occur here in PM reference tracking between mask and unmask?

If CPU 0 executes pt27_irq_unmask() for hwirq 0, it calls
pt27_irq_take_pm_hold(), which sees irq_pm_held == true, skips incrementing
the PM usage count, and drops the lock.

Before CPU 0 re-acquires the lock, CPU 1 could execute pt27_irq_mask() for
the only active hwirq 1, clear enabled_irq_mask, set irq_pm_held to false,
and call pm_runtime_put_autosuspend().

CPU 0 then re-acquires the lock and sets enabled_irq_mask |= BIT(0), but
irq_pm_held remains false. The PM usage count is now unbalanced and the device
may enter runtime suspend while hwirq 0 is actively relying on it.

[ ... ]

> +static irqreturn_t pt27_gpio_isr(int irq, void *data)
> +{
> +	struct pt27_gpio *chip = data;
> +	unsigned long pending;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +	pending = pt27_pending_irqs_locked(chip);
> +	raw_spin_unlock_irqrestore(&chip->lock, flags);

[Severity: High]
Does this ISR access hardware registers without checking if the device is
active?

The device supports shared INTx interrupts via IRQF_SHARED, which means the
interrupt can fire at any time, including when the AMD PT GPIO controller is
in a suspended PM state (D3hot/D3cold).

pt27_gpio_isr() unconditionally calls pt27_pending_irqs_locked(), which
performs MMIO reads. Reading MMIO from a suspended PCI device typically
returns 0xffffffff and can cause PCIe target aborts, system hangs, or
interrupt storms.

Could we use pm_runtime_get_if_active() before accessing hardware registers?

[ ... ]

> +static int pt27_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct device    *dev = &pdev->dev;
> +	struct pt27_gpio *chip;

[ ... ]

> +err_free_irq:
> +	devm_free_irq(dev, chip->irq, chip);
> +err_wakeup:
> +	device_init_wakeup(dev, false);
> +err_free_vectors:
> +	pci_free_irq_vectors(pdev);
> +err_clear_master:
> +	pci_clear_master(pdev);
> +	return ret;
> +}

[Severity: Medium]
Are we leaking the runtime PM state in the probe error paths?

Since pm_runtime_use_autosuspend(dev) is called earlier in probe, if probe
fails at devm_request_irq() or devm_gpiochip_add_data(), it will jump to these
error labels without calling pm_runtime_dont_use_autosuspend(dev).

This leaves the PCI device with use_autosuspend = true. If another driver
binds to this device later, it will unexpectedly inherit an active autosuspend
configuration.

> +static void pt27_gpio_remove(struct pci_dev *pdev)
> +{
> +	struct device     *dev  = &pdev->dev;
> +	struct pt27_gpio  *chip = pci_get_drvdata(pdev);
> +	unsigned long flags;
> +	bool drop_pm;

[ ... ]

> +	devm_free_irq(dev, chip->irq, chip);
> +	pci_free_irq_vectors(pdev);
> +	pci_clear_master(pdev);
> +
> +	mutex_unlock(&chip->irq_bus_lock);
> +}

[Severity: Medium]
Does manual cleanup of the IRQ vectors invert the teardown order?

Because the gpiochip was registered using devm_gpiochip_add_data(), it won't
be unregistered until after pt27_gpio_remove() returns. However,
pci_free_irq_vectors() is called manually here, destroying the backing
hardware resources while the logical device remains active and exposed.

Userspace could successfully request GPIO interrupts during this window that
will silently never fire, breaking the hardware-to-software dependency chain.
Could we use devm_add_action_or_reset() for freeing the IRQ vectors?

> +static int __maybe_unused pt27_gpio_suspend(struct device *dev)
> +{
> +	struct pt27_gpio *chip = dev_get_drvdata(dev);
> +	unsigned long flags;
> +	u32 int_mask;
> +
> +	if (pm_runtime_status_suspended(dev)) {
> +		dev_dbg(dev, "%s: already runtime-suspended, skipping save\n",
> +			__func__);
> +		return 0;
> +	}
> +
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +	pt27_save_regs(chip);
> +	pt27_wr32(chip, PT27_REG_INT_EN,    0);
> +	int_mask = PT27_REG_INT_MASK & ~chip->wake_mask;
> +	pt27_wr32(chip, PT27_REG_INT_MASK,  int_mask);

[Severity: High]
Does this bitwise operation use a register offset address instead of a valid
mask?

PT27_REG_INT_MASK is a register offset macro (0x0020), not the GPIO bitmask
(PT27_GPIO_MASK). This bitwise AND limits int_mask to only bit 5, forcing all
other 23 bits to 0.

Since writing 0 to the mask register enables/unmasks the interrupt, this writes
a corrupted state to the device that inadvertently unmasks almost all interrupts
during system sleep, potentially causing an interrupt storm and preventing
sleep.

> +	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
> +	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
> +	raw_spin_unlock_irqrestore(&chip->lock, flags);
> +
> +	return 0;
> +}

[ ... ]

> +static int __maybe_unused pt27_gpio_runtime_suspend(struct device *dev)
> +{
> +	struct pt27_gpio *chip = dev_get_drvdata(dev);
> +	unsigned long flags;
> +	int ret = 0;
> +
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +
> +	if (chip->irq_pm_held || chip->enabled_irq_mask) {
> +		ret = -EBUSY;
> +		goto out_unlock;
> +	}
> +
> +	if (chip->wake_mask && !device_may_wakeup(dev)) {
> +		ret = -EBUSY;
> +		goto out_unlock;
> +	}
> +
> +	if (pt27_pending_irqs_locked(chip)) {
> +		ret = -EBUSY;
> +		goto out_unlock;
> +	}
> +
> +	pt27_save_regs(chip);
> +
> +	pt27_wr32(chip, PT27_REG_INT_EN,    0);
> +	pt27_wr32(chip, PT27_REG_INT_MASK,  PT27_GPIO_MASK);
> +	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
> +	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
> +
> +out_unlock:
> +	raw_spin_unlock_irqrestore(&chip->lock, flags);
> +	return ret;
> +}

[Severity: High]
Should this function call synchronize_irq(chip->irq)?

The device can use shared interrupts (IRQF_SHARED). When drivers use shared
IRQs, the PM subsystem guidelines strictly require calling synchronize_irq()
in the runtime suspend callback.

This initiates power-down and masks interrupts without previously synchronizing
the shared IRQ line, allowing the hardware to be powered down while the shared
IRQ handler is still executing mid-flight on another CPU, leading to invalid
MMIO accesses and potential crashes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013074327.4961-1-Chloe_Chen@asmedia.com.tw?part=1

[PATCH v2] gpio: amdptpci: add support for AMD PT PCI controller

[Szuying Chen]

This commit implements the new gpio-amdptpci driver to support the
GPIO functionality on AMD PT PCI platforms.

Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
Co-authored-by: YD Tseng <Yd_Tseng@asmedia.com.tw>

v1->v2:
	- Add .irq_request/release_resources to set irq_pm_held flag.
	- switch to devres in probe/remove.
	- Add synchronize_irq().
---
 drivers/gpio/Kconfig         |   6 +
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-amdptpci.c | 884 +++++++++++++++++++++++++++++++++++
 3 files changed, 891 insertions(+)
 create mode 100644 drivers/gpio/gpio-amdptpci.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a48586bb8edb..1c2f6fc164a1 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1833,6 +1833,12 @@ endmenu
 menu "PCI GPIO expanders"
 	depends on PCI

+config GPIO_AMDPTPCI
+	tristate "AMD Promontory PCI GPIO support"
+	depends on X86
+	help
+	  Driver for Promontory IOHub PCI GPIO functionality.
+
 config GPIO_AMD8111
 	tristate "AMD 8111 GPIO driver"
 	depends on X86 || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index dc9e6d643b5b..ca26e98fc8e8 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_GPIO_ALTERA)  		+= gpio-altera.o
 obj-$(CONFIG_GPIO_AMD8111)		+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_AMD_FCH)		+= gpio-amd-fch.o
 obj-$(CONFIG_GPIO_AMDPT)		+= gpio-amdpt.o
+obj-$(CONFIG_GPIO_AMDPTPCI)		+= gpio-amdptpci.o
 obj-$(CONFIG_GPIO_ARIZONA)		+= gpio-arizona.o
 obj-$(CONFIG_GPIO_ASPEED)		+= gpio-aspeed.o
 obj-$(CONFIG_GPIO_ASPEED_SGPIO)		+= gpio-aspeed-sgpio.o
diff --git a/drivers/gpio/gpio-amdptpci.c b/drivers/gpio/gpio-amdptpci.c
new file mode 100644
index 000000000000..c63e9b6dcf7a
--- /dev/null
+++ b/drivers/gpio/gpio-amdptpci.c
@@ -0,0 +1,884 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * gpio-amdptpci.c - AMD Promontory PCI GPIO Controller Driver
+ *
+ * Copyright (C) 2026 ASMedia Technology Inc.
+ * Author:	Chloe Chen <chloe_chen@asmedia.com.tw>
+ *		YD Tseng <yd_tseng@asmedia.com.tw>
+ */
+
+#include <linux/bits.h>
+#include <linux/compiler.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_wakeup.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#define PT27_NUM_GPIOS          24
+#define PT27_WAKE_GPIOS         12
+#define PT27_GPIO_MASK          GENMASK(PT27_NUM_GPIOS - 1, 0)
+#define PT27_WAKE_MASK          GENMASK(PT27_WAKE_GPIOS - 1, 0)
+
+#define PT27_REG_DIR            0x0000
+#define PT27_REG_IN             0x0004
+#define PT27_REG_OUT            0x0008
+#define PT27_REG_INT_EN         0x000C
+#define PT27_REG_INT_LVL        0x0010
+#define PT27_REG_INT_MODE0      0x0014
+#define PT27_REG_INT_MODE1      0x0018
+#define PT27_REG_INT_STAT       0x001C
+#define PT27_REG_INT_MASK       0x0020
+#define PT27_REG_INT_CTL        0x0024
+
+#define PT27_INT_CTL_MODE       BIT(0)
+#define PT27_INT_CTL_LVL        BIT(1)
+#define PT27_INT_CTL_PAD_EN     BIT(2)
+
+#define PT27_REG_VENDOR0        0x0028
+#define PT27_REG_VENDOR1        0x002C
+
+#define PT27_REG_WAKE_EN        0x0034
+#define PT27_REG_WAKE_LVL       0x0036
+#define PT27_REG_WAKE_MODE      0x0038
+#define PT27_REG_WAKE_STAT      0x003A
+
+#define PT27_INT_MODE_MASK	0x3U
+#define PT27_MODE_LEVEL         0x0U
+#define PT27_MODE_RISING        0x1U
+#define PT27_MODE_FALLING       0x2U
+#define PT27_MODE_BOTH          0x3U
+
+#define PT27_AUTOSUSPEND_DELAY_MS   2000
+
+struct pt27_saved_regs {
+	u32 dir;
+	u32 out;
+	u32 int_en;
+	u32 int_lvl;
+	u32 int_mode0;
+	u16 int_mode1;
+	u32 int_mask;
+	u8 int_ctl;
+	u32 vendor0;
+	u32 vendor1;
+	u16 wake_en;
+	u16 wake_lvl;
+	u16 wake_mode;
+};
+
+struct pt27_gpio {
+	struct device          *dev;
+	struct gpio_chip        gc;
+	void __iomem           *base;
+	struct pt27_saved_regs  saved;
+	bool                    saved_valid;
+	raw_spinlock_t          lock;
+	struct mutex            irq_bus_lock;
+	bool                    irq_bus_active;
+	u32                     requested_irq_mask;
+	bool                    irq_pm_held;
+	u16                     wake_mask;
+	bool                    use_msi;
+	int                     irq;
+};
+
+static inline u32  pt27_rd32(struct pt27_gpio *c, u32 off)
+{
+	return ioread32(c->base + off);
+}
+
+static inline void pt27_wr32(struct pt27_gpio *c, u32 off, u32 v)
+{
+	iowrite32(v, c->base + off);
+}
+
+static inline u16  pt27_rd16(struct pt27_gpio *c, u32 off)
+{
+	return ioread16(c->base + off);
+}
+
+static inline void pt27_wr16(struct pt27_gpio *c, u32 off, u16 v)
+{
+	iowrite16(v, c->base + off);
+}
+
+static inline u8   pt27_rd8(struct pt27_gpio *c, u32 off)
+{
+	return ioread8(c->base + off);
+}
+
+static inline void pt27_wr8(struct pt27_gpio *c, u32 off, u8 v)
+{
+	iowrite8(v, c->base + off);
+}
+
+static void pt27_runtime_put_autosuspend(struct pt27_gpio *chip)
+{
+	pm_runtime_mark_last_busy(chip->dev);
+	pm_runtime_put_autosuspend(chip->dev);
+
+}
+
+static void pt27_save_regs(struct pt27_gpio *chip)
+{
+	chip->saved.dir       = pt27_rd32(chip, PT27_REG_DIR);
+	chip->saved.out       = pt27_rd32(chip, PT27_REG_OUT);
+	chip->saved.int_en    = pt27_rd32(chip, PT27_REG_INT_EN);
+	chip->saved.int_lvl   = pt27_rd32(chip, PT27_REG_INT_LVL);
+	chip->saved.int_mode0 = pt27_rd32(chip, PT27_REG_INT_MODE0);
+	chip->saved.int_mode1 = pt27_rd16(chip, PT27_REG_INT_MODE1);
+	chip->saved.int_mask  = pt27_rd32(chip, PT27_REG_INT_MASK);
+	chip->saved.int_ctl   = pt27_rd8(chip, PT27_REG_INT_CTL);
+	chip->saved.vendor0   = pt27_rd32(chip, PT27_REG_VENDOR0);
+	chip->saved.vendor1   = pt27_rd32(chip, PT27_REG_VENDOR1);
+	chip->saved.wake_en   = pt27_rd16(chip, PT27_REG_WAKE_EN);
+	chip->saved.wake_lvl  = pt27_rd16(chip, PT27_REG_WAKE_LVL);
+	chip->saved.wake_mode = pt27_rd16(chip, PT27_REG_WAKE_MODE);
+	chip->saved_valid = true;
+
+}
+
+static void pt27_restore_regs(struct pt27_gpio *chip)
+{
+	pt27_wr32(chip, PT27_REG_INT_EN, 0);
+	pt27_wr32(chip, PT27_REG_INT_MASK, PT27_GPIO_MASK);
+
+	pt27_wr32(chip, PT27_REG_DIR,       chip->saved.dir);
+	pt27_wr32(chip, PT27_REG_OUT,       chip->saved.out);
+	pt27_wr32(chip, PT27_REG_INT_LVL,   chip->saved.int_lvl);
+	pt27_wr32(chip, PT27_REG_INT_MODE0, chip->saved.int_mode0);
+	pt27_wr16(chip, PT27_REG_INT_MODE1, chip->saved.int_mode1);
+	pt27_wr8(chip, PT27_REG_INT_CTL,    chip->saved.int_ctl);
+	pt27_wr32(chip, PT27_REG_VENDOR0,   chip->saved.vendor0);
+	pt27_wr32(chip, PT27_REG_VENDOR1,   chip->saved.vendor1);
+
+	pt27_wr16(chip, PT27_REG_WAKE_LVL,  chip->saved.wake_lvl);
+	pt27_wr16(chip, PT27_REG_WAKE_MODE, chip->saved.wake_mode);
+	pt27_wr16(chip, PT27_REG_WAKE_EN,   chip->saved.wake_en);
+
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+
+	pt27_wr32(chip, PT27_REG_INT_EN,   chip->saved.int_en);
+	pt27_wr32(chip, PT27_REG_INT_MASK, chip->saved.int_mask);
+
+}
+
+static int pt27_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	val  = pt27_rd32(chip, PT27_REG_DIR);
+	val &= ~BIT(offset);   /* 0 = input */
+	pt27_wr32(chip, PT27_REG_DIR, val);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+	return 0;
+}
+
+static int pt27_gpio_direction_output(struct gpio_chip *gc, unsigned int offset,
+				      int value)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	val = pt27_rd32(chip, PT27_REG_OUT);
+	if (value)
+		val |= BIT(offset);
+	else
+		val &= ~BIT(offset);
+	pt27_wr32(chip, PT27_REG_OUT, val);
+
+	val  = pt27_rd32(chip, PT27_REG_DIR);
+	val |= BIT(offset);
+	pt27_wr32(chip, PT27_REG_DIR, val);
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+	return 0;
+}
+
+static int pt27_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	val = pt27_rd32(chip, PT27_REG_DIR);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+
+	return (val & BIT(offset)) ? GPIO_LINE_DIRECTION_OUT
+				   : GPIO_LINE_DIRECTION_IN;
+}
+
+static int pt27_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 dir, val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	dir = pt27_rd32(chip, PT27_REG_DIR);
+	val = pt27_rd32(chip, (dir & BIT(offset)) ? PT27_REG_OUT : PT27_REG_IN);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+
+	return !!(val & BIT(offset));
+}
+
+static int pt27_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0) {
+		dev_err_ratelimited(chip->dev,
+				    "%s: cannot resume to set GPIO%u: %d\n",
+				     __func__, offset, ret);
+		return ret;
+	}
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	val = pt27_rd32(chip, PT27_REG_OUT);
+	if (value)
+		val |= BIT(offset);
+	else
+		val &= ~BIT(offset);
+	pt27_wr32(chip, PT27_REG_OUT, val);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	pt27_runtime_put_autosuspend(chip);
+	return 0;
+}
+
+static u32 pt27_pending_irqs_locked(struct pt27_gpio *chip)
+{
+	u32 stat = pt27_rd32(chip, PT27_REG_INT_STAT);
+	u32 en   = pt27_rd32(chip, PT27_REG_INT_EN);
+	u32 mask = pt27_rd32(chip, PT27_REG_INT_MASK);
+
+	return stat & en & ~mask & PT27_GPIO_MASK;
+}
+
+static bool pt27_irq_regs_accessible(struct pt27_gpio *chip)
+{
+
+	return READ_ONCE(chip->irq_bus_active) ||
+	       READ_ONCE(chip->irq_pm_held);
+}
+
+static int pt27_irq_request_resources(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	int ret;
+
+	ret = gpiochip_reqres_irq(gc, hwirq);
+	if (ret)
+		return ret;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	chip->requested_irq_mask |= BIT(hwirq);
+	if (!chip->irq_pm_held) {
+		chip->irq_pm_held = true;
+
+		pm_runtime_get_noresume(chip->dev);
+	}
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static void pt27_irq_release_resources(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	bool drop_pm = false;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	chip->requested_irq_mask &= ~BIT(hwirq);
+	if (!chip->requested_irq_mask && chip->irq_pm_held) {
+		chip->irq_pm_held = false;
+		drop_pm = true;
+	}
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (drop_pm)
+		pt27_runtime_put_autosuspend(chip);
+
+	gpiochip_relres_irq(gc, hwirq);
+}
+
+static void pt27_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	u32 val;
+
+	gpiochip_disable_irq(gc, hwirq);
+
+	if (!pt27_irq_regs_accessible(chip)) {
+		dev_err_ratelimited(chip->dev,
+				    "%s: GPIO%lu remains masked; PT27 is not in D0\n",
+				     __func__, hwirq);
+		return;
+	}
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	val  = pt27_rd32(chip, PT27_REG_INT_MASK);
+	val |= BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_MASK, val);
+
+	val  = pt27_rd32(chip, PT27_REG_INT_EN);
+	val &= ~BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_EN, val);
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+}
+
+static void pt27_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	u32 val;
+
+	if (!pt27_irq_regs_accessible(chip)) {
+		dev_err_ratelimited(chip->dev,
+				    "%s: cannot unmask GPIO%lu; PT27 is not in D0\n",
+				     __func__, hwirq);
+		return;
+	}
+
+	gpiochip_enable_irq(gc, hwirq);
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	pt27_wr32(chip, PT27_REG_INT_STAT, BIT(hwirq));
+
+	val  = pt27_rd32(chip, PT27_REG_INT_EN);
+	val |= BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_EN, val);
+
+	val  = pt27_rd32(chip, PT27_REG_INT_MASK);
+	val &= ~BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_MASK, val);
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+}
+
+static void pt27_irq_ack(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+
+	pt27_wr32(chip, PT27_REG_INT_STAT, BIT(hwirq));
+}
+
+static int pt27_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	u32 lvl_reg;
+	u8  lvl_bit, mode_val;
+	unsigned int shift;
+
+	if (!pt27_irq_regs_accessible(chip))
+		return -EIO;
+
+	switch (type) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		lvl_bit  = 1; mode_val = PT27_MODE_LEVEL;   break;
+	case IRQ_TYPE_LEVEL_LOW:
+		lvl_bit  = 0; mode_val = PT27_MODE_LEVEL;   break;
+	case IRQ_TYPE_EDGE_RISING:
+		lvl_bit  = 1; mode_val = PT27_MODE_RISING;  break;
+	case IRQ_TYPE_EDGE_FALLING:
+		lvl_bit  = 0; mode_val = PT27_MODE_FALLING; break;
+	case IRQ_TYPE_EDGE_BOTH:
+		lvl_bit  = 0; mode_val = PT27_MODE_BOTH;    break;
+	default:
+		return -EINVAL;
+	}
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	lvl_reg = pt27_rd32(chip, PT27_REG_INT_LVL);
+	if (lvl_bit)
+		lvl_reg |= BIT(hwirq);
+	else
+		lvl_reg &= ~BIT(hwirq);
+	pt27_wr32(chip, PT27_REG_INT_LVL, lvl_reg);
+
+	if (hwirq < 16) {
+		u32 reg = pt27_rd32(chip, PT27_REG_INT_MODE0);
+
+		shift  = hwirq * 2;
+		reg   &= ~(PT27_INT_MODE_MASK << shift);
+		reg   |= ((u32)mode_val << shift);
+		pt27_wr32(chip, PT27_REG_INT_MODE0, reg);
+	} else {
+		u16 reg = pt27_rd16(chip, PT27_REG_INT_MODE1);
+
+		shift  = (hwirq - 16) * 2;
+		reg   &= ~((u16)(PT27_INT_MODE_MASK << shift));
+		reg   |= (u16)(mode_val << shift);
+		pt27_wr16(chip, PT27_REG_INT_MODE1, reg);
+	}
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (type & IRQ_TYPE_LEVEL_MASK)
+		irq_set_handler_locked(d, handle_level_irq);
+	else
+		irq_set_handler_locked(d, handle_edge_irq);
+
+	return 0;
+}
+
+static int pt27_irq_set_wake(struct irq_data *d, unsigned int on)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	irq_hw_number_t   hwirq = irqd_to_hwirq(d);
+	unsigned int trig = irqd_get_trigger_type(d);
+	unsigned long flags;
+	u16  wake_en, wake_lvl, wake_mode;
+
+	if (hwirq >= PT27_WAKE_GPIOS) {
+		dev_warn(chip->dev,
+			 "%s: GPIO%lu does not support wake (max GPIO%u)\n",
+			 __func__, hwirq, PT27_WAKE_GPIOS - 1);
+		return -EINVAL;
+	}
+	if (!pt27_irq_regs_accessible(chip))
+		return -EIO;
+	if (on && trig == IRQ_TYPE_NONE)
+		return -EINVAL;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	wake_en   = pt27_rd16(chip, PT27_REG_WAKE_EN);
+	wake_lvl  = pt27_rd16(chip, PT27_REG_WAKE_LVL);
+	wake_mode = pt27_rd16(chip, PT27_REG_WAKE_MODE);
+
+	if (on) {
+		wake_en |= BIT(hwirq);
+		chip->wake_mask |= (u16)BIT(hwirq);
+
+		if (trig & IRQ_TYPE_LEVEL_MASK) {
+			wake_mode |= BIT(hwirq);
+			if (trig == IRQ_TYPE_LEVEL_HIGH)
+				wake_lvl |= BIT(hwirq);
+			else
+				wake_lvl &= ~BIT(hwirq);
+		} else {
+			wake_mode &= ~BIT(hwirq);
+			if (trig == IRQ_TYPE_EDGE_RISING ||
+			    trig == IRQ_TYPE_EDGE_BOTH)
+				wake_lvl |= BIT(hwirq);
+			else
+				wake_lvl &= ~BIT(hwirq);
+		}
+	} else {
+		wake_en  &= ~BIT(hwirq);
+		chip->wake_mask &= ~(u16)BIT(hwirq);
+	}
+
+	pt27_wr16(chip, PT27_REG_WAKE_EN,   wake_en);
+	pt27_wr16(chip, PT27_REG_WAKE_LVL,  wake_lvl);
+	pt27_wr16(chip, PT27_REG_WAKE_MODE, wake_mode);
+
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static void pt27_irq_bus_lock(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+	int ret;
+
+	mutex_lock(&chip->irq_bus_lock);
+
+	ret = pm_runtime_resume_and_get(chip->dev);
+	if (ret < 0) {
+		WRITE_ONCE(chip->irq_bus_active, false);
+		dev_err_ratelimited(chip->dev,
+				    "%s: cannot resume for IRQ configuration: %d\n",
+				     __func__, ret);
+		return;
+	}
+
+	WRITE_ONCE(chip->irq_bus_active, true);
+}
+
+static void pt27_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
+	struct pt27_gpio *chip = gpiochip_get_data(gc);
+
+	if (READ_ONCE(chip->irq_bus_active)) {
+		WRITE_ONCE(chip->irq_bus_active, false);
+		pt27_runtime_put_autosuspend(chip);
+	}
+
+	mutex_unlock(&chip->irq_bus_lock);
+}
+
+static const struct irq_chip pt27_gpio_irq_chip = {
+	.name                = "pt27_gpio",
+	.irq_ack             = pt27_irq_ack,
+	.irq_mask            = pt27_irq_mask,
+	.irq_unmask          = pt27_irq_unmask,
+	.irq_set_type        = pt27_irq_set_type,
+	.irq_set_wake        = pt27_irq_set_wake,
+	.irq_bus_lock        = pt27_irq_bus_lock,
+	.irq_bus_sync_unlock = pt27_irq_bus_sync_unlock,
+	.irq_request_resources = pt27_irq_request_resources,
+	.irq_release_resources = pt27_irq_release_resources,
+	.flags               = IRQCHIP_IMMUTABLE,
+};
+
+static irqreturn_t pt27_gpio_isr(int irq, void *data)
+{
+	struct pt27_gpio *chip = data;
+	unsigned long pending;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	/*
+	 * No requested IRQ means nothing to dispatch, and the device may be
+	 * runtime-suspended (D3): a shared INTx line can still call us, so do
+	 * not touch MMIO unless the PM hold keeps the device in D0.
+	 */
+	if (!chip->irq_pm_held) {
+		raw_spin_unlock_irqrestore(&chip->lock, flags);
+		return IRQ_NONE;
+	}
+	pending = pt27_pending_irqs_locked(chip);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (!pending)
+		return IRQ_NONE;
+
+	while (pending) {
+		unsigned int hwirq = __ffs(pending);
+
+		pending &= ~BIT(hwirq);
+		generic_handle_domain_irq(chip->gc.irq.domain, hwirq);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void pt27_hw_init(struct pt27_gpio *chip)
+{
+	u8 int_ctl;
+
+	int_ctl  = pt27_rd8(chip, PT27_REG_INT_CTL);
+	int_ctl |=  PT27_INT_CTL_MODE;
+	int_ctl &= ~PT27_INT_CTL_PAD_EN;
+	pt27_wr8(chip, PT27_REG_INT_CTL, int_ctl);
+
+	pt27_wr32(chip, PT27_REG_INT_EN,    0);
+	pt27_wr32(chip, PT27_REG_INT_MASK,  PT27_GPIO_MASK);
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_EN,   0);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+}
+
+static int pt27_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct device    *dev = &pdev->dev;
+	struct pt27_gpio *chip;
+	struct gpio_irq_chip *girq;
+	unsigned long irq_flags;
+	int ret;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = dev;
+	raw_spin_lock_init(&chip->lock);
+	mutex_init(&chip->irq_bus_lock);
+	pci_set_drvdata(pdev, chip);
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable PCI device\n");
+
+	chip->base = pcim_iomap_region(pdev, 0, "pt27_gpio");
+	if (IS_ERR(chip->base))
+		return dev_err_probe(dev, PTR_ERR(chip->base), "failed to claim and map BAR0\n");
+
+	pci_set_master(pdev);
+
+	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to allocate IRQ vector\n");
+
+	chip->use_msi = pdev->msi_enabled || pdev->msix_enabled;
+	irq_flags     = chip->use_msi ? 0 : IRQF_SHARED;
+	chip->irq     = pci_irq_vector(pdev, 0);
+
+	pt27_hw_init(chip);
+
+	chip->gc.label            = dev_name(dev);
+	chip->gc.parent           = dev;
+	chip->gc.owner            = THIS_MODULE;
+	chip->gc.base             = -1;             /* Dynamic base assignment */
+	chip->gc.ngpio            = PT27_NUM_GPIOS;
+	chip->gc.direction_input  = pt27_gpio_direction_input;
+	chip->gc.direction_output = pt27_gpio_direction_output;
+	chip->gc.get_direction    = pt27_gpio_get_direction;
+	chip->gc.get              = pt27_gpio_get;
+	chip->gc.set              = pt27_gpio_set;
+	chip->gc.can_sleep        = true;
+
+	girq = &chip->gc.irq;
+	gpio_irq_chip_set_chip(girq, &pt27_gpio_irq_chip);
+	girq->parent_handler = NULL;
+	girq->num_parents    = 0;
+	girq->parents        = NULL;
+	girq->default_type   = IRQ_TYPE_NONE;
+	girq->handler        = handle_bad_irq;
+
+	ret = devm_device_init_wakeup(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable wake capability\n");
+
+
+	dev_pm_set_driver_flags(dev, DPM_FLAG_SMART_SUSPEND);
+	pm_runtime_set_autosuspend_delay(dev, PT27_AUTOSUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
+
+	ret = devm_gpiochip_add_data(dev, &chip->gc, chip);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register GPIO chip\n");
+
+	ret = devm_request_irq(dev, chip->irq, pt27_gpio_isr, irq_flags | IRQF_NO_THREAD, dev_name(dev), chip);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to request IRQ %d\n", chip->irq);
+
+	dev_info(dev,
+		 "PT27 GPIO controller: %u GPIOs (%u wake-capable), IRQ=%d (%s)\n",
+		 PT27_NUM_GPIOS, PT27_WAKE_GPIOS, chip->irq,
+		 chip->use_msi ? (pdev->msix_enabled ? "MSI-X" : "MSI") : "INTx");
+
+	pm_runtime_put_autosuspend(dev);
+	pm_runtime_allow(dev);
+	return 0;
+
+}
+
+static void pt27_gpio_remove(struct pci_dev *pdev)
+{
+	struct device     *dev  = &pdev->dev;
+	struct pt27_gpio  *chip = pci_get_drvdata(pdev);
+	unsigned long flags;
+	bool drop_pm;
+
+	/* Restore the canonical "unbound PCI device" Runtime-PM state. */
+	pm_runtime_forbid(dev);
+	pm_runtime_get_noresume(dev);
+	pm_runtime_dont_use_autosuspend(dev);
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	pt27_wr32(chip, PT27_REG_INT_EN,    0);
+	pt27_wr32(chip, PT27_REG_INT_MASK,  PT27_GPIO_MASK);
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_EN,   0);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+
+	chip->requested_irq_mask = 0;
+	chip->wake_mask        = 0;
+	drop_pm = chip->irq_pm_held;
+	chip->irq_pm_held = false;
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (drop_pm)
+		pm_runtime_put_noidle(dev);
+
+}
+
+static int __maybe_unused pt27_gpio_suspend(struct device *dev)
+{
+	struct pt27_gpio *chip = dev_get_drvdata(dev);
+	unsigned long flags;
+	u32 int_mask;
+
+	if (pm_runtime_status_suspended(dev)) {
+		dev_dbg(dev, "%s: already runtime-suspended, skipping save\n",
+			__func__);
+		return 0;
+	}
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	pt27_save_regs(chip);
+	pt27_wr32(chip, PT27_REG_INT_EN,    0);
+	int_mask = 0xFFFFFFFF & ~chip->wake_mask;
+	//int_mask = PT27_REG_INT_MASK & ~chip->wake_mask;
+	pt27_wr32(chip, PT27_REG_INT_MASK,  int_mask);
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	synchronize_irq(chip->irq);
+	return 0;
+}
+
+static int __maybe_unused pt27_gpio_resume(struct device *dev)
+{
+	struct pt27_gpio *chip = dev_get_drvdata(dev);
+	unsigned long flags;
+	u16 wake_status;
+
+	if (!chip->saved_valid)
+		return 0;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+	wake_status = pt27_rd16(chip, PT27_REG_WAKE_STAT) & PT27_WAKE_MASK;
+	pt27_restore_regs(chip);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	if (wake_status)
+		pm_wakeup_event(dev, 0);
+
+	return 0;
+}
+
+static int __maybe_unused pt27_gpio_runtime_suspend(struct device *dev)
+{
+	struct pt27_gpio *chip = dev_get_drvdata(dev);
+	unsigned long flags;
+	int ret = 0;
+
+	raw_spin_lock_irqsave(&chip->lock, flags);
+
+	if (chip->irq_pm_held || chip->requested_irq_mask) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+	if (chip->wake_mask && !device_may_wakeup(dev)) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+	if (pt27_pending_irqs_locked(chip)) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+	pt27_save_regs(chip);
+
+	pt27_wr32(chip, PT27_REG_INT_EN,    0);
+	pt27_wr32(chip, PT27_REG_INT_MASK,  PT27_GPIO_MASK);
+	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
+	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	synchronize_irq(chip->irq);
+	return 0;
+
+out_unlock:
+	raw_spin_unlock_irqrestore(&chip->lock, flags);
+
+	return ret;
+}
+
+static int __maybe_unused pt27_gpio_runtime_resume(struct device *dev)
+{
+
+	return pt27_gpio_resume(dev);
+}
+
+static const struct dev_pm_ops pt27_gpio_pm_ops __maybe_unused = {
+	SYSTEM_SLEEP_PM_OPS(pt27_gpio_suspend, pt27_gpio_resume)
+	RUNTIME_PM_OPS(pt27_gpio_runtime_suspend, pt27_gpio_runtime_resume, NULL)
+};
+
+#define PCI_DEVICE_ID_PT27	0x444b
+
+static const struct pci_device_id pt27_gpio_pci_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_PT27) },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, pt27_gpio_pci_ids);
+
+static struct pci_driver pt27_gpio_pci_driver = {
+	.name     = "ptpci-gpio",
+	.id_table = pt27_gpio_pci_ids,
+	.probe    = pt27_gpio_probe,
+	.remove   = pt27_gpio_remove,
+	.driver   = {
+		.pm = pm_ptr(&pt27_gpio_pm_ops),
+	},
+};
+
+module_pci_driver(pt27_gpio_pci_driver);
+
+MODULE_DESCRIPTION("AMD PT PCI GPIO Controller");
+MODULE_LICENSE("GPL");
+
--
2.48.1

==================================================================================================================
This email and any attachments to it contain confidential information and are intended solely for the use of the individual to whom it 
is addressed.If you are not the intended recipient or receive it accidentally, please immediately notify the sender by e-mail and delete 
the message and any attachments from your computer system, and destroy all hard copies. If any, please be advised that any unauthorized 
disclosure, copying, distribution or any action taken or omitted in reliance on this, is illegal and prohibited. Furthermore, any views 
or opinions expressed are solely those of the author and do not represent those of ASMedia Technology Inc. Thank you for your cooperation.
==================================================================================================================

Re: [PATCH v2] gpio: amdptpci: add support for AMD PT PCI controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] gpio: amdptpci: hardware access occurs after failed device resume
- [High] gpio: amdptpci: pending wakeup interrupts are destroyed during resume
- [High] pm: ISR accesses suspended MMIO due to irq_pm_held flag race
- [High] gpio: interrupt storm due to registering GPIO chip before parent IRQ
- [Medium] gpio: amdptpci: invalid bitmask writes to reserved bits during suspend
- [Medium] gpio: amdptpci: device can never runtime suspend if wake IRQ is configured
- [Medium] gpio: amdptpci: S3 wakeups broken if device autosuspends before system suspend
- [Low] gpio: amdptpci: missing pm_runtime_dont_use_autosuspend() on probe error path
--

commit 84767db1836e202d0ec481c787ac2cf3407a669a
Author: Szuying Chen <Chloe_Chen@asmedia.com.tw>

gpio: amdptpci: add support for AMD PT PCI controller

This commit implements the new gpio-amdptpci driver to support the
GPIO functionality on AMD PT PCI platforms.

> diff --git a/drivers/gpio/gpio-amdptpci.c b/drivers/gpio/gpio-amdptpci.c
> new file mode 100644
> index 0000000000000..c63e9b6dcf7ad
> --- /dev/null
> +++ b/drivers/gpio/gpio-amdptpci.c
[ ... ]
> +static void pt27_restore_regs(struct pt27_gpio *chip)
> +{
[ ... ]
> +	pt27_wr16(chip, PT27_REG_WAKE_LVL,  chip->saved.wake_lvl);
> +	pt27_wr16(chip, PT27_REG_WAKE_MODE, chip->saved.wake_mode);
> +	pt27_wr16(chip, PT27_REG_WAKE_EN,   chip->saved.wake_en);
> +
> +	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
> +	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);

[Severity: High]
Does writing the mask to the status register here clear all pending interrupts
unconditionally before the ISR can service them? 

When the system wakes up due to a GPIO event, the hardware logs the event in
the status register. If pt27_restore_regs() clears them assuming W1C
semantics, won't the core IRQ handler read 0 pending interrupts and return
IRQ_NONE, permanently losing the wakeup interrupt?

> +
> +	pt27_wr32(chip, PT27_REG_INT_EN,   chip->saved.int_en);
> +	pt27_wr32(chip, PT27_REG_INT_MASK, chip->saved.int_mask);
> +
> +}
[ ... ]
> +static bool pt27_irq_regs_accessible(struct pt27_gpio *chip)
> +{
> +
> +	return READ_ONCE(chip->irq_bus_active) ||
> +	       READ_ONCE(chip->irq_pm_held);
> +}

[Severity: High]
Will this OR condition cause the device to be accessed even if it failed to
resume? 

In pt27_irq_bus_lock() below, a failed resume sets chip->irq_bus_active to
false, but chip->irq_pm_held is already set to true by
pt27_irq_request_resources(). This means pt27_irq_regs_accessible() will
evaluate to true, and subsequent IRQ operations will attempt to write to MMIO
registers of a powered-down device.

[ ... ]
> +static int pt27_irq_request_resources(struct irq_data *d)
> +{
[ ... ]
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +	chip->requested_irq_mask |= BIT(hwirq);
> +	if (!chip->irq_pm_held) {
> +		chip->irq_pm_held = true;
> +
> +		pm_runtime_get_noresume(chip->dev);
> +	}
> +	raw_spin_unlock_irqrestore(&chip->lock, flags);

[Severity: High]
Is there a race condition here where the ISR might access suspended MMIO
hardware? 

This sets chip->irq_pm_held to true and calls pm_runtime_get_noresume(),
which only increments the usage counter but does not wake the device from D3.
The device is not actually woken until pt27_irq_bus_lock() runs. 

If a shared PCI INTx interrupt fires in this window, the ISR pt27_gpio_isr()
will see chip->irq_pm_held is true and bypass its suspend safeguard, leading
to an invalid MMIO read. Should this use pm_runtime_get_if_active() instead?

[ ... ]
> +static void pt27_irq_bus_lock(struct irq_data *d)
> +{
> +	struct gpio_chip *gc   = irq_data_get_irq_chip_data(d);
> +	struct pt27_gpio *chip = gpiochip_get_data(gc);
> +	int ret;
> +
> +	mutex_lock(&chip->irq_bus_lock);
> +
> +	ret = pm_runtime_resume_and_get(chip->dev);
> +	if (ret < 0) {
> +		WRITE_ONCE(chip->irq_bus_active, false);
> +		dev_err_ratelimited(chip->dev,
> +				    "%s: cannot resume for IRQ configuration: %d\n",
> +				     __func__, ret);
> +		return;
> +	}

[Severity: High]
If pm_runtime_resume_and_get() fails here, it correctly records that the bus
is inactive. Since the core IRQ operation is not aborted, and the static flag
forces pt27_irq_regs_accessible() to evaluate to true, will the outer IRQ
code proceed to interact with the device despite this failure?

[ ... ]
> +static irqreturn_t pt27_gpio_isr(int irq, void *data)
> +{
> +	struct pt27_gpio *chip = data;
> +	unsigned long pending;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +	/*
> +	 * No requested IRQ means nothing to dispatch, and the device may be
> +	 * runtime-suspended (D3): a shared INTx line can still call us, so do
> +	 * not touch MMIO unless the PM hold keeps the device in D0.
> +	 */
> +	if (!chip->irq_pm_held) {
> +		raw_spin_unlock_irqrestore(&chip->lock, flags);
> +		return IRQ_NONE;
> +	}
> +	pending = pt27_pending_irqs_locked(chip);

[Severity: High]
Could this ISR bypass its suspend safeguard based on the static flag race
discussed earlier? If the hardware is suspended but the flag evaluates to
true, could this lead to a bus error or spurious interrupt?

[ ... ]
> +static int pt27_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
[ ... ]
> +	ret = devm_gpiochip_add_data(dev, &chip->gc, chip);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register GPIO chip\n");
> +
> +	ret = devm_request_irq(dev, chip->irq, pt27_gpio_isr, irq_flags | IRQF_NO_THREAD, dev_name(dev), chip);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to request IRQ %d\n", chip->irq);

[Severity: High]
Can this initialization sequence lead to an unhandled interrupt storm? 

By calling devm_gpiochip_add_data() before devm_request_irq(), the GPIO chip
is exposed to consumers before its parent ISR is registered. If a consumer
immediately requests and unmasks a GPIO IRQ, the hardware will assert the
shared PCI IRQ line, but pt27_gpio_isr() will not yet be registered to handle
it.

Reversing the order is safe because if a spurious shared interrupt occurs
before the domain is initialized, chip->irq_pm_held will be false, and the
ISR will safely return IRQ_NONE without dereferencing anything.

[Severity: Low]
If either of these registration steps fail, the probe function returns
directly. Does this leak the PM configuration by missing a call to
pm_runtime_dont_use_autosuspend(dev) to revert the PM state?

[ ... ]
> +static int __maybe_unused pt27_gpio_suspend(struct device *dev)
> +{
> +	struct pt27_gpio *chip = dev_get_drvdata(dev);
> +	unsigned long flags;
> +	u32 int_mask;
> +
> +	if (pm_runtime_status_suspended(dev)) {
> +		dev_dbg(dev, "%s: already runtime-suspended, skipping save\n",
> +			__func__);
> +		return 0;
> +	}

[Severity: Medium]
Will returning early here break S3 wakeups if the device autosuspends before
system suspend? 

pt27_gpio_runtime_suspend() masks all interrupts, configuring no wake events.
By skipping the resume preparation when already suspended, this function
never unmasks wake-capable pins, completely breaking the controller's ability
to wake the system.

> +
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +	pt27_save_regs(chip);
> +	pt27_wr32(chip, PT27_REG_INT_EN,    0);
> +	int_mask = 0xFFFFFFFF & ~chip->wake_mask;
> +	//int_mask = PT27_REG_INT_MASK & ~chip->wake_mask;
> +	pt27_wr32(chip, PT27_REG_INT_MASK,  int_mask);

[Severity: Medium]
Does this write invalid bitmasks to reserved hardware bits? 

The controller supports 24 GPIOs (PT27_GPIO_MASK is 0x00FFFFFF). By using
0xFFFFFFFF, the bitwise NOT operation will write 1s to bits 24-31 of the
PT27_REG_INT_MASK register, which could trigger unintended hardware faults.

> +	pt27_wr32(chip, PT27_REG_INT_STAT,  PT27_GPIO_MASK);
> +	pt27_wr16(chip, PT27_REG_WAKE_STAT, PT27_WAKE_MASK);
> +	raw_spin_unlock_irqrestore(&chip->lock, flags);
> +
> +	synchronize_irq(chip->irq);
> +	return 0;
> +}
[ ... ]
> +static int __maybe_unused pt27_gpio_runtime_suspend(struct device *dev)
> +{
> +	struct pt27_gpio *chip = dev_get_drvdata(dev);
> +	unsigned long flags;
> +	int ret = 0;
> +
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +
> +	if (chip->irq_pm_held || chip->requested_irq_mask) {
> +		ret = -EBUSY;
> +		goto out_unlock;
> +	}
> +
> +	if (chip->wake_mask && !device_may_wakeup(dev)) {
> +		ret = -EBUSY;
> +		goto out_unlock;
> +	}

[Severity: Medium]
Does the first check unintentionally prevent the device from ever entering
runtime suspend when a wake IRQ is configured? 

A wake IRQ is inherently a requested IRQ, meaning chip->requested_irq_mask
will always be non-zero if chip->wake_mask is non-zero. The first block will
therefore unconditionally abort the suspend with -EBUSY, making the
subsequent wake_mask check dead code and preventing the PCI device from
reaching D3.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013081631.5841-1-Chloe_Chen@asmedia.com.tw?part=1

Re: [PATCH v2] gpio: amdptpci: add support for AMD PT PCI controller

[Markus Elfring]

> This commit implements the new gpio-amdptpci driver to support the
…

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.3-rc7#n94


> Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> Co-authored-by: YD Tseng <Yd_Tseng@asmedia.com.tw>

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.3-rc7#n502


> v1->v2:
…
> ---
…
>  drivers/gpio/gpio-amdptpci.c | 891 +++++++++++++++++++++++++++++++++++

Please move patch version descriptions behind the marker line.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.3-rc7#n793


…
> +++ b/drivers/gpio/gpio-amdptpci.c
> @@ -0,0 +1,891 @@
…
> +static int pt27_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
> +{
…
> +	raw_spin_lock_irqsave(&chip->lock, flags);
> +	val = pt27_rd32(chip, PT27_REG_DIR);
> +	raw_spin_unlock_irqrestore(&chip->lock, flags);
…

Under which circumstances would you become interested to apply a statement
like “guard(raw_spinlock_irqsave)(&chip->lock);”?
https://elixir.bootlin.com/linux/v7.3-rc6/source/include/linux/spinlock.h#L594-L597

Regards,
Markus