[PATCH] gpio: ml-ioh: use raw_spinlock_t for the register lock

[PATCH] gpio: ml-ioh: use raw_spinlock_t for the register lock

[Junjie Cao]

ioh_irq_type() is registered as the irq_chip .irq_set_type callback and
takes chip->spinlock with spin_lock_irqsave().  This callback is reached
from __setup_irq() -> __irq_set_trigger() -> chip->irq_set_type() while
the caller holds desc->lock, a raw_spinlock_t, with hardirqs disabled.
That context is not sleepable, but on PREEMPT_RT a regular spinlock_t is
an rtmutex-backed sleeping lock, so acquiring it there is invalid.
ioh_irq_enable() and ioh_irq_disable() take the same lock from the
.irq_enable/.irq_disable callbacks, which are likewise invoked with
desc->lock held.

Convert the register lock to raw_spinlock_t.  The same lock also
serializes the GPIO direction/value callbacks and the suspend/resume
register save/restore, and those critical sections only perform short
sequences of MMIO register accesses (ioread32()/iowrite32()); the
.irq_set_type callback additionally emits a dev_warn() on an unsupported
type.  None of these are sleepable operations, so keeping this register
lock non-sleeping is appropriate for the irqchip callbacks and does not
change the GPIO-side locking contract.

This is the same fix as commit a02b8950d619 ("gpio: pch: use
raw_spinlock_t for the register lock"); this driver shares the same
structure as gpio-pch.

Fixes: 54be566317b6 ("gpio-ml-ioh: Support interrupt function")
Cc: stable@vger.kernel.org
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
 drivers/gpio/gpio-ml-ioh.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpio/gpio-ml-ioh.c b/drivers/gpio/gpio-ml-ioh.c
index 6576e5dcb0ee..0a9d34b7636e 100644
--- a/drivers/gpio/gpio-ml-ioh.c
+++ b/drivers/gpio/gpio-ml-ioh.c
@@ -84,7 +84,7 @@ struct ioh_gpio {
 	u32 gpio_use_sel;
 	int ch;
 	int irq_base;
-	spinlock_t spinlock;
+	raw_spinlock_t spinlock;
 };
 
 static const int num_ports[] = {6, 12, 16, 16, 15, 16, 16, 12};
@@ -95,7 +95,7 @@ static int ioh_gpio_set(struct gpio_chip *gpio, unsigned int nr, int val)
 	struct ioh_gpio *chip =	gpiochip_get_data(gpio);
 	unsigned long flags;
 
-	spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&chip->spinlock, flags);
 	reg_val = ioread32(&chip->reg->regs[chip->ch].po);
 	if (val)
 		reg_val |= BIT(nr);
@@ -103,7 +103,7 @@ static int ioh_gpio_set(struct gpio_chip *gpio, unsigned int nr, int val)
 		reg_val &= ~BIT(nr);
 
 	iowrite32(reg_val, &chip->reg->regs[chip->ch].po);
-	spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
 
 	return 0;
 }
@@ -123,7 +123,7 @@ static int ioh_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 	u32 reg_val;
 	unsigned long flags;
 
-	spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&chip->spinlock, flags);
 	pm = ioread32(&chip->reg->regs[chip->ch].pm);
 	pm &= BIT(num_ports[chip->ch]) - 1;
 	pm |= BIT(nr);
@@ -136,7 +136,7 @@ static int ioh_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 		reg_val &= ~BIT(nr);
 	iowrite32(reg_val, &chip->reg->regs[chip->ch].po);
 
-	spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
 
 	return 0;
 }
@@ -147,12 +147,12 @@ static int ioh_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
 	u32 pm;
 	unsigned long flags;
 
-	spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&chip->spinlock, flags);
 	pm = ioread32(&chip->reg->regs[chip->ch].pm);
 	pm &= BIT(num_ports[chip->ch]) - 1;
 	pm &= ~BIT(nr);
 	iowrite32(pm, &chip->reg->regs[chip->ch].pm);
-	spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
 
 	return 0;
 }
