All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes
@ 2026-09-06 15:58 Felix Gu
  2026-09-06 15:58 ` [PATCH 1/2] iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls Felix Gu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Felix Gu @ 2026-09-06 15:58 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Quentin Schulz,
	Maxime Ripard
  Cc: linux-iio, linux-arm-kernel, linux-sunxi, linux-kernel, Felix Gu,
	Jonathan Cameron

Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
Felix Gu (2):
      iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls
      iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure

 drivers/iio/adc/sun4i-gpadc-iio.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
---
base-commit: 9d80aa4617b32f5054c5aa471d06b66704854935
change-id: 20260906-gpadc-d41b964d8f17

Best regards,
--  
Felix Gu <ustc.gu@gmail.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls
  2026-09-06 15:58 [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes Felix Gu
@ 2026-09-06 15:58 ` Felix Gu
  2026-09-06 16:08   ` sashiko-bot
  2026-09-06 15:58 ` [PATCH 2/2] iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure Felix Gu
  2026-09-06 17:36 ` [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes Jonathan Cameron
  2 siblings, 1 reply; 6+ messages in thread
From: Felix Gu @ 2026-09-06 15:58 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Quentin Schulz,
	Maxime Ripard
  Cc: linux-iio, linux-arm-kernel, linux-sunxi, linux-kernel, Felix Gu,
	Jonathan Cameron

Neither the error path in sun4i_gpadc_probe() nor sun4i_gpadc_remove()
ever holds a runtime PM usage count. So the pm_runtime_put() in both
places always triggers the "Runtime PM usage count underflow!" warning
on every failed probe and every unbind.

Drop both calls.

Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 203459ca9907..85382e435f2a 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -657,7 +657,6 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
 	if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
 		iio_map_array_unregister(indio_dev);
 
-	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	return ret;
@@ -668,7 +667,6 @@ static void sun4i_gpadc_remove(struct platform_device *pdev)
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
 	struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
 
-	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	if (!IS_ENABLED(CONFIG_THERMAL_OF))

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure
  2026-09-06 15:58 [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes Felix Gu
  2026-09-06 15:58 ` [PATCH 1/2] iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls Felix Gu
@ 2026-09-06 15:58 ` Felix Gu
  2026-09-06 16:08   ` sashiko-bot
  2026-09-06 17:36 ` [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes Jonathan Cameron
  2 siblings, 1 reply; 6+ messages in thread
From: Felix Gu @ 2026-09-06 15:58 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Quentin Schulz,
	Maxime Ripard
  Cc: linux-iio, linux-arm-kernel, linux-sunxi, linux-kernel, Felix Gu,
	Jonathan Cameron

If devm_thermal_of_zone_register() fails, probe returns without
unregistering the IIO map array or disabling runtime PM.

Jump to err_map to release them.

Fixes: b0a242894f11 ("iio: adc: sun4i-gpadc-iio: register in the thermal after registering in pm")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/iio/adc/sun4i-gpadc-iio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index 85382e435f2a..ac5ba76b979e 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -638,10 +638,10 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
 		 * thermal because no thermal DT node is found.
 		 */
 		if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
-			dev_err(&pdev->dev,
-				"could not register thermal sensor: %ld\n",
-				PTR_ERR(info->tzd));
-			return PTR_ERR(info->tzd);
+			ret = dev_err_probe(&pdev->dev,
+					    PTR_ERR(info->tzd),
+					    "could not register thermal sensor\n");
+			goto err_map;
 		}
 	}
 

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls
  2026-09-06 15:58 ` [PATCH 1/2] iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls Felix Gu
@ 2026-09-06 16:08   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-06 16:08 UTC (permalink / raw)
  To: Felix Gu; +Cc: linux-sunxi

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

Pre-existing issues:
- [High] Unbalanced IRQ Disable on error path permanently breaks device interrupts.
- [High] Ignored pm_runtime_get_sync() return value leads to unsafe hardware access.
- [High] Race condition / use-after-PM-disable due to mixing manual PM disable with devres registration.
--

commit 800a3d2cff62e2654821d4421a37d86abb9c6de0
Author: Felix Gu <ustc.gu@gmail.com>

    iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls
    
    Neither the error path in sun4i_gpadc_probe() nor sun4i_gpadc_remove()
    ever holds a runtime PM usage count. So the pm_runtime_put() in both
    places always triggers the "Runtime PM usage count underflow!" warning
    on every failed probe and every unbind.
    
    Drop both calls.

> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 203459ca99079..85382e435f2a7 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
[ ... ]
> @@ -668,7 +667,6 @@ static void sun4i_gpadc_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>  
> -	pm_runtime_put(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);

[Severity: High]
This isn't a bug introduced by this patch, but does calling
pm_runtime_disable() here create a race condition?

The IIO device and thermal zone are registered using devres functions
(devm_iio_device_register / devm_thermal_of_zone_register) in
sun4i_gpadc_probe(), which means they are unregistered after
sun4i_gpadc_remove() completes.

Since pm_runtime_disable() is called before the devres-managed IIO device and
thermal zone are unregistered, the sysfs device and thermal zone remain fully
accessible while PM transitions are disabled.

Could a concurrent background poll from the thermal core or a userspace IIO
read during driver unbind trigger a pm_runtime_get_sync() that fails with
-EACCES, leading to an unclocked hardware crash?

This is another pre-existing issue, but does sun4i_gpadc_read() have an
unbalanced IRQ disable on its error path?

drivers/iio/adc/sun4i-gpadc-iio.c:sun4i_gpadc_read() {
    ...
	ret = sun4i_prepare_for_irq(indio_dev, channel, irq);
	if (ret)
		goto err;

	enable_irq(irq);
    ...
err:
	pm_runtime_put_autosuspend(indio_dev->dev.parent);
	disable_irq(irq);
	mutex_unlock(&info->mutex);

	return ret;
}

[Severity: High]
If sun4i_prepare_for_irq() returns an error, execution skips enable_irq()
but goes to the err: label, which unconditionally executes disable_irq().
Since the IRQ was requested with IRQF_NO_AUTOEN (disabled, depth=1), the
unmatched disable increments the depth to 2.

A subsequent successful read will call enable_irq(), only decrementing the
depth to 1, leaving the IRQ permanently disabled. Does a single transient
error during a userspace IIO read permanently break the hardware interface?

This is also a pre-existing issue, but should the return value of
pm_runtime_get_sync() be checked before accessing hardware registers?

