Re: [PATCH v1 1/8] thermal: Add Remote Proc cooling driver

[Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>]

On 1/8/2026 5:29 PM, Zhongqiu Han wrote:
> On 12/23/2025 8:32 PM, 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.
>>
>> Suggested-by: Amit Kucheria <amit.kucheria@oss.qualcomm.com>
>> Signed-off-by: Gaurav Kohli <gaurav.kohli@oss.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@oss.qualcomm.com>
>> +L:    linux-pm@vger.kernel.org
>> +S:    Supported
>> +F:    drivers/thermal/remoteproc_cooling.c
>> +F:    include/linux/remoteproc_cooling.h
>> +
>> +
>>   THERMAL/POWER_ALLOCATOR
>>   M:    Lukasz Luba <lukasz.luba@arm.com>
>>   L:    linux-pm@vger.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"
>
> Hi Gaurav,
>
> May I know any depends here?


Thanks for review.

Apologies for the late reply,  this was missed on my end.

This does not require a dependency, as error check is there for cooling 
registration api.


>
>> +    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);
>> +};
> It's better to have a document to explain member?
>
> And may I know why double define here(another is in .h file)? should it
> include .h file remoteproc_cooling.h ?
>

Sure, i will add comment for each callback and will fix this double 
definition.


>
>> +
>> +/**
>> + * 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);
>> +    }
>> +
>
> May I know which ops callbacks are required and which are optional?
> If the callback is optional, should we check for null before calling it?
>

Actually all are mandatory for cooling api, will put null check for all 
to be on safer side.


>
>> +    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);
>
> It should have a NULL check when alloc memory.
>
>

Thanks for pointing this, will update this.


>> +    /* 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;
>> +    }
>> +
>> +    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__ */
>
>