@@ -256,7 +256,7 @@ static int ioh_irq_type(struct irq_data *d, unsigned int type)
 	dev_dbg(chip->dev, "%s:irq=%d type=%d ch=%d pos=%d type=%d\n",
 		__func__, irq, type, ch, im_pos, type);
 
-	spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&chip->spinlock, flags);
 
 	switch (type) {
 	case IRQ_TYPE_EDGE_RISING:
@@ -296,7 +296,7 @@ static int ioh_irq_type(struct irq_data *d, unsigned int type)
 	ien = ioread32(&chip->reg->regs[chip->ch].ien);
 	iowrite32(ien | BIT(ch), &chip->reg->regs[chip->ch].ien);
 end:
-	spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
 
 	return 0;
 }
@@ -326,11 +326,11 @@ static void ioh_irq_disable(struct irq_data *d)
 	unsigned long flags;
 	u32 ien;
 
-	spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&chip->spinlock, flags);
 	ien = ioread32(&chip->reg->regs[chip->ch].ien);
 	ien &= ~BIT(d->irq - chip->irq_base);
 	iowrite32(ien, &chip->reg->regs[chip->ch].ien);
-	spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
 }
 
 static void ioh_irq_enable(struct irq_data *d)
@@ -340,11 +340,11 @@ static void ioh_irq_enable(struct irq_data *d)
 	unsigned long flags;
 	u32 ien;
 
-	spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&chip->spinlock, flags);
 	ien = ioread32(&chip->reg->regs[chip->ch].ien);
 	ien |= BIT(d->irq - chip->irq_base);
 	iowrite32(ien, &chip->reg->regs[chip->ch].ien);
-	spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
 }
 
 static irqreturn_t ioh_gpio_handler(int irq, void *dev_id)
@@ -440,7 +440,7 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 		chip->base = base;
 		chip->reg = chip->base;
 		chip->ch = i;
-		spin_lock_init(&chip->spinlock);
+		raw_spin_lock_init(&chip->spinlock);
 		ioh_gpio_setup(chip, num_ports[i]);
 		ret = devm_gpiochip_add_data(dev, &chip->gpio, chip);
 		if (ret) {
@@ -484,9 +484,9 @@ static int ioh_gpio_suspend(struct device *dev)
 	struct ioh_gpio *chip = dev_get_drvdata(dev);
 	unsigned long flags;
 
-	spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&chip->spinlock, flags);
 	ioh_gpio_save_reg_conf(chip);
-	spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
 
 	return 0;
 }
@@ -496,11 +496,11 @@ static int ioh_gpio_resume(struct device *dev)
 	struct ioh_gpio *chip = dev_get_drvdata(dev);
 	unsigned long flags;
 
-	spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&chip->spinlock, flags);
 	iowrite32(0x01, &chip->reg->srst);
 	iowrite32(0x00, &chip->reg->srst);
 	ioh_gpio_restore_reg_conf(chip);
-	spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
 
 	return 0;
 }

base-commit: a02b8950d619123da64f69b70fe1dadef217dfe4
-- 
2.43.0

Re: [PATCH] gpio: ml-ioh: use raw_spinlock_t for the register lock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Suspend and resume functions bypass spinlocks for channels 1-7, causing data races on hardware registers.
- [High] The driver uses `handle_simple_irq` for all interrupts, which causes an interrupt storm for level-triggered threaded interrupts.
--

--- Patch [1]: [PATCH] gpio: ml-ioh: use raw_spinlock_t for the register lock ---
commit c58bab978628257e8b70587775813cefa999a4d6
Author: Junjie Cao <junjie.cao@intel.com>

gpio: ml-ioh: use raw_spinlock_t for the register lock

This patch converts the driver's register lock from spinlock_t to
raw_spinlock_t. This fixes an issue on PREEMPT_RT where irqchip callbacks
invoked with hardirqs disabled would attempt to acquire a sleepable lock.

> diff --git a/drivers/gpio/gpio-ml-ioh.c b/drivers/gpio/gpio-ml-ioh.c
> index 6576e5dcb0eef..0a9d34b7636ef 100644
> --- a/drivers/gpio/gpio-ml-ioh.c
> +++ b/drivers/gpio/gpio-ml-ioh.c

