Re: [PATCH 3/6] iio: adc: versal-sysmon: disable interrupts on shutdown

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 2 Nov 2026 17:51:09 +0530
Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com> wrote:

> Interrupt teardown is registered with devm_add_action_or_reset(), so
> sysmon_disable_interrupts() runs when the driver is unbound but not when
> the system shuts down: device_shutdown() invokes only the driver's
> .shutdown callback, which this driver does not implement.
> 
> The device is therefore left with its alarm interrupts enabled in
> hardware once Linux has stopped servicing them.
> 
> Add a .shutdown callback performing the same teardown, so the device is
> quiesced before control is handed on. The helper lives in the core
> module, so it is exported for the platform front-end. It skips a
> device described without an interrupt, which never set up interrupt
> handling.
> 
> Disabling the interrupts at shutdown is safe to have and not a must have
> requirement for shutdown.

So what is disadvantage of not doing it?  Otherwise, why have the complexity
of doing so?

J
> 
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> ---
>  drivers/iio/adc/versal-sysmon-core.c | 19 +++++++++++++++++++
>  drivers/iio/adc/versal-sysmon.c      |  6 ++++++
>  drivers/iio/adc/versal-sysmon.h      |  1 +
>  3 files changed, 26 insertions(+)
> 
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index 6595d07038c1..d39aaba9a8b6 100644
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
> @@ -1000,6 +1000,8 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
> +	dev_set_drvdata(dev, indio_dev);
> +
>  	sysmon = iio_priv(indio_dev);
>  	sysmon->regmap = regmap;
>  	sysmon->temp_oversampling = 1;
> @@ -1050,6 +1052,23 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_versal_sysmon_core_probe, "VERSAL_SYSMON");
>  
> +/**
> + * versal_sysmon_shutdown() - Disable SysMon interrupts on system shutdown
> + * @dev: SysMon device
> + *
> + * Interrupt teardown is a devm action, which runs on driver unbind but not
> + * on system shutdown, so disable the interrupts here as well.
> + */
> +void versal_sysmon_shutdown(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct sysmon *sysmon = iio_priv(indio_dev);
> +
> +	if (sysmon->irq > 0)
> +		sysmon_disable_interrupts(sysmon);
> +}
> +EXPORT_SYMBOL_NS_GPL(versal_sysmon_shutdown, "VERSAL_SYSMON");
> +
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("AMD Versal SysMon Core Driver");
>  MODULE_AUTHOR("Salih Erim <salih.erim@amd.com>");
> diff --git a/drivers/iio/adc/versal-sysmon.c b/drivers/iio/adc/versal-sysmon.c
> index 529d0486c9f9..4d5e3d14cf15 100644
> --- a/drivers/iio/adc/versal-sysmon.c
> +++ b/drivers/iio/adc/versal-sysmon.c
> @@ -71,6 +71,11 @@ static int sysmon_platform_probe(struct platform_device *pdev)
>  	return devm_versal_sysmon_core_probe(dev, regmap);
>  }
>  
> +static void sysmon_platform_shutdown(struct platform_device *pdev)
> +{
> +	versal_sysmon_shutdown(&pdev->dev);
> +}
> +
>  static const struct of_device_id sysmon_of_match_table[] = {
>  	{ .compatible = "xlnx,versal-sysmon" },
>  	{ }
> @@ -79,6 +84,7 @@ MODULE_DEVICE_TABLE(of, sysmon_of_match_table);
>  
>  static struct platform_driver sysmon_platform_driver = {
>  	.probe = sysmon_platform_probe,
> +	.shutdown = sysmon_platform_shutdown,
>  	.driver = {
>  		.name = "versal-sysmon",
>  		.of_match_table = sysmon_of_match_table,
> diff --git a/drivers/iio/adc/versal-sysmon.h b/drivers/iio/adc/versal-sysmon.h
> index a1ac3289719c..a21dacc13471 100644
> --- a/drivers/iio/adc/versal-sysmon.h
> +++ b/drivers/iio/adc/versal-sysmon.h
> @@ -122,5 +122,6 @@ struct sysmon {
>  };
>  
>  int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap);
> +void versal_sysmon_shutdown(struct device *dev);
>  
>  #endif /* _VERSAL_SYSMON_H_ */