drivers/iio/adc/sun4i-gpadc-iio.c:sun4i_prepare_for_irq() {
    ...
	pm_runtime_get_sync(indio_dev->dev.parent);

	reinit_completion(&info->completion);

	ret = regmap_write(info->regmap, SUN4I_GPADC_INT_FIFOC,
    ...
}

And in drivers/iio/adc/sun4i-gpadc-iio.c:sun4i_gpadc_temp_read():

	if (info->no_irq) {
		pm_runtime_get_sync(indio_dev->dev.parent);

		regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
    ...
}

[Severity: High]
pm_runtime_get_sync() can return a negative error code if the device fails to
resume or if runtime PM has been disabled. The return value is ignored before
issuing regmap_read() or regmap_write() calls.

If PM transitions fail or runtime PM is disabled, could this lead to accessing
unclocked or powered-off hardware, potentially resulting in system hangs or
bus faults?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-gpadc-v1-0-92d3dc8ef355@gmail.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure
  2026-09-06 15:58 ` [PATCH 2/2] iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure Felix Gu
@ 2026-09-06 16:08   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-06 16:08 UTC (permalink / raw)
  To: Felix Gu; +Cc: linux-sunxi

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

Pre-existing issues:
- [High] pm_runtime_disable() is called manually before devm-managed subsystems are unregistered, exposing a window where callbacks run with runtime PM disabled.
- [High] The driver ignores the return value of pm_runtime_get_sync(), which can lead to accessing hardware registers while the device is suspended or powered off.
--

commit 0cc8a4f7ab93d097709950676951a80ac9c05108
Author: Felix Gu <ustc.gu@gmail.com>

    iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure
    
    If devm_thermal_of_zone_register() fails, probe returns without
    unregistering the IIO map array or disabling runtime PM.
    
    Jump to err_map to release them.

> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 85382e435f2a7..ac5ba76b979e6 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -638,10 +638,10 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
>  		 * thermal because no thermal DT node is found.
>  		 */
>  		if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
> -			dev_err(&pdev->dev,
> -				"could not register thermal sensor: %ld\n",
> -				PTR_ERR(info->tzd));
> -			return PTR_ERR(info->tzd);
> +			ret = dev_err_probe(&pdev->dev,
> +					    PTR_ERR(info->tzd),
> +					    "could not register thermal sensor\n");
> +			goto err_map;
>  		}
>  	}
>  
>  	ret = devm_iio_device_register(&pdev->dev, indio_dev);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "could not register the device\n");
>  		goto err_map;
>  	}
>  
>  	return 0;
>  
>  err_map:
>  	if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
>  		iio_map_array_unregister(indio_dev);
>  
>  	pm_runtime_disable(&pdev->dev);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This is a pre-existing issue, but does explicitly calling pm_runtime_disable()
here create a window where the devres-managed thermal zone and IIO devices are
still registered while runtime PM is disabled?

Since devm_thermal_of_zone_register() and devm_iio_device_register() are used,
devres will not unregister them until after the probe error path returns.

In the interim, if the thermal core's polling thread invokes .get_temp() or
userspace reads IIO sysfs, could this access hardware while clocks are
disabled, triggering a synchronous external abort?

A similar pre-existing issue appears in sun4i_gpadc_remove():

drivers/iio/adc/sun4i-gpadc-iio.c:sun4i_gpadc_remove() {
	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
	struct sun4i_gpadc_iio *info = iio_priv(indio_dev);

	pm_runtime_disable(&pdev->dev);
...
}

Is pm_runtime_disable() being called here before devres tears down the
registered IIO and thermal zones?

There is another pre-existing issue where return values for
pm_runtime_get_sync() are not checked before accessing hardware. Can this lead
to accessing hardware registers while the device is suspended or powered off?

If pm_runtime_get_sync() fails (e.g., due to runtime PM being disabled during
the teardown race from err_map or remove, or a genuine PM failure), it returns
a negative error code but regmap accesses proceed anyway:

drivers/iio/adc/sun4i-gpadc-iio.c:sun4i_prepare_for_irq() {
	...
	pm_runtime_get_sync(indio_dev->dev.parent);

	reinit_completion(&info->completion);

	ret = regmap_write(info->regmap, SUN4I_GPADC_INT_FIFOC,
	...
}

And also in sun4i_gpadc_temp_read():

drivers/iio/adc/sun4i-gpadc-iio.c:sun4i_gpadc_temp_read() {
	struct sun4i_gpadc_iio *info = iio_priv(indio_dev);

	if (info->no_irq) {
		pm_runtime_get_sync(indio_dev->dev.parent);

		regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
	...
}

Should the return value of pm_runtime_get_sync() be checked to avoid a bus
fault or kernel panic when reading or writing to unpowered MMIO registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-gpadc-v1-0-92d3dc8ef355@gmail.com?part=2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes
  2026-09-06 15:58 [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes Felix Gu
  2026-09-06 15:58 ` [PATCH 1/2] iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls Felix Gu
  2026-09-06 15:58 ` [PATCH 2/2] iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure Felix Gu
@ 2026-09-06 17:36 ` Jonathan Cameron
  2 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2026-09-06 17:36 UTC (permalink / raw)
  To: Felix Gu
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Quentin Schulz, Maxime Ripard,
	linux-iio, linux-arm-kernel, linux-sunxi, linux-kernel

On Sun, 06 Sep 2026 23:58:33 +0800
Felix Gu <ustc.gu@gmail.com> wrote:

Some text here would be normal even if it doesn't add much!

Anyhow, both fall in the obviously correct category so
I've queued them up on the fixes-togreg branch of iio.git
and marked them for stable.

Thanks,

Jonathan

> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
> Felix Gu (2):
>       iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls
>       iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure
> 
>  drivers/iio/adc/sun4i-gpadc-iio.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> ---
> base-commit: 9d80aa4617b32f5054c5aa471d06b66704854935
> change-id: 20260906-gpadc-d41b964d8f17
> 
> Best regards,
> --  
> Felix Gu <ustc.gu@gmail.com>
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-06 17:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-06 15:58 [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes Felix Gu
2026-09-06 15:58 ` [PATCH 1/2] iio: adc: sun4i-gpadc-iio: drop underflowing pm_runtime_put() calls Felix Gu
2026-09-06 16:08   ` sashiko-bot
2026-09-06 15:58 ` [PATCH 2/2] iio: adc: sun4i-gpadc-iio: clean up on thermal zone registration failure Felix Gu
2026-09-06 16:08   ` sashiko-bot
2026-09-06 17:36 ` [PATCH 0/2] iio: adc: sun4i-gpadc-iio: two fixes Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.