[ ... ]

> @@ -256,7 +256,7 @@ static int ioh_irq_type(struct irq_data *d, unsigned int type)
>  	dev_dbg(chip->dev, "%s:irq=%d type=%d ch=%d pos=%d type=%d\n",
>  		__func__, irq, type, ch, im_pos, type);
>  
> -	spin_lock_irqsave(&chip->spinlock, flags);
> +	raw_spin_lock_irqsave(&chip->spinlock, flags);
>  
>  	switch (type) {
>  	case IRQ_TYPE_EDGE_RISING:

[Severity: High]
This is a pre-existing issue, but does this driver fail to assign the
correct flow handler for level-triggered interrupts?

During initialization, ioh_gpio_alloc_generic_chip() in
drivers/gpio/gpio-ml-ioh.c assigns handle_simple_irq to all interrupts:

gc = devm_irq_alloc_generic_chip(chip->dev, "ioh_gpio", 1, irq_start,
                                 chip->base, handle_simple_irq);

When a client driver requests a level-triggered threaded interrupt,
shouldn't ioh_irq_type() call irq_set_handler_locked() to switch the
flow handler to handle_level_irq?

Because handle_simple_irq does not mask the interrupt before waking the
thread, returning from the primary handler will immediately re-assert the
interrupt if the external hardware hasn't been serviced yet, leading to a
hardirq storm.

[ ... ]

> @@ -484,9 +484,9 @@ static int ioh_gpio_suspend(struct device *dev)
>  	struct ioh_gpio *chip = dev_get_drvdata(dev);
>  	unsigned long flags;
>  
> -	spin_lock_irqsave(&chip->spinlock, flags);
> +	raw_spin_lock_irqsave(&chip->spinlock, flags);
>  	ioh_gpio_save_reg_conf(chip);

[Severity: High]
This isn't a bug introduced by this patch, but does this lock only protect
the first of the eight GPIO chips?

In ioh_gpio_suspend(), we retrieve the base pointer to an array of 8 chips
and acquire the lock for chip[0]. However, ioh_gpio_save_reg_conf()
iterates through and saves registers for all 8 chips without acquiring
their respective locks:

static void ioh_gpio_save_reg_conf(struct ioh_gpio *chip)
{
    int i;
    for (i = 0; i < 8; i++, chip++) {
        chip->ioh_gpio_reg.po_reg =
                    ioread32(&chip->reg->regs[chip->ch].po);
        ...

If a concurrent operation accesses channels 1-7 during this time, it
will acquire the lock for that specific channel (like chip[1].spinlock),
which isn't held by the suspend path, potentially causing a data race.

> -	spin_unlock_irqrestore(&chip->spinlock, flags);
> +	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
>  
>  	return 0;
>  }
> @@ -496,11 +496,11 @@ static int ioh_gpio_resume(struct device *dev)
>  	struct ioh_gpio *chip = dev_get_drvdata(dev);
>  	unsigned long flags;
>  
> -	spin_lock_irqsave(&chip->spinlock, flags);
> +	raw_spin_lock_irqsave(&chip->spinlock, flags);
>  	iowrite32(0x01, &chip->reg->srst);
>  	iowrite32(0x00, &chip->reg->srst);
>  	ioh_gpio_restore_reg_conf(chip);

[Severity: High]
This is a pre-existing issue, but does the same data race occur here
during resume?

ioh_gpio_restore_reg_conf() also loops over all 8 chips and blindly
restores their registers, protected solely by chip[0]'s spinlock. Could
this overwrite concurrent changes to channels 1-7?

> -	spin_unlock_irqrestore(&chip->spinlock, flags);
> +	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731032747.2987292-1-junjie.cao@intel.com?part=1

Re: [PATCH] gpio: ml-ioh: use raw_spinlock_t for the register lock

[Linus Walleij]

On Fri, Jul 31, 2026 at 5:30 AM Junjie Cao <junjie.cao@intel.com> wrote:

> ioh_irq_type() is registered as the irq_chip .irq_set_type callback and
> takes chip->spinlock with spin_lock_irqsave().  This callback is reached
> from __setup_irq() -> __irq_set_trigger() -> chip->irq_set_type() while
> the caller holds desc->lock, a raw_spinlock_t, with hardirqs disabled.
> That context is not sleepable, but on PREEMPT_RT a regular spinlock_t is
> an rtmutex-backed sleeping lock, so acquiring it there is invalid.
> ioh_irq_enable() and ioh_irq_disable() take the same lock from the
> .irq_enable/.irq_disable callbacks, which are likewise invoked with
> desc->lock held.
>
> Convert the register lock to raw_spinlock_t.  The same lock also
> serializes the GPIO direction/value callbacks and the suspend/resume
> register save/restore, and those critical sections only perform short
> sequences of MMIO register accesses (ioread32()/iowrite32()); the
> .irq_set_type callback additionally emits a dev_warn() on an unsupported
> type.  None of these are sleepable operations, so keeping this register
> lock non-sleeping is appropriate for the irqchip callbacks and does not
> change the GPIO-side locking contract.
>
> This is the same fix as commit a02b8950d619 ("gpio: pch: use
> raw_spinlock_t for the register lock"); this driver shares the same
> structure as gpio-pch.
>
> Fixes: 54be566317b6 ("gpio-ml-ioh: Support interrupt function")
> Cc: stable@vger.kernel.org
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

If you have time, look into the issue reported by sashiko about suspend/resume
ignoring the spinlock.

Thanks!
Linus Walleij

Re: [PATCH] gpio: ml-ioh: use raw_spinlock_t for the register lock

[Junjie Cao]

Hi Linus,

Thanks for the review.

I checked it, and the report is correct. The PM callbacks take
chip[0].spinlock while the save and restore helpers walk all eight
channels, each with its own lock.

I have a follow-up that uses one lock shared by all channels. I'll send
it separately on top of this patch.

Thanks,
Junjie

[PATCH] gpio: ml-ioh: share the register lock across channels

[Junjie Cao]

Suspend and resume hold channel 0's lock while saving and restoring
registers for all eight channels. Code paths using the other seven locks
can therefore run concurrently with PM.

Use one controller-wide lock shared by all channels.

Fixes: b490fa0bf86e ("gpio-ml-ioh: Fix suspend/resume issue")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260731033956.EE6F61F000E9@smtp.kernel.org
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
 drivers/gpio/gpio-ml-ioh.c | 69 ++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 32 deletions(-)

diff --git a/drivers/gpio/gpio-ml-ioh.c b/drivers/gpio/gpio-ml-ioh.c
index 0a9d34b7636e..391188c0f2c2 100644
--- a/drivers/gpio/gpio-ml-ioh.c
+++ b/drivers/gpio/gpio-ml-ioh.c
@@ -73,7 +73,7 @@ struct ioh_gpio_reg_data {
  * @gpio_use_sel:		Save GPIO_USE_SEL1~4 register for PM
  * @ch:				Indicate GPIO channel
  * @irq_base:		Save base of IRQ number for interrupt
- * @spinlock:		Used for register access protection
+ * @spinlock:		Shared register access lock
  */
 struct ioh_gpio {
 	void __iomem *base;
@@ -84,7 +84,12 @@ struct ioh_gpio {
 	u32 gpio_use_sel;
 	int ch;
 	int irq_base;
+	raw_spinlock_t *spinlock;
+};
+
+struct ioh_gpio_device {
 	raw_spinlock_t spinlock;
+	struct ioh_gpio chip[8];
 };
 
 static const int num_ports[] = {6, 12, 16, 16, 15, 16, 16, 12};
@@ -95,7 +100,7 @@ static int ioh_gpio_set(struct gpio_chip *gpio, unsigned int nr, int val)
 	struct ioh_gpio *chip =	gpiochip_get_data(gpio);
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(chip->spinlock, flags);
 	reg_val = ioread32(&chip->reg->regs[chip->ch].po);
 	if (val)
 		reg_val |= BIT(nr);
@@ -103,7 +108,7 @@ static int ioh_gpio_set(struct gpio_chip *gpio, unsigned int nr, int val)
 		reg_val &= ~BIT(nr);
 
 	iowrite32(reg_val, &chip->reg->regs[chip->ch].po);
-	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(chip->spinlock, flags);
 
 	return 0;
 }
@@ -123,7 +128,7 @@ static int ioh_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 	u32 reg_val;
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(chip->spinlock, flags);
 	pm = ioread32(&chip->reg->regs[chip->ch].pm);
 	pm &= BIT(num_ports[chip->ch]) - 1;
 	pm |= BIT(nr);
@@ -136,7 +141,7 @@ static int ioh_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 		reg_val &= ~BIT(nr);
 	iowrite32(reg_val, &chip->reg->regs[chip->ch].po);
 
-	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(chip->spinlock, flags);
 
 	return 0;
 }
@@ -147,12 +152,12 @@ static int ioh_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
 	u32 pm;
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(chip->spinlock, flags);
 	pm = ioread32(&chip->reg->regs[chip->ch].pm);
 	pm &= BIT(num_ports[chip->ch]) - 1;
 	pm &= ~BIT(nr);
 	iowrite32(pm, &chip->reg->regs[chip->ch].pm);
-	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(chip->spinlock, flags);
 
 	return 0;
 }
@@ -256,7 +261,7 @@ static int ioh_irq_type(struct irq_data *d, unsigned int type)
 	dev_dbg(chip->dev, "%s:irq=%d type=%d ch=%d pos=%d type=%d\n",
 		__func__, irq, type, ch, im_pos, type);
 
-	raw_spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(chip->spinlock, flags);
 
 	switch (type) {
 	case IRQ_TYPE_EDGE_RISING:
@@ -296,7 +301,7 @@ static int ioh_irq_type(struct irq_data *d, unsigned int type)
 	ien = ioread32(&chip->reg->regs[chip->ch].ien);
 	iowrite32(ien | BIT(ch), &chip->reg->regs[chip->ch].ien);
 end:
-	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(chip->spinlock, flags);
 
 	return 0;
 }
@@ -326,11 +331,11 @@ static void ioh_irq_disable(struct irq_data *d)
 	unsigned long flags;
 	u32 ien;
 
-	raw_spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(chip->spinlock, flags);
 	ien = ioread32(&chip->reg->regs[chip->ch].ien);
 	ien &= ~BIT(d->irq - chip->irq_base);
 	iowrite32(ien, &chip->reg->regs[chip->ch].ien);
-	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(chip->spinlock, flags);
 }
 
 static void ioh_irq_enable(struct irq_data *d)
@@ -340,11 +345,11 @@ static void ioh_irq_enable(struct irq_data *d)
 	unsigned long flags;
 	u32 ien;
 
-	raw_spin_lock_irqsave(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(chip->spinlock, flags);
 	ien = ioread32(&chip->reg->regs[chip->ch].ien);
 	ien |= BIT(d->irq - chip->irq_base);
 	iowrite32(ien, &chip->reg->regs[chip->ch].ien);
-	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_unlock_irqrestore(chip->spinlock, flags);
 }
 
 static irqreturn_t ioh_gpio_handler(int irq, void *dev_id)
@@ -407,8 +412,8 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 	int ret;
 	int i, j;
 	struct ioh_gpio *chip;
+	struct ioh_gpio_device *priv;
 	void __iomem *base;
-	void *chip_save;
 	int irq_base;
 
 	ret = pcim_enable_device(pdev);
@@ -429,18 +434,18 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 		return -ENOMEM;
 	}
 
