| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <iz3e2ojepgjo4c36ibl5sfpw2oidkj7sao6o2jint4midgsynm@adhsfkukk2ba>
Date: Tue, 23 Dec 2025 21:23:40 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Gaurav Kohli <gaurav.kohli@....qualcomm.com>
Cc: andersson@...nel.org, mathieu.poirier@...aro.org, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, rafael@...nel.org,
daniel.lezcano@...aro.org, rui.zhang@...el.com, lukasz.luba@....com,
konradybcio@...nel.org, amitk@...nel.org, mani@...nel.org,
casey.connolly@...aro.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org,
Amit Kucheria <amit.kucheria@....qualcomm.com>
Subject: Re: [PATCH v1 1/8] thermal: Add Remote Proc cooling driver
On Tue, Dec 23, 2025 at 06:02:20PM +0530, Gaurav Kohli wrote:
> Add a new generic driver for thermal cooling devices that control
> remote processors (modem, DSP, etc.) through various communication
> channels.
>
> This driver provides an abstraction layer between the thermal
> subsystem and vendor-specific remote processor communication
> mechanisms.
This driver simply wraps 3 callbacks. Please explain in the commit
message, why do we need it? Why can't the consumer of this API simply
provide those callbacks directly?
>
> Suggested-by: Amit Kucheria <amit.kucheria@....qualcomm.com>
> Signed-off-by: Gaurav Kohli <gaurav.kohli@....qualcomm.com>
> ---
> MAINTAINERS | 8 ++
> drivers/thermal/Kconfig | 11 ++
> drivers/thermal/Makefile | 2 +
> drivers/thermal/remoteproc_cooling.c | 154 +++++++++++++++++++++++++++
> include/linux/remoteproc_cooling.h | 52 +++++++++
> 5 files changed, 227 insertions(+)
> create mode 100644 drivers/thermal/remoteproc_cooling.c
> create mode 100644 include/linux/remoteproc_cooling.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 679e5f11e672..c1ba87315cdf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25935,6 +25935,14 @@ F: drivers/thermal/cpufreq_cooling.c
> F: drivers/thermal/cpuidle_cooling.c
> F: include/linux/cpu_cooling.h
>
> +THERMAL/REMOTEPROC_COOLING
> +M: Gaurav Kohli <gaurav.kohli@....qualcomm.com>
> +L: linux-pm@...r.kernel.org
> +S: Supported
> +F: drivers/thermal/remoteproc_cooling.c
> +F: include/linux/remoteproc_cooling.h
> +
> +
> THERMAL/POWER_ALLOCATOR
> M: Lukasz Luba <lukasz.luba@....com>
> L: linux-pm@...r.kernel.org
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index b10080d61860..31e92be34387 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -229,6 +229,17 @@ config PCIE_THERMAL
>
> If you want this support, you should say Y here.
>
> +
> +config REMOTEPROC_THERMAL
> + bool "Remote processor cooling support"
Why this is 'bool' rather than 'tristate'?
> + help
> + This implements a generic cooling mechanism for remote processors
> + (modem, DSP, etc.) that allows vendor-specific implementations to
> + register thermal cooling devices and provide callbacks for thermal
> + mitigation.
> +
> + If you want this support, you should say Y here.
> +
> config THERMAL_EMULATION
> bool "Thermal emulation mode support"
> help
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index bb21e7ea7fc6..ae747dde54fe 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -34,6 +34,8 @@ thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>
> thermal_sys-$(CONFIG_PCIE_THERMAL) += pcie_cooling.o
>
> +thermal_sys-$(CONFIG_REMOTEPROC_THERMAL) += remoteproc_cooling.o
> +
> obj-$(CONFIG_K3_THERMAL) += k3_bandgap.o k3_j72xx_bandgap.o
> # platform thermal drivers
> obj-y += broadcom/
> diff --git a/drivers/thermal/remoteproc_cooling.c b/drivers/thermal/remoteproc_cooling.c
> new file mode 100644
> index 000000000000..a1f948cbde0f
> --- /dev/null
> +++ b/drivers/thermal/remoteproc_cooling.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Remote Processor Cooling Device
> + *
> + * Copyright (c) 2025, Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/thermal.h>
> +
> +#define REMOTEPROC_PREFIX "rproc_"
> +
> +struct remoteproc_cooling_ops {
> + int (*get_max_level)(void *devdata, unsigned long *level);
> + int (*get_cur_level)(void *devdata, unsigned long *level);
> + int (*set_cur_level)(void *devdata, unsigned long level);
> +};
> +
> +/**
> + * struct remoteproc_cdev - Remote processor cooling device
> + * @cdev: Thermal cooling device handle
> + * @ops: Vendor-specific operation callbacks
> + * @devdata: Private data for vendor implementation
> + * @np: Device tree node associated with this cooling device
> + * @lock: Mutex to protect cooling device operations
> + */
> +struct remoteproc_cdev {
> + struct thermal_cooling_device *cdev;
> + const struct remoteproc_cooling_ops *ops;
> + void *devdata;
> + struct device_node *np;
> + struct mutex lock;
> +};
> +
> +
> +/* Thermal cooling device callbacks */
> +
> +static int remoteproc_get_max_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + struct remoteproc_cdev *rproc_cdev = cdev->devdata;
> + int ret;
> +
> + if (!rproc_cdev || !rproc_cdev->ops)
> + return -EINVAL;
> +
> + mutex_lock(&rproc_cdev->lock);
> + ret = rproc_cdev->ops->get_max_level(rproc_cdev->devdata, state);
> + mutex_unlock(&rproc_cdev->lock);
> +
> + return ret;
> +}
> +
> +static int remoteproc_get_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + struct remoteproc_cdev *rproc_cdev = cdev->devdata;
> + int ret;
> +
> + if (!rproc_cdev || !rproc_cdev->ops)
> + return -EINVAL;
> +
> + mutex_lock(&rproc_cdev->lock);
> + ret = rproc_cdev->ops->get_cur_level(rproc_cdev->devdata, state);
> + mutex_unlock(&rproc_cdev->lock);
> +
> + return ret;
> +}
> +
> +static int remoteproc_set_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long state)
> +{
> + struct remoteproc_cdev *rproc_cdev = cdev->devdata;
> + int ret;
> +
> + if (!rproc_cdev || !rproc_cdev->ops)
> + return -EINVAL;
> +
> + mutex_lock(&rproc_cdev->lock);
> + ret = rproc_cdev->ops->set_cur_level(rproc_cdev->devdata, state);
> + mutex_unlock(&rproc_cdev->lock);
> +
> + return ret;
> +}
> +
> +static const struct thermal_cooling_device_ops remoteproc_cooling_ops = {
> + .get_max_state = remoteproc_get_max_state,
> + .get_cur_state = remoteproc_get_cur_state,
> + .set_cur_state = remoteproc_set_cur_state,
> +};
> +
> +struct remoteproc_cdev *
> +remoteproc_cooling_register(struct device_node *np,
> + const char *name, const struct remoteproc_cooling_ops *ops,
> + void *devdata)
> +{
> + struct remoteproc_cdev *rproc_cdev;
> + struct thermal_cooling_device *cdev;
> + int ret;
> +
> + if (!name || !ops) {
> + return ERR_PTR(-EINVAL);
> + }
> +
> + rproc_cdev = kzalloc(sizeof(*rproc_cdev), GFP_KERNEL);
> + if (!rproc_cdev)
> + return ERR_PTR(-ENOMEM);
> +
> + rproc_cdev->ops = ops;
> + rproc_cdev->devdata = devdata;
> + rproc_cdev->np = np;
> + mutex_init(&rproc_cdev->lock);
> +
> + char *rproc_name __free(kfree) =
> + kasprintf(GFP_KERNEL, REMOTEPROC_PREFIX "%s", name);
> + /* Register with thermal framework */
> + if (np) {
> + cdev = thermal_of_cooling_device_register(np, rproc_name, rproc_cdev,
> + &remoteproc_cooling_ops);
> + }
> +
> + if (IS_ERR(cdev)) {
> + ret = PTR_ERR(cdev);
> + goto free_rproc_cdev;
> + }
So, if np == NULL, we register nothing, but still return 0 (aka no
error). Why?
> +
> + rproc_cdev->cdev = cdev;
> +
> + return rproc_cdev;
> +
> +free_rproc_cdev:
> + kfree(rproc_cdev);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(remoteproc_cooling_register);
> +
> +void remoteproc_cooling_unregister(struct remoteproc_cdev *rproc_cdev)
> +{
> + if (!rproc_cdev)
> + return;
> +
> + thermal_cooling_device_unregister(rproc_cdev->cdev);
> + mutex_destroy(&rproc_cdev->lock);
> + kfree(rproc_cdev);
> +}
> +EXPORT_SYMBOL_GPL(remoteproc_cooling_unregister);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Remote Processor Cooling Device");
> diff --git a/include/linux/remoteproc_cooling.h b/include/linux/remoteproc_cooling.h
> new file mode 100644
> index 000000000000..ef94019d220d
> --- /dev/null
> +++ b/include/linux/remoteproc_cooling.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Remote Processor Cooling Device
> + *
> + * Copyright (c) 2025, Qualcomm Innovation Center
> + */
> +
> +#ifndef __REMOTEPROC_COOLING_H__
> +#define __REMOTEPROC_COOLING_H__
> +
> +#include <linux/thermal.h>
> +
> +struct device;
> +struct device_node;
> +
> +struct remoteproc_cooling_ops {
> + int (*get_max_level)(void *devdata, unsigned long *level);
> + int (*get_cur_level)(void *devdata, unsigned long *level);
> + int (*set_cur_level)(void *devdata, unsigned long level);
> +};
> +
> +struct remoteproc_cdev;
> +
> +#ifdef CONFIG_REMOTEPROC_THERMAL
> +
> +struct remoteproc_cdev *
> +remoteproc_cooling_register(struct device_node *np,
> + const char *name,
> + const struct remoteproc_cooling_ops *ops,
> + void *devdata);
> +
> +void remoteproc_cooling_unregister(struct remoteproc_cdev *rproc_cdev);
> +
> +#else /* !CONFIG_REMOTEPROC_THERMAL */
> +
> +static inline struct remoteproc_cdev *
> +remoteproc_cooling_register(struct device_node *np,
> + const char *name,
> + const struct remoteproc_cooling_ops *ops,
> + void *devdata)
> +{
> + return ERR_PTR(-EINVAL);
> +}
> +
> +static inline void
> +remoteproc_cooling_unregister(struct remoteproc_cdev *rproc_cdev)
> +{
> +}
> +
> +#endif /* CONFIG_REMOTEPROC_THERMAL */
> +
> +#endif /* __REMOTEPROC_COOLING_H__ */
> --
> 2.34.1
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists