[PATCH v3 0/3] mfd: tps65217: Handle IRQ initialization errors

[PATCH v3 0/3] mfd: tps65217: Handle IRQ initialization errors

[Жамбакиев Радий Рикардинович]

From: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>

The TPS65217 MFD driver does not validate the outcome of IRQ
initialisation: tps65217_probe() ignores the return value of
tps65217_irq_init(), so a failed irq_domain creation leaves the
driver bound with a NULL tps->irq_domain and the remove callback
oopses, and a failed write of the interrupt mask leaves the software
mask out of sync with the hardware.

On top of that the irq_domain is torn down in the wrong order: the
remove callback disposes the mappings and removes the domain before
the MFD children are unbound, so children that requested interrupts
(e.g. tps65217-charger) release virtual IRQs whose descriptors are
already disposed, and on probe failure the domain is leaked with its
host_data pointing to freed memory. The enable_irq_wake() in probe
is never balanced.

This series plugs the probe error paths and moves the domain and
wake-up teardown into devres, registered right after the domain is
created. Devres actions are released in reverse order of
registration, so on both removal and probe failure the MFD children
release their IRQs first, then the parent interrupt is freed and its
thread stopped, and only then is the domain removed.

Changes since v2:
 - Replace "Fix irq_domain leak and use-after-free on probe failure"
   with "Use devres for IRQ domain and wake teardown". The domain
   teardown is a devres action registered before the parent
   interrupt, so it is released after devm_request_threaded_irq()
   has freed the interrupt and stopped the threaded handler. This
   closes the window flagged in review, in which a late interrupt
   could run the handler against a removed or NULL irq_domain, and
   makes the manual disable_irq()/synchronize_irq() on the error
   path unnecessary.
 - Drop "Fix NULL pointer dereference in remove callback", now
   obsolete: the remove callback is gone.
 - Added "From" to the body of the patches to avoid checkpatch.pl
   email name mismatch error.

Radiy Zhambakiev (3):
  mfd: tps65217: Fix NULL pointer dereference on IRQ init failure
  mfd: tps65217: Check return value when masking interrupt sources
  mfd: tps65217: Use devres for IRQ domain and wake teardown

 drivers/mfd/tps65217.c | 81 +++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 21 deletions(-)

-- 
2.55.0

[PATCH v3 1/3] mfd: tps65217: Fix NULL pointer dereference on IRQ init failure

[Жамбакиев Радий Рикардинович]

From: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>

tps65217_probe() ignores the return value of tps65217_irq_init(), so
when the irq domain creation fails the probe still completes and the
driver ends up bound with a NULL tps->irq_domain. Unloading the
module then makes tps65217_remove() call irq_domain_remove() on the
NULL pointer and oops the kernel. On top of that, irq_find_mapping()
may fall back to the default irq domain and dispose of mappings that
belong to other interrupt controllers.

Check the return value and abort the probe on failure so the error
is reported and no inconsistent state is left for removal.

Fixes: 6556bdacf646fcaa ("mfd: tps65217: Add support for IRQs")
Cc: stable@vger.kernel.org
Signed-off-by: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
---
 drivers/mfd/tps65217.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
index c240fac0ede7..2d04d9e0ae29 100644
--- a/drivers/mfd/tps65217.c
+++ b/drivers/mfd/tps65217.c
@@ -333,7 +333,9 @@ static int tps65217_probe(struct i2c_client *client)
 	}
 
 	if (client->irq) {
-		tps65217_irq_init(tps, client->irq);
+		ret = tps65217_irq_init(tps, client->irq);
+		if (ret)
+			return ret;
 	} else {
 		int i;
 
-- 
2.55.0

[PATCH v3 2/3] mfd: tps65217: Check return value when masking interrupt sources

[Жамбакиев Радий Рикардинович]

From: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>

tps65217_irq_init() ignores the error returned by
tps65217_set_bits() when masking all interrupt sources. A failed
register write leaves the driver's software mask out of sync with the
hardware and may result in spurious interrupts.

Check the return value and propagate the error to the caller.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 6556bdacf646fcaa ("mfd: tps65217: Add support for IRQs")
Cc: stable@vger.kernel.org
Signed-off-by: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
---
 drivers/mfd/tps65217.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
index 2d04d9e0ae29..9a1528456ffc 100644
--- a/drivers/mfd/tps65217.c
+++ b/drivers/mfd/tps65217.c
@@ -155,8 +155,13 @@ static int tps65217_irq_init(struct tps65217 *tps, int irq)
 
 	/* Mask all interrupt sources */
 	tps->irq_mask = TPS65217_INT_MASK;
-	tps65217_set_bits(tps, TPS65217_REG_INT, TPS65217_INT_MASK,
-			  TPS65217_INT_MASK, TPS65217_PROTECT_NONE);
+	ret = tps65217_set_bits(tps, TPS65217_REG_INT, TPS65217_INT_MASK,
+				TPS65217_INT_MASK, TPS65217_PROTECT_NONE);
+	if (ret) {
+		dev_err(tps->dev, "Failed to mask interrupt sources: %d\n",
+			ret);
+		return ret;
+	}
 
 	tps->irq_domain = irq_domain_create_linear(dev_fwnode(tps->dev), TPS65217_NUM_IRQ,
 						   &tps65217_irq_domain_ops, tps);
-- 
2.55.0

[PATCH v3 3/3] mfd: tps65217: Use devres for IRQ domain and wake teardown

[Жамбакиев Радий Рикардинович]

From: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>

Removing the parent device used to tear the IRQ domain down in
tps65217_remove() before the MFD children are unbound: the i2c core
only releases the parent's devres after the remove callback returns,
whereas devm_mfd_add_devices() registered its own devres action which
unbinds the children. Children that requested interrupts from the
domain (e.g. tps65217-charger) therefore call free_irq() on virtual
IRQs whose descriptors have already been disposed. With
CONFIG_SPARSE_IRQ free_irq() then returns early without stopping the
threaded handler, leaking the irqaction, the IRQ kthread and a module
reference.

Register the domain teardown as a devres action right after the domain
is created instead. Devres actions are released in reverse order of
registration, so the MFD children release their IRQs first, then the
parent interrupt is freed and its thread stopped, and only then is the
domain torn down. The same ordering covers probe failure, which no
longer needs manual cleanup, and the remove callback can be dropped
entirely.

Balance the enable_irq_wake() in tps65217_irq_init() by registering
disable_irq_wake() as a devres action as well. Only do so when
enabling actually succeeded, since the parent chip may not support
setting wake-up.

Fixes: 6556bdacf646fcaa ("mfd: tps65217: Add support for IRQs")
Cc: stable@vger.kernel.org
Signed-off-by: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
---
 drivers/mfd/tps65217.c | 68 +++++++++++++++++++++++++++++++-----------
 1 file changed, 50 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
index 9a1528456ffc..2d799b60c79b 100644
--- a/drivers/mfd/tps65217.c
+++ b/drivers/mfd/tps65217.c
@@ -146,6 +146,36 @@ static const struct irq_domain_ops tps65217_irq_domain_ops = {
 	.map = tps65217_irq_map,
 };
 
+static void tps65217_irq_cleanup(struct tps65217 *tps)
+{
+	unsigned int virq;
+	int i;
+
+	if (!tps->irq_domain)
+		return;
+
+	for (i = 0; i < TPS65217_NUM_IRQ; i++) {
+		virq = irq_find_mapping(tps->irq_domain, i);
+		if (virq)
+			irq_dispose_mapping(virq);
+	}
+
+	irq_domain_remove(tps->irq_domain);
+	tps->irq_domain = NULL;
+}
+
+static void tps65217_domain_release(void *data)
+{
+	struct tps65217 *tps = data;
+
+	tps65217_irq_cleanup(tps);
+}
+
+static void tps65217_irq_wake_disable(void *data)
+{
+	disable_irq_wake((unsigned int)(unsigned long)data);
+}
+
 static int tps65217_irq_init(struct tps65217 *tps, int irq)
 {
 	int ret;
@@ -170,6 +200,16 @@ static int tps65217_irq_init(struct tps65217 *tps, int irq)
 		return -ENOMEM;
 	}
 
+	/*
+	 * Devres actions are released in reverse order of registration,
+	 * so the domain is torn down after the parent interrupt and the
+	 * MFD children, which are registered later in probe, have
+	 * released their IRQs.
+	 */
+	ret = devm_add_action_or_reset(tps->dev, tps65217_domain_release, tps);
+	if (ret)
+		return ret;
+
 	ret = devm_request_threaded_irq(tps->dev, irq, NULL,
 					tps65217_irq_thread, IRQF_ONESHOT,
 					"tps65217-irq", tps);
@@ -179,7 +219,16 @@ static int tps65217_irq_init(struct tps65217 *tps, int irq)
 		return ret;
 	}
 
