| 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: <20260128-quick-maroon-dragon-d832c8@quoll>
Date: Wed, 28 Jan 2026 12:32:27 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
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, rui.zhang@...el.com, lukasz.luba@....com,
konradybcio@...nel.org, mani@...nel.org, casey.connolly@...aro.org,
amit.kucheria@....qualcomm.com, linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, manaf.pallikunhi@....qualcomm.com
Subject: Re: [PATCH v2 1/8] thermal: Add Remote Proc cooling driver
On Tue, Jan 27, 2026 at 09:27:15PM +0530, Gaurav Kohli wrote:
> Add a new generic driver for thermal cooling devices that control
There is no driver here. You did not a single driver entry point.
> remote processors (modem, DSP, etc.) through various communication
> channels.
>
> This driver provides an abstraction layer between the thermal
Please read coding style how much we like abstraction layers.
> subsystem and vendor-specific remote processor communication
> mechanisms.
>
> Advantage of this to avoid duplicating vendor-specific logic
> in the thermal subsystem and make it easier for different vendors
> to plug in their own cooling mechanisms via callbacks.
>
> Suggested-by: Amit Kucheria <amit.kucheria@....qualcomm.com>
> Signed-off-by: Gaurav Kohli <gaurav.kohli@....qualcomm.com>
> ---
> MAINTAINERS | 7 ++
> drivers/thermal/Kconfig | 10 ++
> drivers/thermal/Makefile | 2 +
> drivers/thermal/remoteproc_cooling.c | 143 +++++++++++++++++++++++++++
> include/linux/remoteproc_cooling.h | 52 ++++++++++
> 5 files changed, 214 insertions(+)
> create mode 100644 drivers/thermal/remoteproc_cooling.c
> create mode 100644 include/linux/remoteproc_cooling.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 414f44093269..5ebc7819d2cf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26169,6 +26169,13 @@ 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
Please beginning of this file. P < R.
> M: Lukasz Luba <lukasz.luba@....com>
> L: linux-pm@...r.kernel.org
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index b10080d61860..dfc52eed64de 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -229,6 +229,16 @@ config PCIE_THERMAL
>
> If you want this support, you should say Y here.
>
> +config REMOTEPROC_THERMAL
> + tristate "Remote processor cooling support"
> + 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..f958efa691b3
> --- /dev/null
> +++ b/drivers/thermal/remoteproc_cooling.c
> @@ -0,0 +1,143 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Remote Processor Cooling Device
> + *
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
Where do you use it?
> +#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 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;
> +
> + 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;
> +
> + 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;
> +
> + 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;
> + 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);
> + else
> + cdev = thermal_cooling_device_register(rproc_name, rproc_cdev,
> + &remoteproc_cooling_ops);
> +
> + if (IS_ERR(cdev)) {
> + ret = PTR_ERR(cdev);
> + goto free_rproc_cdev;
> + }
> +
> + 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");
I do not see any driver here, just bunch of exported functions. I do not
see point in this abstraction/wrapping layer.
Another abstraction layer, NAK.
Best regards,
Krzysztof
Powered by blists - more mailing lists