-	chip_save = devm_kcalloc(dev, 8, sizeof(*chip), GFP_KERNEL);
-	if (chip_save == NULL) {
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
 		return -ENOMEM;
-	}
 
-	chip = chip_save;
+	raw_spin_lock_init(&priv->spinlock);
+	chip = priv->chip;
 	for (i = 0; i < 8; i++, chip++) {
 		chip->dev = dev;
 		chip->base = base;
 		chip->reg = chip->base;
 		chip->ch = i;
-		raw_spin_lock_init(&chip->spinlock);
+		chip->spinlock = &priv->spinlock;
 		ioh_gpio_setup(chip, num_ports[i]);
 		ret = devm_gpiochip_add_data(dev, &chip->gpio, chip);
 		if (ret) {
@@ -449,7 +454,7 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 		}
 	}
 
-	chip = chip_save;
+	chip = priv->chip;
 	for (j = 0; j < 8; j++, chip++) {
 		irq_base = devm_irq_alloc_descs(dev, -1, IOH_IRQ_BASE,
 						num_ports[j], NUMA_NO_NODE);
@@ -466,7 +471,7 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 			return ret;
 	}
 
-	chip = chip_save;
+	chip = priv->chip;
 	ret = devm_request_irq(dev, pdev->irq, ioh_gpio_handler,
 			       IRQF_SHARED, KBUILD_MODNAME, chip);
 	if (ret != 0) {
@@ -474,33 +479,33 @@ static int ioh_gpio_probe(struct pci_dev *pdev,
 		return ret;
 	}
 
-	pci_set_drvdata(pdev, chip);
+	pci_set_drvdata(pdev, priv);
 
 	return 0;
 }
 
 static int ioh_gpio_suspend(struct device *dev)
 {
-	struct ioh_gpio *chip = dev_get_drvdata(dev);
+	struct ioh_gpio_device *priv = dev_get_drvdata(dev);
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&chip->spinlock, flags);
-	ioh_gpio_save_reg_conf(chip);
-	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&priv->spinlock, flags);
+	ioh_gpio_save_reg_conf(priv->chip);
+	raw_spin_unlock_irqrestore(&priv->spinlock, flags);
 
 	return 0;
 }
 
 static int ioh_gpio_resume(struct device *dev)
 {
-	struct ioh_gpio *chip = dev_get_drvdata(dev);
+	struct ioh_gpio_device *priv = dev_get_drvdata(dev);
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&chip->spinlock, flags);
-	iowrite32(0x01, &chip->reg->srst);
-	iowrite32(0x00, &chip->reg->srst);
-	ioh_gpio_restore_reg_conf(chip);
-	raw_spin_unlock_irqrestore(&chip->spinlock, flags);
+	raw_spin_lock_irqsave(&priv->spinlock, flags);
+	iowrite32(0x01, &priv->chip->reg->srst);
+	iowrite32(0x00, &priv->chip->reg->srst);
+	ioh_gpio_restore_reg_conf(priv->chip);
+	raw_spin_unlock_irqrestore(&priv->spinlock, flags);
 
 	return 0;
 }

base-commit: a02b8950d619123da64f69b70fe1dadef217dfe4
prerequisite-patch-id: cebc220b508b7d9216be1fe01348b9989f0b957e
-- 
2.43.0

Re: [PATCH] gpio: ml-ioh: share the register lock across channels

[Linus Walleij]

On Tue, Aug 4, 2026 at 12:00 PM Junjie Cao <junjie.cao@intel.com> wrote:

> Suspend and resume hold channel 0's lock while saving and restoring
> registers for all eight channels. Code paths using the other seven locks
> can therefore run concurrently with PM.
>
> Use one controller-wide lock shared by all channels.
>
> Fixes: b490fa0bf86e ("gpio-ml-ioh: Fix suspend/resume issue")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/r/20260731033956.EE6F61F000E9@smtp.kernel.org
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH] gpio: ml-ioh: share the register lock across channels

[Bartosz Golaszewski]

On Tue, 04 Aug 2026 17:59:35 +0800, Junjie Cao wrote:
> Suspend and resume hold channel 0's lock while saving and restoring
> registers for all eight channels. Code paths using the other seven locks
> can therefore run concurrently with PM.
> 
> Use one controller-wide lock shared by all channels.
> 
> 
> [...]

Applied, thanks!

[1/1] gpio: ml-ioh: share the register lock across channels
      https://git.kernel.org/brgl/c/a9253ee6771c8ab3c6de07ea75d9e2c1cef3cd97

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>