-	enable_irq_wake(irq);
+	ret = enable_irq_wake(irq);
+	if (ret) {
+		dev_warn(tps->dev, "failed to enable IRQ wake: %d\n", ret);
+	} else {
+		ret = devm_add_action_or_reset(tps->dev,
+				tps65217_irq_wake_disable,
+				(void *)(unsigned long)irq);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
@@ -380,22 +429,6 @@ static int tps65217_probe(struct i2c_client *client)
 	return 0;
 }
 
-static void tps65217_remove(struct i2c_client *client)
-{
-	struct tps65217 *tps = i2c_get_clientdata(client);
-	unsigned int virq;
-	int i;
-
-	for (i = 0; i < TPS65217_NUM_IRQ; i++) {
-		virq = irq_find_mapping(tps->irq_domain, i);
-		if (virq)
-			irq_dispose_mapping(virq);
-	}
-
-	irq_domain_remove(tps->irq_domain);
-	tps->irq_domain = NULL;
-}
-
 static const struct i2c_device_id tps65217_id_table[] = {
 	{"tps65217", TPS65217},
 	{ /* sentinel */ }
@@ -409,7 +442,6 @@ static struct i2c_driver tps65217_driver = {
 	},
 	.id_table	= tps65217_id_table,
 	.probe		= tps65217_probe,
-	.remove		= tps65217_remove,
 };
 
 static int __init tps65217_init(void)
-- 
2.55.0

Re: [PATCH v3 3/3] mfd: tps65217: Use devres for IRQ domain and wake teardown

[sashiko-bot]

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

Pre-existing issues:
- [High] The global static `mfd_cell` array `tps65217s` is dynamically mutated during probe, which violates MFD subsystem guidelines and corrupts platform data for subsequent probes.
--

commit 5ed0fd0cf78137c5505f644659e59027eb7df6d2
Author: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>

mfd: tps65217: Use devres for IRQ domain and wake teardown

This commit registers the IRQ domain teardown and disable_irq_wake() as
devres actions instead of manually cleaning them up in the remove callback.
It ensures that MFD children release their IRQs before the parent interrupt
is freed and the domain is torn down, fixing a use-after-free issue.

> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
> index 9a1528456ffc2..2d799b60c79b4 100644
> --- a/drivers/mfd/tps65217.c
> +++ b/drivers/mfd/tps65217.c
[ ... ]
> @@ -380,22 +429,6 @@ static int tps65217_probe(struct i2c_client *client)
>  	return 0;
>  }
>  

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this file I
noticed a potential issue with how the global MFD cell array is handled in
tps65217_probe().

Does tps65217_probe() persistently corrupt platform data if a device without
an assigned IRQ is probed?

The global array is defined as:

static struct mfd_cell tps65217s[] = {

If client->irq is 0, tps65217_probe() mutates this shared array:

	} else {
		int i;

		/* Don't tell children about IRQ resources which won't fire */
		for (i = 0; i < ARRAY_SIZE(tps65217s); i++)
			tps65217s[i].num_resources = 0;
	}

Because this array is shared, modifying it zeroes out the resources for all
subsequent probes of tps65217 devices, or even for the same device if its
probe defers and is retried later.

Should tps65217s be declared as static const to prevent this, and should
this dynamic modification be avoided?

