| 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: <611b87db-9226-4c99-a07d-03351dd10546@oss.qualcomm.com>
Date: Wed, 24 Dec 2025 13:50:00 +0530
From: Gaurav Kohli <gaurav.kohli@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....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 12/24/2025 12:53 AM, Dmitry Baryshkov wrote:
> 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?
thanks for review, sure i will update in next post.
>> 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'?
tristate will be fine, will update this.
>
>> + 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
>>
Powered by blists - more mailing lists