> -static void tps65217_remove(struct i2c_client *client)
> -{
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904063439.69881-1-r.zhambakiev@prosoftsystems.ru?part=3

Re: [PATCH v3 1/3] mfd: tps65217: Fix NULL pointer dereference on IRQ init failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Dynamically mutating the global non-const tps65217s mfd_cell array during probe violates MFD architecture and corrupts state across devices.
--

commit 2466cdcefeecaa1a479ae1c537a7a6c523875175
Author: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>

mfd: tps65217: Fix NULL pointer dereference on IRQ init failure

tps65217_probe() ignores the return value of tps65217_irq_init(), so
when the irq domain creation fails the probe still completes and the
driver ends up bound with a NULL tps->irq_domain. Check the return value
and abort the probe on failure so the error is reported and no
inconsistent state is left for removal.

> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
> index c240fac0ede76..2d04d9e0ae29c 100644
> --- a/drivers/mfd/tps65217.c
> +++ b/drivers/mfd/tps65217.c
> @@ -333,7 +333,9 @@ static int tps65217_probe(struct i2c_client *client)
>  	}
>  
>  	if (client->irq) {
> -		tps65217_irq_init(tps, client->irq);
> +		ret = tps65217_irq_init(tps, client->irq);
> +		if (ret)
> +			return ret;
>  	} else {
>  		int i;

[Severity: High]
This is a pre-existing issue, but does the code following this else branch
corrupt the state for other devices?

If a device is probed with client->irq == 0, the else branch executes:

		/* Don't tell children about IRQ resources which won't fire */
		for (i = 0; i < ARRAY_SIZE(tps65217s); i++)
			tps65217s[i].num_resources = 0;

Because tps65217s is a global array defined earlier without const:

static struct mfd_cell tps65217s[] = {

This permanently zeroes num_resources for all instances in the module after
the first driver instantiation without an IRQ. This cross-device state
corruption leads to missing IRQ resources and non-functional driver features
for legitimately configured devices.

Should the mfd_cell array be defined as static const to adhere to MFD
architectural guidelines and prevent this state corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904063439.69881-1-r.zhambakiev@prosoftsystems.ru?part=1

Re: [PATCH v3 2/3] mfd: tps65217: Check return value when masking interrupt sources

[sashiko-bot]

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

Pre-existing issues:
- [High] Dynamic modification of a shared global mfd_cell array corrupts state for multiple device instances, violating MFD subsystem guidelines.
- [High] Exported register access helpers perform multi-step non-atomic operations without locking, risking data corruption and hardware sequence failures.
--

commit 0a08dce693b0c2fddb33c4fabc4a4eb02027d125
Author: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
mfd: tps65217: Check return value when masking interrupt sources

This commit updates tps65217_irq_init() to check the return value of
tps65217_set_bits() when masking interrupt sources. If the register write
fails, it logs an error and propagates the failure to the caller, preventing
spurious interrupts.

> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
> index 2d04d9e0ae29c..9a1528456ffc2 100644
> --- a/drivers/mfd/tps65217.c
> +++ b/drivers/mfd/tps65217.c
> @@ -155,8 +155,13 @@ static int tps65217_irq_init(struct tps65217 *tps, int irq)
>  
>  	/* Mask all interrupt sources */
>  	tps->irq_mask = TPS65217_INT_MASK;
> -	tps65217_set_bits(tps, TPS65217_REG_INT, TPS65217_INT_MASK,
> -			  TPS65217_INT_MASK, TPS65217_PROTECT_NONE);
> +	ret = tps65217_set_bits(tps, TPS65217_REG_INT, TPS65217_INT_MASK,
> +				TPS65217_INT_MASK, TPS65217_PROTECT_NONE);

[Severity: High]
This isn't a regression introduced by this patch, but since we are calling
tps65217_set_bits() here, can concurrent register accesses cause data
corruption?

Looking at tps65217_update_bits() in drivers/mfd/tps65217.c, it performs a
read-modify-write sequence without holding a driver-level mutex:

	ret = tps65217_reg_read(tps, reg, &data);
	...
	data &= ~mask;
	data |= val & mask;

	ret = tps65217_reg_write(tps, reg, data, level);

If two concurrent accesses from child drivers (such as regulators accessed
via sysfs) modify the same shared PMIC register, couldn't one of the updates
be overwritten?

Furthermore, tps65217_reg_write() performs multiple consecutive I2C writes
for password protection levels (e.g. L1/L2), which also appear to lack
locking:

	case TPS65217_PROTECT_L1:
		xor_reg_val = reg ^ TPS65217_PASSWORD_REGS_UNLOCK;
		ret = regmap_write(tps->regmap, TPS65217_REG_PASSWORD,
							xor_reg_val);
		if (ret < 0)
			return ret;

		return regmap_write(tps->regmap, reg, val);

Will concurrent password writes interleave and cause the PMIC I2C password
unlock sequence to fail?

> +	if (ret) {
> +		dev_err(tps->dev, "Failed to mask interrupt sources: %d\n",
> +			ret);
> +		return ret;
> +	}
>  
>  	tps->irq_domain = irq_domain_create_linear(dev_fwnode(tps->dev), TPS65217_NUM_IRQ,
>  						   &tps65217_irq_domain_ops, tps);

[Severity: High]
This isn't a regression introduced by this patch, but does the driver corrupt
shared global state during probe?

Looking at drivers/mfd/tps65217.c, the mfd_cell array is defined globally and
mutably instead of being static const:

static struct mfd_cell tps65217s[] = {
	{
		.name = "tps65217-pmic",
		...

Then in tps65217_probe(), if a device probes with client->irq == 0, the
driver iterates over this global array and mutates it:

	} else {
		int i;

		/* Don't tell children about IRQ resources which won't fire */
		for (i = 0; i < ARRAY_SIZE(tps65217s); i++)
			tps65217s[i].num_resources = 0;
	}

Doesn't this modification permanently strip IRQ resources from all future
instances of the driver? If a second TPS65217 chip is probed (or the module
is reloaded and the state persists incorrectly), won't it silently lose its
IRQ resources even if it possesses a valid IRQ? This dynamic mutation of
shared global state based on a single instance's properties violates MFD
subsystem guidelines.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904063439.69881-1-r.zhambakiev@prosoftsystems.ru?part=2

Re: [PATCH v3 2/3] mfd: tps65217: Check return value when masking interrupt sources

[Lee Jones]

On Fri, 04 Sep 2026, Жамбакиев Радий Рикардинович wrote:

> From: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
> 
> tps65217_irq_init() ignores the error returned by
> tps65217_set_bits() when masking all interrupt sources. A failed
> register write leaves the driver's software mask out of sync with the
> hardware and may result in spurious interrupts.
> 
> Check the return value and propagate the error to the caller.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 6556bdacf646fcaa ("mfd: tps65217: Add support for IRQs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
> ---
>  drivers/mfd/tps65217.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
> index 2d04d9e0ae29..9a1528456ffc 100644
> --- a/drivers/mfd/tps65217.c
> +++ b/drivers/mfd/tps65217.c
> @@ -155,8 +155,13 @@ static int tps65217_irq_init(struct tps65217 *tps, int irq)
>  
>  	/* Mask all interrupt sources */
>  	tps->irq_mask = TPS65217_INT_MASK;
> -	tps65217_set_bits(tps, TPS65217_REG_INT, TPS65217_INT_MASK,
> -			  TPS65217_INT_MASK, TPS65217_PROTECT_NONE);
> +	ret = tps65217_set_bits(tps, TPS65217_REG_INT, TPS65217_INT_MASK,
> +				TPS65217_INT_MASK, TPS65217_PROTECT_NONE);
> +	if (ret) {
> +		dev_err(tps->dev, "Failed to mask interrupt sources: %d\n",
> +			ret);

Nit: Why wrap here?  The line below is clearly longer than this one.

> +		return ret;
> +	}
>  
>  	tps->irq_domain = irq_domain_create_linear(dev_fwnode(tps->dev), TPS65217_NUM_IRQ,
>  						   &tps65217_irq_domain_ops, tps);
> -- 
> 2.55.0

-- 
Lee Jones

Re: [PATCH v3 3/3] mfd: tps65217: Use devres for IRQ domain and wake teardown

[Lee Jones]

On Fri, 04 Sep 2026, Жамбакиев Радий Рикардинович wrote:

> From: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
> 
> Removing the parent device used to tear the IRQ domain down in
> tps65217_remove() before the MFD children are unbound: the i2c core
> only releases the parent's devres after the remove callback returns,
> whereas devm_mfd_add_devices() registered its own devres action which
> unbinds the children. Children that requested interrupts from the
> domain (e.g. tps65217-charger) therefore call free_irq() on virtual
> IRQs whose descriptors have already been disposed. With
> CONFIG_SPARSE_IRQ free_irq() then returns early without stopping the
> threaded handler, leaking the irqaction, the IRQ kthread and a module
> reference.
> 
> Register the domain teardown as a devres action right after the domain
> is created instead. Devres actions are released in reverse order of
> registration, so the MFD children release their IRQs first, then the
> parent interrupt is freed and its thread stopped, and only then is the
> domain torn down. The same ordering covers probe failure, which no
> longer needs manual cleanup, and the remove callback can be dropped
> entirely.
> 
> Balance the enable_irq_wake() in tps65217_irq_init() by registering
> disable_irq_wake() as a devres action as well. Only do so when
> enabling actually succeeded, since the parent chip may not support
> setting wake-up.
> 
> Fixes: 6556bdacf646fcaa ("mfd: tps65217: Add support for IRQs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
> ---
>  drivers/mfd/tps65217.c | 68 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 50 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
> index 9a1528456ffc..2d799b60c79b 100644
> --- a/drivers/mfd/tps65217.c
> +++ b/drivers/mfd/tps65217.c
> @@ -146,6 +146,36 @@ static const struct irq_domain_ops tps65217_irq_domain_ops = {
>  	.map = tps65217_irq_map,
>  };
>  
> +static void tps65217_irq_cleanup(struct tps65217 *tps)
> +{
> +	unsigned int virq;
> +	int i;
> +
> +	if (!tps->irq_domain)
> +		return;
> +
> +	for (i = 0; i < TPS65217_NUM_IRQ; i++) {

for (int i = 0; ...

> +		virq = irq_find_mapping(tps->irq_domain, i);
> +		if (virq)
> +			irq_dispose_mapping(virq);
> +	}
> +
> +	irq_domain_remove(tps->irq_domain);
> +	tps->irq_domain = NULL;
> +}
> +
> +static void tps65217_domain_release(void *data)
> +{
> +	struct tps65217 *tps = data;
> +
> +	tps65217_irq_cleanup(tps);
> +}

Superfluous abstraction.  Please remove it.

> +
> +static void tps65217_irq_wake_disable(void *data)
> +{
> +	disable_irq_wake((unsigned int)(unsigned long)data);

Introduce a variable, then you can omit the casts.

> +}
> +
>  static int tps65217_irq_init(struct tps65217 *tps, int irq)
>  {
>  	int ret;
> @@ -170,6 +200,16 @@ static int tps65217_irq_init(struct tps65217 *tps, int irq)
>  		return -ENOMEM;
>  	}
>  
> +	/*
> +	 * Devres actions are released in reverse order of registration,
> +	 * so the domain is torn down after the parent interrupt and the
> +	 * MFD children, which are registered later in probe, have
> +	 * released their IRQs.
> +	 */

We don't usually describe how manages resources work when using devm_*().

> +	ret = devm_add_action_or_reset(tps->dev, tps65217_domain_release, tps);
> +	if (ret)
> +		return ret;
> +
>  	ret = devm_request_threaded_irq(tps->dev, irq, NULL,
>  					tps65217_irq_thread, IRQF_ONESHOT,
>  					"tps65217-irq", tps);
> @@ -179,7 +219,16 @@ static int tps65217_irq_init(struct tps65217 *tps, int irq)
>  		return ret;
>  	}
>  
> -	enable_irq_wake(irq);
> +	ret = enable_irq_wake(irq);
> +	if (ret) {
> +		dev_warn(tps->dev, "failed to enable IRQ wake: %d\n", ret);

Return zero here, then omit the else branch.

> +	} else {
> +		ret = devm_add_action_or_reset(tps->dev,
> +				tps65217_irq_wake_disable,
> +				(void *)(unsigned long)irq);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	return 0;
>  }
> @@ -380,22 +429,6 @@ static int tps65217_probe(struct i2c_client *client)
>  	return 0;
>  }
>  
> -static void tps65217_remove(struct i2c_client *client)
> -{
> -	struct tps65217 *tps = i2c_get_clientdata(client);
> -	unsigned int virq;
> -	int i;
> -
> -	for (i = 0; i < TPS65217_NUM_IRQ; i++) {
> -		virq = irq_find_mapping(tps->irq_domain, i);
> -		if (virq)
> -			irq_dispose_mapping(virq);
> -	}
> -
> -	irq_domain_remove(tps->irq_domain);
> -	tps->irq_domain = NULL;
> -}
> -
>  static const struct i2c_device_id tps65217_id_table[] = {
>  	{"tps65217", TPS65217},
>  	{ /* sentinel */ }
> @@ -409,7 +442,6 @@ static struct i2c_driver tps65217_driver = {
>  	},
>  	.id_table	= tps65217_id_table,
>  	.probe		= tps65217_probe,
> -	.remove		= tps65217_remove,
>  };

-- 
Lee Jones