[v1] Add RemoteProc cooling support

[PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Gaurav Kohli 1 month, 2 weeks ago

From: Casey Connolly <casey.connolly@linaro.org>

Generalise the qcom,bam-dmux child node support by probing all
remoteproc children with of_platform_populate(). This will be used to
enable support for devices which are best represented as subnodes of the
remoteproc, such as those representing QMI clients.

Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
---
 drivers/remoteproc/qcom_q6v5.c     | 4 ++++
 drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
index 58d5b85e58cd..a02839c7ed8c 100644
--- a/drivers/remoteproc/qcom_q6v5.c
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2014 Sony Mobile Communications AB
  * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
  */
+#include <linux/of_platform.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/interconnect.h>
@@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
 		return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
 				     "failed to acquire interconnect path\n");
 
+	of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qcom_q6v5_init);
@@ -361,6 +364,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_init);
  */
 void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5)
 {
+	of_platform_depopulate(q6v5->dev);
 	qmp_put(q6v5->qmp);
 }
 EXPORT_SYMBOL_GPL(qcom_q6v5_deinit);
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 91940977ca89..d40565c1cc62 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -238,7 +238,6 @@ struct q6v5 {
 	struct qcom_rproc_pdm pdm_subdev;
 	struct qcom_rproc_ssr ssr_subdev;
 	struct qcom_sysmon *sysmon;
-	struct platform_device *bam_dmux;
 	bool need_mem_protection;
 	bool has_alt_reset;
 	bool has_mba_logs;
@@ -2029,7 +2028,6 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
 static int q6v5_probe(struct platform_device *pdev)
 {
 	const struct rproc_hexagon_res *desc;
-	struct device_node *node;
 	struct q6v5 *qproc;
 	struct rproc *rproc;
 	const char *mba_image;
@@ -2163,10 +2161,6 @@ static int q6v5_probe(struct platform_device *pdev)
 	if (ret)
 		goto remove_sysmon_subdev;
 
-	node = of_get_compatible_child(pdev->dev.of_node, "qcom,bam-dmux");
-	qproc->bam_dmux = of_platform_device_create(node, NULL, &pdev->dev);
-	of_node_put(node);
-
 	return 0;
 
 remove_sysmon_subdev:
@@ -2186,8 +2180,6 @@ static void q6v5_remove(struct platform_device *pdev)
 	struct q6v5 *qproc = platform_get_drvdata(pdev);
 	struct rproc *rproc = qproc->rproc;
 
-	if (qproc->bam_dmux)
-		of_platform_device_destroy(&qproc->bam_dmux->dev, NULL);
 	rproc_del(rproc);
 
 	qcom_q6v5_deinit(&qproc->q6v5);
-- 
2.34.1

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Bjorn Andersson 1 month, 1 week ago

On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
> From: Casey Connolly <casey.connolly@linaro.org>
> 
> Generalise the qcom,bam-dmux child node support by probing all
> remoteproc children with of_platform_populate(). This will be used to
> enable support for devices which are best represented as subnodes of the
> remoteproc, such as those representing QMI clients.

Please flip this around, start with the description of the problem
you're trying to solve.

> 
> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>

This must have your signed-off-by, where you certifies the origin of
this patch.

> ---
>  drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>  drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> index 58d5b85e58cd..a02839c7ed8c 100644
> --- a/drivers/remoteproc/qcom_q6v5.c
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -6,6 +6,7 @@
>   * Copyright (C) 2014 Sony Mobile Communications AB
>   * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
>   */
> +#include <linux/of_platform.h>
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  #include <linux/interconnect.h>
> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>  		return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>  				     "failed to acquire interconnect path\n");
>  
> +	of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);

There are other child nodes here, in particular the GLINK and SMD edges.
Do we really want platform_devices registered for them?

Regards,
Bjorn

> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(qcom_q6v5_init);
> @@ -361,6 +364,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_init);
>   */
>  void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5)
>  {
> +	of_platform_depopulate(q6v5->dev);
>  	qmp_put(q6v5->qmp);
>  }
>  EXPORT_SYMBOL_GPL(qcom_q6v5_deinit);
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 91940977ca89..d40565c1cc62 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -238,7 +238,6 @@ struct q6v5 {
>  	struct qcom_rproc_pdm pdm_subdev;
>  	struct qcom_rproc_ssr ssr_subdev;
>  	struct qcom_sysmon *sysmon;
> -	struct platform_device *bam_dmux;
>  	bool need_mem_protection;
>  	bool has_alt_reset;
>  	bool has_mba_logs;
> @@ -2029,7 +2028,6 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
>  static int q6v5_probe(struct platform_device *pdev)
>  {
>  	const struct rproc_hexagon_res *desc;
> -	struct device_node *node;
>  	struct q6v5 *qproc;
>  	struct rproc *rproc;
>  	const char *mba_image;
> @@ -2163,10 +2161,6 @@ static int q6v5_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto remove_sysmon_subdev;
>  
> -	node = of_get_compatible_child(pdev->dev.of_node, "qcom,bam-dmux");
> -	qproc->bam_dmux = of_platform_device_create(node, NULL, &pdev->dev);
> -	of_node_put(node);
> -
>  	return 0;
>  
>  remove_sysmon_subdev:
> @@ -2186,8 +2180,6 @@ static void q6v5_remove(struct platform_device *pdev)
>  	struct q6v5 *qproc = platform_get_drvdata(pdev);
>  	struct rproc *rproc = qproc->rproc;
>  
> -	if (qproc->bam_dmux)
> -		of_platform_device_destroy(&qproc->bam_dmux->dev, NULL);
>  	rproc_del(rproc);
>  
>  	qcom_q6v5_deinit(&qproc->q6v5);
> -- 
> 2.34.1
>

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Gaurav Kohli 1 month ago

On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>> From: Casey Connolly <casey.connolly@linaro.org>
>>
>> Generalise the qcom,bam-dmux child node support by probing all
>> remoteproc children with of_platform_populate(). This will be used to
>> enable support for devices which are best represented as subnodes of the
>> remoteproc, such as those representing QMI clients.
> Please flip this around, start with the description of the problem
> you're trying to solve.
>
>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> This must have your signed-off-by, where you certifies the origin of
> this patch.
>
>> ---
>>   drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>   drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>   2 files changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
>> index 58d5b85e58cd..a02839c7ed8c 100644
>> --- a/drivers/remoteproc/qcom_q6v5.c
>> +++ b/drivers/remoteproc/qcom_q6v5.c
>> @@ -6,6 +6,7 @@
>>    * Copyright (C) 2014 Sony Mobile Communications AB
>>    * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
>>    */
>> +#include <linux/of_platform.h>
>>   #include <linux/kernel.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/interconnect.h>
>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>>   		return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>   				     "failed to acquire interconnect path\n");
>>   
>> +	of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
> There are other child nodes here, in particular the GLINK and SMD edges.
> Do we really want platform_devices registered for them?
>
> Regards,
> Bjorn


thanks for pointing this, can you please suggest the right approach.

This should not impact glink, as that is registering as rproc sub node, 
And we need rproc cooling as child node

of remote proc subsytem to create probe dependency only.


Can we do platform populate for specific child, would that be right 
approach. or we should create rproc cooling as independent of parent ?


>> +
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(qcom_q6v5_init);
>> @@ -361,6 +364,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_init);
>>    */
>>   void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5)
>>   {
>> +	of_platform_depopulate(q6v5->dev);
>>   	qmp_put(q6v5->qmp);
>>   }
>>   EXPORT_SYMBOL_GPL(qcom_q6v5_deinit);
>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
>> index 91940977ca89..d40565c1cc62 100644
>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>> @@ -238,7 +238,6 @@ struct q6v5 {
>>   	struct qcom_rproc_pdm pdm_subdev;
>>   	struct qcom_rproc_ssr ssr_subdev;
>>   	struct qcom_sysmon *sysmon;
>> -	struct platform_device *bam_dmux;
>>   	bool need_mem_protection;
>>   	bool has_alt_reset;
>>   	bool has_mba_logs;
>> @@ -2029,7 +2028,6 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
>>   static int q6v5_probe(struct platform_device *pdev)
>>   {
>>   	const struct rproc_hexagon_res *desc;
>> -	struct device_node *node;
>>   	struct q6v5 *qproc;
>>   	struct rproc *rproc;
>>   	const char *mba_image;
>> @@ -2163,10 +2161,6 @@ static int q6v5_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		goto remove_sysmon_subdev;
>>   
>> -	node = of_get_compatible_child(pdev->dev.of_node, "qcom,bam-dmux");
>> -	qproc->bam_dmux = of_platform_device_create(node, NULL, &pdev->dev);
>> -	of_node_put(node);
>> -
>>   	return 0;
>>   
>>   remove_sysmon_subdev:
>> @@ -2186,8 +2180,6 @@ static void q6v5_remove(struct platform_device *pdev)
>>   	struct q6v5 *qproc = platform_get_drvdata(pdev);
>>   	struct rproc *rproc = qproc->rproc;
>>   
>> -	if (qproc->bam_dmux)
>> -		of_platform_device_destroy(&qproc->bam_dmux->dev, NULL);
>>   	rproc_del(rproc);
>>   
>>   	qcom_q6v5_deinit(&qproc->q6v5);
>> -- 
>> 2.34.1
>>

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Gaurav Kohli 2 weeks, 3 days ago

On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
>
> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>
>>> Generalise the qcom,bam-dmux child node support by probing all
>>> remoteproc children with of_platform_populate(). This will be used to
>>> enable support for devices which are best represented as subnodes of 
>>> the
>>> remoteproc, such as those representing QMI clients.
>> Please flip this around, start with the description of the problem
>> you're trying to solve.
>>
>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>> This must have your signed-off-by, where you certifies the origin of
>> this patch.
>>
>>> ---
>>>   drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>>   drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>>   2 files changed, 4 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/remoteproc/qcom_q6v5.c 
>>> b/drivers/remoteproc/qcom_q6v5.c
>>> index 58d5b85e58cd..a02839c7ed8c 100644
>>> --- a/drivers/remoteproc/qcom_q6v5.c
>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>> @@ -6,6 +6,7 @@
>>>    * Copyright (C) 2014 Sony Mobile Communications AB
>>>    * Copyright (c) 2012-2013, The Linux Foundation. All rights 
>>> reserved.
>>>    */
>>> +#include <linux/of_platform.h>
>>>   #include <linux/kernel.h>
>>>   #include <linux/platform_device.h>
>>>   #include <linux/interconnect.h>
>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, 
>>> struct platform_device *pdev,
>>>           return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>>                        "failed to acquire interconnect path\n");
>>>   +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
>> There are other child nodes here, in particular the GLINK and SMD edges.
>> Do we really want platform_devices registered for them?
>>
>> Regards,
>> Bjorn
>
>
> thanks for pointing this, can you please suggest the right approach.
>
> This should not impact glink, as that is registering as rproc sub 
> node, And we need rproc cooling as child node
>
> of remote proc subsytem to create probe dependency only.
>
>
> Can we do platform populate for specific child, would that be right 
> approach. or we should create rproc cooling as independent of parent ?
>

HI Bjorn,

I’d like to highlight the impact and details of placement of remoteproc 
cooling dt node:


->As a child of the remote proc subsystem node:
     In this configuration, the cooling device will only be probed once 
the corresponding remote proc subsystem itself is probed.

->Outside the remote proc subsystem, may be part of soc node:
     In this setup, the cooling device will be probed independently. It 
will wait until the remoteproc subsystem is brought up
     before completing cooling registration.
     The drawback here is that if the parent remoteproc subsystem is 
disabled, the cooling device will still undergo an
     unnecessary probe, even though it cannot be registered.

>
>>> +
>>>       return 0;
>>>   }
>>>   EXPORT_SYMBOL_GPL(qcom_q6v5_init);
>>> @@ -361,6 +364,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_init);
>>>    */
>>>   void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5)
>>>   {
>>> +    of_platform_depopulate(q6v5->dev);
>>>       qmp_put(q6v5->qmp);
>>>   }
>>>   EXPORT_SYMBOL_GPL(qcom_q6v5_deinit);
>>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c 
>>> b/drivers/remoteproc/qcom_q6v5_mss.c
>>> index 91940977ca89..d40565c1cc62 100644
>>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>>> @@ -238,7 +238,6 @@ struct q6v5 {
>>>       struct qcom_rproc_pdm pdm_subdev;
>>>       struct qcom_rproc_ssr ssr_subdev;
>>>       struct qcom_sysmon *sysmon;
>>> -    struct platform_device *bam_dmux;
>>>       bool need_mem_protection;
>>>       bool has_alt_reset;
>>>       bool has_mba_logs;
>>> @@ -2029,7 +2028,6 @@ static int q6v5_alloc_memory_region(struct 
>>> q6v5 *qproc)
>>>   static int q6v5_probe(struct platform_device *pdev)
>>>   {
>>>       const struct rproc_hexagon_res *desc;
>>> -    struct device_node *node;
>>>       struct q6v5 *qproc;
>>>       struct rproc *rproc;
>>>       const char *mba_image;
>>> @@ -2163,10 +2161,6 @@ static int q6v5_probe(struct platform_device 
>>> *pdev)
>>>       if (ret)
>>>           goto remove_sysmon_subdev;
>>>   -    node = of_get_compatible_child(pdev->dev.of_node, 
>>> "qcom,bam-dmux");
>>> -    qproc->bam_dmux = of_platform_device_create(node, NULL, 
>>> &pdev->dev);
>>> -    of_node_put(node);
>>> -
>>>       return 0;
>>>     remove_sysmon_subdev:
>>> @@ -2186,8 +2180,6 @@ static void q6v5_remove(struct platform_device 
>>> *pdev)
>>>       struct q6v5 *qproc = platform_get_drvdata(pdev);
>>>       struct rproc *rproc = qproc->rproc;
>>>   -    if (qproc->bam_dmux)
>>> - of_platform_device_destroy(&qproc->bam_dmux->dev, NULL);
>>>       rproc_del(rproc);
>>>         qcom_q6v5_deinit(&qproc->q6v5);
>>> -- 
>>> 2.34.1
>>>

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Dmitry Baryshkov 2 weeks, 3 days ago

On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
> 
> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
> > 
> > On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
> > > On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
> > > > From: Casey Connolly <casey.connolly@linaro.org>
> > > > 
> > > > Generalise the qcom,bam-dmux child node support by probing all
> > > > remoteproc children with of_platform_populate(). This will be used to
> > > > enable support for devices which are best represented as
> > > > subnodes of the
> > > > remoteproc, such as those representing QMI clients.
> > > Please flip this around, start with the description of the problem
> > > you're trying to solve.
> > > 
> > > > Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> > > This must have your signed-off-by, where you certifies the origin of
> > > this patch.
> > > 
> > > > ---
> > > >   drivers/remoteproc/qcom_q6v5.c     | 4 ++++
> > > >   drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
> > > >   2 files changed, 4 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/remoteproc/qcom_q6v5.c
> > > > b/drivers/remoteproc/qcom_q6v5.c
> > > > index 58d5b85e58cd..a02839c7ed8c 100644
> > > > --- a/drivers/remoteproc/qcom_q6v5.c
> > > > +++ b/drivers/remoteproc/qcom_q6v5.c
> > > > @@ -6,6 +6,7 @@
> > > >    * Copyright (C) 2014 Sony Mobile Communications AB
> > > >    * Copyright (c) 2012-2013, The Linux Foundation. All rights
> > > > reserved.
> > > >    */
> > > > +#include <linux/of_platform.h>
> > > >   #include <linux/kernel.h>
> > > >   #include <linux/platform_device.h>
> > > >   #include <linux/interconnect.h>
> > > > @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
> > > > struct platform_device *pdev,
> > > >           return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
> > > >                        "failed to acquire interconnect path\n");
> > > >   +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
> > > There are other child nodes here, in particular the GLINK and SMD edges.
> > > Do we really want platform_devices registered for them?
> > > 
> > > Regards,
> > > Bjorn
> > 
> > 
> > thanks for pointing this, can you please suggest the right approach.
> > 
> > This should not impact glink, as that is registering as rproc sub node,
> > And we need rproc cooling as child node
> > 
> > of remote proc subsytem to create probe dependency only.
> > 
> > 
> > Can we do platform populate for specific child, would that be right
> > approach. or we should create rproc cooling as independent of parent ?
> > 
> 
> HI Bjorn,
> 
> I’d like to highlight the impact and details of placement of remoteproc
> cooling dt node:
> 
> 
> ->As a child of the remote proc subsystem node:
>     In this configuration, the cooling device will only be probed once the
> corresponding remote proc subsystem itself is probed.
> 
> ->Outside the remote proc subsystem, may be part of soc node:
>     In this setup, the cooling device will be probed independently. It will
> wait until the remoteproc subsystem is brought up
>     before completing cooling registration.
>     The drawback here is that if the parent remoteproc subsystem is
> disabled, the cooling device will still undergo an
>     unnecessary probe, even though it cannot be registered.

Bjorns question was different. It wasn't about pushing cooling device
outside of the remoteproc node. It is about not registering the devices.

Can we follow the approach outlined by qcom_add_smd_subdev() /
qcom_add_glink_subdev()?

-- 
With best wishes
Dmitry

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Gaurav Kohli 1 week, 6 days ago

On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>>>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>>>
>>>>> Generalise the qcom,bam-dmux child node support by probing all
>>>>> remoteproc children with of_platform_populate(). This will be used to
>>>>> enable support for devices which are best represented as
>>>>> subnodes of the
>>>>> remoteproc, such as those representing QMI clients.
>>>> Please flip this around, start with the description of the problem
>>>> you're trying to solve.
>>>>
>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>>> This must have your signed-off-by, where you certifies the origin of
>>>> this patch.
>>>>
>>>>> ---
>>>>>    drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>>>>    drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>>>>    2 files changed, 4 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
>>>>> b/drivers/remoteproc/qcom_q6v5.c
>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>>>> @@ -6,6 +6,7 @@
>>>>>     * Copyright (C) 2014 Sony Mobile Communications AB
>>>>>     * Copyright (c) 2012-2013, The Linux Foundation. All rights
>>>>> reserved.
>>>>>     */
>>>>> +#include <linux/of_platform.h>
>>>>>    #include <linux/kernel.h>
>>>>>    #include <linux/platform_device.h>
>>>>>    #include <linux/interconnect.h>
>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
>>>>> struct platform_device *pdev,
>>>>>            return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>>>>                         "failed to acquire interconnect path\n");
>>>>>    +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
>>>> There are other child nodes here, in particular the GLINK and SMD edges.
>>>> Do we really want platform_devices registered for them?
>>>>
>>>> Regards,
>>>> Bjorn
>>>
>>> thanks for pointing this, can you please suggest the right approach.
>>>
>>> This should not impact glink, as that is registering as rproc sub node,
>>> And we need rproc cooling as child node
>>>
>>> of remote proc subsytem to create probe dependency only.
>>>
>>>
>>> Can we do platform populate for specific child, would that be right
>>> approach. or we should create rproc cooling as independent of parent ?
>>>
>> HI Bjorn,
>>
>> I’d like to highlight the impact and details of placement of remoteproc
>> cooling dt node:
>>
>>
>> ->As a child of the remote proc subsystem node:
>>      In this configuration, the cooling device will only be probed once the
>> corresponding remote proc subsystem itself is probed.
>>
>> ->Outside the remote proc subsystem, may be part of soc node:
>>      In this setup, the cooling device will be probed independently. It will
>> wait until the remoteproc subsystem is brought up
>>      before completing cooling registration.
>>      The drawback here is that if the parent remoteproc subsystem is
>> disabled, the cooling device will still undergo an
>>      unnecessary probe, even though it cannot be registered.
> Bjorns question was different. It wasn't about pushing cooling device
> outside of the remoteproc node. It is about not registering the devices.
>
> Can we follow the approach outlined by qcom_add_smd_subdev() /
> qcom_add_glink_subdev()?


Hi Dmitry,

Thanks for the review. Since the remoteproc cooling is a QMI-based 
driver, it will receive the

subsystem up notification directly. Therefore, there’s no need to make 
it a subdev node or

tie it into the init/reset sequence of remoteproc subsytem.


>

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Dmitry Baryshkov 1 week, 6 days ago

On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
> 
> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
> > On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
> > > On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
> > > > On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
> > > > > On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
> > > > > > From: Casey Connolly <casey.connolly@linaro.org>
> > > > > > 
> > > > > > Generalise the qcom,bam-dmux child node support by probing all
> > > > > > remoteproc children with of_platform_populate(). This will be used to
> > > > > > enable support for devices which are best represented as
> > > > > > subnodes of the
> > > > > > remoteproc, such as those representing QMI clients.
> > > > > Please flip this around, start with the description of the problem
> > > > > you're trying to solve.
> > > > > 
> > > > > > Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> > > > > This must have your signed-off-by, where you certifies the origin of
> > > > > this patch.
> > > > > 
> > > > > > ---
> > > > > >    drivers/remoteproc/qcom_q6v5.c     | 4 ++++
> > > > > >    drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
> > > > > >    2 files changed, 4 insertions(+), 8 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/remoteproc/qcom_q6v5.c
> > > > > > b/drivers/remoteproc/qcom_q6v5.c
> > > > > > index 58d5b85e58cd..a02839c7ed8c 100644
> > > > > > --- a/drivers/remoteproc/qcom_q6v5.c
> > > > > > +++ b/drivers/remoteproc/qcom_q6v5.c
> > > > > > @@ -6,6 +6,7 @@
> > > > > >     * Copyright (C) 2014 Sony Mobile Communications AB
> > > > > >     * Copyright (c) 2012-2013, The Linux Foundation. All rights
> > > > > > reserved.
> > > > > >     */
> > > > > > +#include <linux/of_platform.h>
> > > > > >    #include <linux/kernel.h>
> > > > > >    #include <linux/platform_device.h>
> > > > > >    #include <linux/interconnect.h>
> > > > > > @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
> > > > > > struct platform_device *pdev,
> > > > > >            return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
> > > > > >                         "failed to acquire interconnect path\n");
> > > > > >    +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
> > > > > There are other child nodes here, in particular the GLINK and SMD edges.
> > > > > Do we really want platform_devices registered for them?
> > > > > 
> > > > > Regards,
> > > > > Bjorn
> > > > 
> > > > thanks for pointing this, can you please suggest the right approach.
> > > > 
> > > > This should not impact glink, as that is registering as rproc sub node,
> > > > And we need rproc cooling as child node
> > > > 
> > > > of remote proc subsytem to create probe dependency only.
> > > > 
> > > > 
> > > > Can we do platform populate for specific child, would that be right
> > > > approach. or we should create rproc cooling as independent of parent ?
> > > > 
> > > HI Bjorn,
> > > 
> > > I’d like to highlight the impact and details of placement of remoteproc
> > > cooling dt node:
> > > 
> > > 
> > > ->As a child of the remote proc subsystem node:
> > >      In this configuration, the cooling device will only be probed once the
> > > corresponding remote proc subsystem itself is probed.
> > > 
> > > ->Outside the remote proc subsystem, may be part of soc node:
> > >      In this setup, the cooling device will be probed independently. It will
> > > wait until the remoteproc subsystem is brought up
> > >      before completing cooling registration.
> > >      The drawback here is that if the parent remoteproc subsystem is
> > > disabled, the cooling device will still undergo an
> > >      unnecessary probe, even though it cannot be registered.
> > Bjorns question was different. It wasn't about pushing cooling device
> > outside of the remoteproc node. It is about not registering the devices.
> > 
> > Can we follow the approach outlined by qcom_add_smd_subdev() /
> > qcom_add_glink_subdev()?
> 
> 
> Hi Dmitry,
> 
> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
> it will receive the
> subsystem up notification directly. Therefore, there’s no need to make it a
> subdev node or
> tie it into the init/reset sequence of remoteproc subsytem.

But you've added a subnode for it (and we are discussing exactly
of_platform_populate()) call. So, you are tying it to the remoteproc
device lifecycle instead of the remoteproc subsys, which seems strange
to me. There is no cooling device if the DSP is not running.

-- 
With best wishes
Dmitry

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Gaurav Kohli 1 week, 5 days ago

On 1/27/2026 10:11 PM, Dmitry Baryshkov wrote:
> On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
>> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
>>> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
>>>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
>>>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
>>>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>>>>>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>
>>>>>>> Generalise the qcom,bam-dmux child node support by probing all
>>>>>>> remoteproc children with of_platform_populate(). This will be used to
>>>>>>> enable support for devices which are best represented as
>>>>>>> subnodes of the
>>>>>>> remoteproc, such as those representing QMI clients.
>>>>>> Please flip this around, start with the description of the problem
>>>>>> you're trying to solve.
>>>>>>
>>>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>>>>> This must have your signed-off-by, where you certifies the origin of
>>>>>> this patch.
>>>>>>
>>>>>>> ---
>>>>>>>     drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>>>>>>     drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>>>>>>     2 files changed, 4 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
>>>>>>> b/drivers/remoteproc/qcom_q6v5.c
>>>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
>>>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>>>>>> @@ -6,6 +6,7 @@
>>>>>>>      * Copyright (C) 2014 Sony Mobile Communications AB
>>>>>>>      * Copyright (c) 2012-2013, The Linux Foundation. All rights
>>>>>>> reserved.
>>>>>>>      */
>>>>>>> +#include <linux/of_platform.h>
>>>>>>>     #include <linux/kernel.h>
>>>>>>>     #include <linux/platform_device.h>
>>>>>>>     #include <linux/interconnect.h>
>>>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
>>>>>>> struct platform_device *pdev,
>>>>>>>             return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>>>>>>                          "failed to acquire interconnect path\n");
>>>>>>>     +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
>>>>>> There are other child nodes here, in particular the GLINK and SMD edges.
>>>>>> Do we really want platform_devices registered for them?
>>>>>>
>>>>>> Regards,
>>>>>> Bjorn
>>>>> thanks for pointing this, can you please suggest the right approach.
>>>>>
>>>>> This should not impact glink, as that is registering as rproc sub node,
>>>>> And we need rproc cooling as child node
>>>>>
>>>>> of remote proc subsytem to create probe dependency only.
>>>>>
>>>>>
>>>>> Can we do platform populate for specific child, would that be right
>>>>> approach. or we should create rproc cooling as independent of parent ?
>>>>>
>>>> HI Bjorn,
>>>>
>>>> I’d like to highlight the impact and details of placement of remoteproc
>>>> cooling dt node:
>>>>
>>>>
>>>> ->As a child of the remote proc subsystem node:
>>>>       In this configuration, the cooling device will only be probed once the
>>>> corresponding remote proc subsystem itself is probed.
>>>>
>>>> ->Outside the remote proc subsystem, may be part of soc node:
>>>>       In this setup, the cooling device will be probed independently. It will
>>>> wait until the remoteproc subsystem is brought up
>>>>       before completing cooling registration.
>>>>       The drawback here is that if the parent remoteproc subsystem is
>>>> disabled, the cooling device will still undergo an
>>>>       unnecessary probe, even though it cannot be registered.
>>> Bjorns question was different. It wasn't about pushing cooling device
>>> outside of the remoteproc node. It is about not registering the devices.
>>>
>>> Can we follow the approach outlined by qcom_add_smd_subdev() /
>>> qcom_add_glink_subdev()?
>>
>> Hi Dmitry,
>>
>> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
>> it will receive the
>> subsystem up notification directly. Therefore, there’s no need to make it a
>> subdev node or
>> tie it into the init/reset sequence of remoteproc subsytem.
> But you've added a subnode for it (and we are discussing exactly
> of_platform_populate()) call. So, you are tying it to the remoteproc
> device lifecycle instead of the remoteproc subsys, which seems strange
> to me. There is no cooling device if the DSP is not running.


For the cooling feature, we don’t need to define it as a subnode. The 
cooling subsystem becomes relevant only
after the remote subsystem is up, at which point it will receive 
add/delete notifications from the QMI server.


If child nodes must be modeled as subnodes for rproc, we can move the 
CDSP TMD out of the remoteproc and add in soc.
Is there currently a way for the remoteproc core layer to call 
of_platform_populate() without requiring a subnode?

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Konrad Dybcio 1 week, 5 days ago

On 1/28/26 10:39 AM, Gaurav Kohli wrote:
> 
> On 1/27/2026 10:11 PM, Dmitry Baryshkov wrote:
>> On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
>>> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
>>>> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
>>>>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
>>>>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
>>>>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>>>>>>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>>
>>>>>>>> Generalise the qcom,bam-dmux child node support by probing all
>>>>>>>> remoteproc children with of_platform_populate(). This will be used to
>>>>>>>> enable support for devices which are best represented as
>>>>>>>> subnodes of the
>>>>>>>> remoteproc, such as those representing QMI clients.
>>>>>>> Please flip this around, start with the description of the problem
>>>>>>> you're trying to solve.
>>>>>>>
>>>>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>>>>>> This must have your signed-off-by, where you certifies the origin of
>>>>>>> this patch.
>>>>>>>
>>>>>>>> ---
>>>>>>>>     drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>>>>>>>     drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>>>>>>>     2 files changed, 4 insertions(+), 8 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>> b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
>>>>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>> @@ -6,6 +6,7 @@
>>>>>>>>      * Copyright (C) 2014 Sony Mobile Communications AB
>>>>>>>>      * Copyright (c) 2012-2013, The Linux Foundation. All rights
>>>>>>>> reserved.
>>>>>>>>      */
>>>>>>>> +#include <linux/of_platform.h>
>>>>>>>>     #include <linux/kernel.h>
>>>>>>>>     #include <linux/platform_device.h>
>>>>>>>>     #include <linux/interconnect.h>
>>>>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
>>>>>>>> struct platform_device *pdev,
>>>>>>>>             return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>>>>>>>                          "failed to acquire interconnect path\n");
>>>>>>>>     +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
>>>>>>> There are other child nodes here, in particular the GLINK and SMD edges.
>>>>>>> Do we really want platform_devices registered for them?
>>>>>>>
>>>>>>> Regards,
>>>>>>> Bjorn
>>>>>> thanks for pointing this, can you please suggest the right approach.
>>>>>>
>>>>>> This should not impact glink, as that is registering as rproc sub node,
>>>>>> And we need rproc cooling as child node
>>>>>>
>>>>>> of remote proc subsytem to create probe dependency only.
>>>>>>
>>>>>>
>>>>>> Can we do platform populate for specific child, would that be right
>>>>>> approach. or we should create rproc cooling as independent of parent ?
>>>>>>
>>>>> HI Bjorn,
>>>>>
>>>>> I’d like to highlight the impact and details of placement of remoteproc
>>>>> cooling dt node:
>>>>>
>>>>>
>>>>> ->As a child of the remote proc subsystem node:
>>>>>       In this configuration, the cooling device will only be probed once the
>>>>> corresponding remote proc subsystem itself is probed.
>>>>>
>>>>> ->Outside the remote proc subsystem, may be part of soc node:
>>>>>       In this setup, the cooling device will be probed independently. It will
>>>>> wait until the remoteproc subsystem is brought up
>>>>>       before completing cooling registration.
>>>>>       The drawback here is that if the parent remoteproc subsystem is
>>>>> disabled, the cooling device will still undergo an
>>>>>       unnecessary probe, even though it cannot be registered.
>>>> Bjorns question was different. It wasn't about pushing cooling device
>>>> outside of the remoteproc node. It is about not registering the devices.
>>>>
>>>> Can we follow the approach outlined by qcom_add_smd_subdev() /
>>>> qcom_add_glink_subdev()?
>>>
>>> Hi Dmitry,
>>>
>>> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
>>> it will receive the
>>> subsystem up notification directly. Therefore, there’s no need to make it a
>>> subdev node or
>>> tie it into the init/reset sequence of remoteproc subsytem.
>> But you've added a subnode for it (and we are discussing exactly
>> of_platform_populate()) call. So, you are tying it to the remoteproc
>> device lifecycle instead of the remoteproc subsys, which seems strange
>> to me. There is no cooling device if the DSP is not running.
> 
> 
> For the cooling feature, we don’t need to define it as a subnode. The cooling subsystem becomes relevant only
> after the remote subsystem is up, at which point it will receive add/delete notifications from the QMI server.
> 
> 
> If child nodes must be modeled as subnodes for rproc, we can move the CDSP TMD out of the remoteproc and add in soc.
> Is there currently a way for the remoteproc core layer to call of_platform_populate() without requiring a subnode?

I think the question is "why can't you register the remoteproc device
as a cooling_device, with perhaps #cooling-cells = <1>; instead of
any form of children?"

Konrad

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Gaurav Kohli 1 week, 3 days ago

On 1/28/2026 3:15 PM, Konrad Dybcio wrote:
> On 1/28/26 10:39 AM, Gaurav Kohli wrote:
>> On 1/27/2026 10:11 PM, Dmitry Baryshkov wrote:
>>> On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
>>>> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
>>>>> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
>>>>>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
>>>>>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
>>>>>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>>>>>>>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>>>
>>>>>>>>> Generalise the qcom,bam-dmux child node support by probing all
>>>>>>>>> remoteproc children with of_platform_populate(). This will be used to
>>>>>>>>> enable support for devices which are best represented as
>>>>>>>>> subnodes of the
>>>>>>>>> remoteproc, such as those representing QMI clients.
>>>>>>>> Please flip this around, start with the description of the problem
>>>>>>>> you're trying to solve.
>>>>>>>>
>>>>>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>> This must have your signed-off-by, where you certifies the origin of
>>>>>>>> this patch.
>>>>>>>>
>>>>>>>>> ---
>>>>>>>>>      drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>>>>>>>>      drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>>>>>>>>      2 files changed, 4 insertions(+), 8 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>> b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
>>>>>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>> @@ -6,6 +6,7 @@
>>>>>>>>>       * Copyright (C) 2014 Sony Mobile Communications AB
>>>>>>>>>       * Copyright (c) 2012-2013, The Linux Foundation. All rights
>>>>>>>>> reserved.
>>>>>>>>>       */
>>>>>>>>> +#include <linux/of_platform.h>
>>>>>>>>>      #include <linux/kernel.h>
>>>>>>>>>      #include <linux/platform_device.h>
>>>>>>>>>      #include <linux/interconnect.h>
>>>>>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
>>>>>>>>> struct platform_device *pdev,
>>>>>>>>>              return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>>>>>>>>                           "failed to acquire interconnect path\n");
>>>>>>>>>      +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
>>>>>>>> There are other child nodes here, in particular the GLINK and SMD edges.
>>>>>>>> Do we really want platform_devices registered for them?
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Bjorn
>>>>>>> thanks for pointing this, can you please suggest the right approach.
>>>>>>>
>>>>>>> This should not impact glink, as that is registering as rproc sub node,
>>>>>>> And we need rproc cooling as child node
>>>>>>>
>>>>>>> of remote proc subsytem to create probe dependency only.
>>>>>>>
>>>>>>>
>>>>>>> Can we do platform populate for specific child, would that be right
>>>>>>> approach. or we should create rproc cooling as independent of parent ?
>>>>>>>
>>>>>> HI Bjorn,
>>>>>>
>>>>>> I’d like to highlight the impact and details of placement of remoteproc
>>>>>> cooling dt node:
>>>>>>
>>>>>>
>>>>>> ->As a child of the remote proc subsystem node:
>>>>>>        In this configuration, the cooling device will only be probed once the
>>>>>> corresponding remote proc subsystem itself is probed.
>>>>>>
>>>>>> ->Outside the remote proc subsystem, may be part of soc node:
>>>>>>        In this setup, the cooling device will be probed independently. It will
>>>>>> wait until the remoteproc subsystem is brought up
>>>>>>        before completing cooling registration.
>>>>>>        The drawback here is that if the parent remoteproc subsystem is
>>>>>> disabled, the cooling device will still undergo an
>>>>>>        unnecessary probe, even though it cannot be registered.
>>>>> Bjorns question was different. It wasn't about pushing cooling device
>>>>> outside of the remoteproc node. It is about not registering the devices.
>>>>>
>>>>> Can we follow the approach outlined by qcom_add_smd_subdev() /
>>>>> qcom_add_glink_subdev()?
>>>> Hi Dmitry,
>>>>
>>>> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
>>>> it will receive the
>>>> subsystem up notification directly. Therefore, there’s no need to make it a
>>>> subdev node or
>>>> tie it into the init/reset sequence of remoteproc subsytem.
>>> But you've added a subnode for it (and we are discussing exactly
>>> of_platform_populate()) call. So, you are tying it to the remoteproc
>>> device lifecycle instead of the remoteproc subsys, which seems strange
>>> to me. There is no cooling device if the DSP is not running.
>>
>> For the cooling feature, we don’t need to define it as a subnode. The cooling subsystem becomes relevant only
>> after the remote subsystem is up, at which point it will receive add/delete notifications from the QMI server.
>>
>>
>> If child nodes must be modeled as subnodes for rproc, we can move the CDSP TMD out of the remoteproc and add in soc.
>> Is there currently a way for the remoteproc core layer to call of_platform_populate() without requiring a subnode?
> I think the question is "why can't you register the remoteproc device
> as a cooling_device, with perhaps #cooling-cells = <1>; instead of
> any form of children?"
>
> Konrad


thanks Konrad, for the review.

As each subsystem can expose multiple thermal mitigation devices via the 
remoteproc TMD service, so need to define child node.

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Konrad Dybcio 1 week, 3 days ago

On 1/30/26 8:03 AM, Gaurav Kohli wrote:
> 
> On 1/28/2026 3:15 PM, Konrad Dybcio wrote:
>> On 1/28/26 10:39 AM, Gaurav Kohli wrote:
>>> On 1/27/2026 10:11 PM, Dmitry Baryshkov wrote:
>>>> On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
>>>>> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
>>>>>> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
>>>>>>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
>>>>>>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
>>>>>>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>>>>>>>>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>>>>
>>>>>>>>>> Generalise the qcom,bam-dmux child node support by probing all
>>>>>>>>>> remoteproc children with of_platform_populate(). This will be used to
>>>>>>>>>> enable support for devices which are best represented as
>>>>>>>>>> subnodes of the
>>>>>>>>>> remoteproc, such as those representing QMI clients.
>>>>>>>>> Please flip this around, start with the description of the problem
>>>>>>>>> you're trying to solve.
>>>>>>>>>
>>>>>>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>>> This must have your signed-off-by, where you certifies the origin of
>>>>>>>>> this patch.
>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>>      drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>>>>>>>>>      drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>>>>>>>>>      2 files changed, 4 insertions(+), 8 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>> b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
>>>>>>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>> @@ -6,6 +6,7 @@
>>>>>>>>>>       * Copyright (C) 2014 Sony Mobile Communications AB
>>>>>>>>>>       * Copyright (c) 2012-2013, The Linux Foundation. All rights
>>>>>>>>>> reserved.
>>>>>>>>>>       */
>>>>>>>>>> +#include <linux/of_platform.h>
>>>>>>>>>>      #include <linux/kernel.h>
>>>>>>>>>>      #include <linux/platform_device.h>
>>>>>>>>>>      #include <linux/interconnect.h>
>>>>>>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
>>>>>>>>>> struct platform_device *pdev,
>>>>>>>>>>              return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>>>>>>>>>                           "failed to acquire interconnect path\n");
>>>>>>>>>>      +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
>>>>>>>>> There are other child nodes here, in particular the GLINK and SMD edges.
>>>>>>>>> Do we really want platform_devices registered for them?
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Bjorn
>>>>>>>> thanks for pointing this, can you please suggest the right approach.
>>>>>>>>
>>>>>>>> This should not impact glink, as that is registering as rproc sub node,
>>>>>>>> And we need rproc cooling as child node
>>>>>>>>
>>>>>>>> of remote proc subsytem to create probe dependency only.
>>>>>>>>
>>>>>>>>
>>>>>>>> Can we do platform populate for specific child, would that be right
>>>>>>>> approach. or we should create rproc cooling as independent of parent ?
>>>>>>>>
>>>>>>> HI Bjorn,
>>>>>>>
>>>>>>> I’d like to highlight the impact and details of placement of remoteproc
>>>>>>> cooling dt node:
>>>>>>>
>>>>>>>
>>>>>>> ->As a child of the remote proc subsystem node:
>>>>>>>        In this configuration, the cooling device will only be probed once the
>>>>>>> corresponding remote proc subsystem itself is probed.
>>>>>>>
>>>>>>> ->Outside the remote proc subsystem, may be part of soc node:
>>>>>>>        In this setup, the cooling device will be probed independently. It will
>>>>>>> wait until the remoteproc subsystem is brought up
>>>>>>>        before completing cooling registration.
>>>>>>>        The drawback here is that if the parent remoteproc subsystem is
>>>>>>> disabled, the cooling device will still undergo an
>>>>>>>        unnecessary probe, even though it cannot be registered.
>>>>>> Bjorns question was different. It wasn't about pushing cooling device
>>>>>> outside of the remoteproc node. It is about not registering the devices.
>>>>>>
>>>>>> Can we follow the approach outlined by qcom_add_smd_subdev() /
>>>>>> qcom_add_glink_subdev()?
>>>>> Hi Dmitry,
>>>>>
>>>>> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
>>>>> it will receive the
>>>>> subsystem up notification directly. Therefore, there’s no need to make it a
>>>>> subdev node or
>>>>> tie it into the init/reset sequence of remoteproc subsytem.
>>>> But you've added a subnode for it (and we are discussing exactly
>>>> of_platform_populate()) call. So, you are tying it to the remoteproc
>>>> device lifecycle instead of the remoteproc subsys, which seems strange
>>>> to me. There is no cooling device if the DSP is not running.
>>>
>>> For the cooling feature, we don’t need to define it as a subnode. The cooling subsystem becomes relevant only
>>> after the remote subsystem is up, at which point it will receive add/delete notifications from the QMI server.
>>>
>>>
>>> If child nodes must be modeled as subnodes for rproc, we can move the CDSP TMD out of the remoteproc and add in soc.
>>> Is there currently a way for the remoteproc core layer to call of_platform_populate() without requiring a subnode?
>> I think the question is "why can't you register the remoteproc device
>> as a cooling_device, with perhaps #cooling-cells = <1>; instead of
>> any form of children?"
>>
>> Konrad
> 
> 
> thanks Konrad, for the review.
> 
> As each subsystem can expose multiple thermal mitigation devices via the remoteproc TMD service, so need to define child node.

I think you're stuck in an XY problem - you keep insisting that adding
a subnode is your end goal, while you really want to achieve being able
to register multiple cooling devices. Or at least that's how I read your
messages since you happen not to give any explanation as to why it's
actually necessary.

In my previous message, I forgot that cells for cooling devices actually
represent the minimum and maximum cooling state allowed. But since the
API is just part of the kernel, there's nothing preventing us from
evolving it.

Currently, we have:

Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml

properties:
  "#cooling-cells":
    description:
      Must be 2, in order to specify minimum and maximum cooling state used in
      the cooling-maps reference. The first cell is the minimum cooling state
      and the second cell is the maximum cooling state requested.
    const: 2

But I think it would be perfectly fine to suggest a change such that
if cells > 2, the last two cells keep the current behavior and the former
ones let you index into a cooling device exposed through a single OF node

e.g.

rproc_xyz: remoteproc {
	compatible = "qcom,rproc-xyz";

	...

	#cooling-device-cells = <3>;
};

...

thermal-zones {
	super-rproc-therm-a {
		thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_A
				   THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

		trips { ... } ;
	};

	super-rproc-therm-b {
		thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_B
				   THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

		trips { ... } ;
	};
};

This would be resolved by allowing drivers to register an .of_xlate-type
function which would take the RPROC_XYZ_COOLING_n argument and e.g. use
it as an index into struct thermal_cooling_device cdev[]; within the
driver struct.

Konrad

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Dmitry Baryshkov 1 week, 2 days ago

On Fri, Jan 30, 2026 at 10:13:06AM +0100, Konrad Dybcio wrote:
> On 1/30/26 8:03 AM, Gaurav Kohli wrote:
> > 
> > On 1/28/2026 3:15 PM, Konrad Dybcio wrote:
> >> On 1/28/26 10:39 AM, Gaurav Kohli wrote:
> >>> On 1/27/2026 10:11 PM, Dmitry Baryshkov wrote:
> >>>> On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
> >>>>> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
> >>>>>> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
> >>>>>>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
> >>>>>>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
> >>>>>>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
> >>>>>>>>>> From: Casey Connolly <casey.connolly@linaro.org>
> >>>>>>>>>>
> >>>>>>>>>> Generalise the qcom,bam-dmux child node support by probing all
> >>>>>>>>>> remoteproc children with of_platform_populate(). This will be used to
> >>>>>>>>>> enable support for devices which are best represented as
> >>>>>>>>>> subnodes of the
> >>>>>>>>>> remoteproc, such as those representing QMI clients.
> >>>>>>>>> Please flip this around, start with the description of the problem
> >>>>>>>>> you're trying to solve.
> >>>>>>>>>
> >>>>>>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> >>>>>>>>> This must have your signed-off-by, where you certifies the origin of
> >>>>>>>>> this patch.
> >>>>>>>>>
> >>>>>>>>>> ---
> >>>>>>>>>>      drivers/remoteproc/qcom_q6v5.c     | 4 ++++
> >>>>>>>>>>      drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
> >>>>>>>>>>      2 files changed, 4 insertions(+), 8 deletions(-)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
> >>>>>>>>>> b/drivers/remoteproc/qcom_q6v5.c
> >>>>>>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
> >>>>>>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
> >>>>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
> >>>>>>>>>> @@ -6,6 +6,7 @@
> >>>>>>>>>>       * Copyright (C) 2014 Sony Mobile Communications AB
> >>>>>>>>>>       * Copyright (c) 2012-2013, The Linux Foundation. All rights
> >>>>>>>>>> reserved.
> >>>>>>>>>>       */
> >>>>>>>>>> +#include <linux/of_platform.h>
> >>>>>>>>>>      #include <linux/kernel.h>
> >>>>>>>>>>      #include <linux/platform_device.h>
> >>>>>>>>>>      #include <linux/interconnect.h>
> >>>>>>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
> >>>>>>>>>> struct platform_device *pdev,
> >>>>>>>>>>              return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
> >>>>>>>>>>                           "failed to acquire interconnect path\n");
> >>>>>>>>>>      +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
> >>>>>>>>> There are other child nodes here, in particular the GLINK and SMD edges.
> >>>>>>>>> Do we really want platform_devices registered for them?
> >>>>>>>>>
> >>>>>>>>> Regards,
> >>>>>>>>> Bjorn
> >>>>>>>> thanks for pointing this, can you please suggest the right approach.
> >>>>>>>>
> >>>>>>>> This should not impact glink, as that is registering as rproc sub node,
> >>>>>>>> And we need rproc cooling as child node
> >>>>>>>>
> >>>>>>>> of remote proc subsytem to create probe dependency only.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Can we do platform populate for specific child, would that be right
> >>>>>>>> approach. or we should create rproc cooling as independent of parent ?
> >>>>>>>>
> >>>>>>> HI Bjorn,
> >>>>>>>
> >>>>>>> I’d like to highlight the impact and details of placement of remoteproc
> >>>>>>> cooling dt node:
> >>>>>>>
> >>>>>>>
> >>>>>>> ->As a child of the remote proc subsystem node:
> >>>>>>>        In this configuration, the cooling device will only be probed once the
> >>>>>>> corresponding remote proc subsystem itself is probed.
> >>>>>>>
> >>>>>>> ->Outside the remote proc subsystem, may be part of soc node:
> >>>>>>>        In this setup, the cooling device will be probed independently. It will
> >>>>>>> wait until the remoteproc subsystem is brought up
> >>>>>>>        before completing cooling registration.
> >>>>>>>        The drawback here is that if the parent remoteproc subsystem is
> >>>>>>> disabled, the cooling device will still undergo an
> >>>>>>>        unnecessary probe, even though it cannot be registered.
> >>>>>> Bjorns question was different. It wasn't about pushing cooling device
> >>>>>> outside of the remoteproc node. It is about not registering the devices.
> >>>>>>
> >>>>>> Can we follow the approach outlined by qcom_add_smd_subdev() /
> >>>>>> qcom_add_glink_subdev()?
> >>>>> Hi Dmitry,
> >>>>>
> >>>>> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
> >>>>> it will receive the
> >>>>> subsystem up notification directly. Therefore, there’s no need to make it a
> >>>>> subdev node or
> >>>>> tie it into the init/reset sequence of remoteproc subsytem.
> >>>> But you've added a subnode for it (and we are discussing exactly
> >>>> of_platform_populate()) call. So, you are tying it to the remoteproc
> >>>> device lifecycle instead of the remoteproc subsys, which seems strange
> >>>> to me. There is no cooling device if the DSP is not running.
> >>>
> >>> For the cooling feature, we don’t need to define it as a subnode. The cooling subsystem becomes relevant only
> >>> after the remote subsystem is up, at which point it will receive add/delete notifications from the QMI server.
> >>>
> >>>
> >>> If child nodes must be modeled as subnodes for rproc, we can move the CDSP TMD out of the remoteproc and add in soc.
> >>> Is there currently a way for the remoteproc core layer to call of_platform_populate() without requiring a subnode?
> >> I think the question is "why can't you register the remoteproc device
> >> as a cooling_device, with perhaps #cooling-cells = <1>; instead of
> >> any form of children?"
> >>
> >> Konrad
> > 
> > 
> > thanks Konrad, for the review.
> > 
> > As each subsystem can expose multiple thermal mitigation devices via the remoteproc TMD service, so need to define child node.
> 
> I think you're stuck in an XY problem - you keep insisting that adding
> a subnode is your end goal, while you really want to achieve being able
> to register multiple cooling devices. Or at least that's how I read your
> messages since you happen not to give any explanation as to why it's
> actually necessary.
> 
> In my previous message, I forgot that cells for cooling devices actually
> represent the minimum and maximum cooling state allowed. But since the
> API is just part of the kernel, there's nothing preventing us from
> evolving it.
> 
> Currently, we have:
> 
> Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
> 
> properties:
>   "#cooling-cells":
>     description:
>       Must be 2, in order to specify minimum and maximum cooling state used in
>       the cooling-maps reference. The first cell is the minimum cooling state
>       and the second cell is the maximum cooling state requested.
>     const: 2
> 
> But I think it would be perfectly fine to suggest a change such that
> if cells > 2, the last two cells keep the current behavior and the former
> ones let you index into a cooling device exposed through a single OF node

This might be a big change, which probably needs to be coordinated with
thermal and DT maintainers first.

> 
> e.g.
> 
> rproc_xyz: remoteproc {
> 	compatible = "qcom,rproc-xyz";
> 
> 	...
> 
> 	#cooling-device-cells = <3>;
> };

Which brings in another topic. In DT we have labels for different DT
children, which correspond to different handlers on the DSP side. For
the CDSP we see a "cdsp_sw" only. I think I've asked several times, but
didn't get an example of the device having more than one, just claims
that there might be more thane one TMD.

Do we need different cooling cells here? Or would it be enough to send
the same max state to all TMDs on the DSP side?

> 
> ...
> 
> thermal-zones {
> 	super-rproc-therm-a {
> 		thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_A
> 				   THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> 
> 		trips { ... } ;
> 	};
> 
> 	super-rproc-therm-b {
> 		thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_B
> 				   THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> 
> 		trips { ... } ;
> 	};
> };
> 
> This would be resolved by allowing drivers to register an .of_xlate-type
> function which would take the RPROC_XYZ_COOLING_n argument and e.g. use
> it as an index into struct thermal_cooling_device cdev[]; within the
> driver struct.



> 
> Konrad

-- 
With best wishes
Dmitry

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Gaurav Kohli 1 week, 2 days ago


On 1/31/2026 1:36 PM, Dmitry Baryshkov wrote:
> On Fri, Jan 30, 2026 at 10:13:06AM +0100, Konrad Dybcio wrote:
>> On 1/30/26 8:03 AM, Gaurav Kohli wrote:
>>>
>>> On 1/28/2026 3:15 PM, Konrad Dybcio wrote:
>>>> On 1/28/26 10:39 AM, Gaurav Kohli wrote:
>>>>> On 1/27/2026 10:11 PM, Dmitry Baryshkov wrote:
>>>>>> On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
>>>>>>> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
>>>>>>>> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
>>>>>>>>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
>>>>>>>>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
>>>>>>>>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>>>>>>>>>>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>>>>>>
>>>>>>>>>>>> Generalise the qcom,bam-dmux child node support by probing all
>>>>>>>>>>>> remoteproc children with of_platform_populate(). This will be used to
>>>>>>>>>>>> enable support for devices which are best represented as
>>>>>>>>>>>> subnodes of the
>>>>>>>>>>>> remoteproc, such as those representing QMI clients.
>>>>>>>>>>> Please flip this around, start with the description of the problem
>>>>>>>>>>> you're trying to solve.
>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>>>>> This must have your signed-off-by, where you certifies the origin of
>>>>>>>>>>> this patch.
>>>>>>>>>>>
>>>>>>>>>>>> ---
>>>>>>>>>>>>       drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>>>>>>>>>>>       drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>>>>>>>>>>>       2 files changed, 4 insertions(+), 8 deletions(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>>>> b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
>>>>>>>>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>>>> @@ -6,6 +6,7 @@
>>>>>>>>>>>>        * Copyright (C) 2014 Sony Mobile Communications AB
>>>>>>>>>>>>        * Copyright (c) 2012-2013, The Linux Foundation. All rights
>>>>>>>>>>>> reserved.
>>>>>>>>>>>>        */
>>>>>>>>>>>> +#include <linux/of_platform.h>
>>>>>>>>>>>>       #include <linux/kernel.h>
>>>>>>>>>>>>       #include <linux/platform_device.h>
>>>>>>>>>>>>       #include <linux/interconnect.h>
>>>>>>>>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
>>>>>>>>>>>> struct platform_device *pdev,
>>>>>>>>>>>>               return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>>>>>>>>>>>                            "failed to acquire interconnect path\n");
>>>>>>>>>>>>       +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
>>>>>>>>>>> There are other child nodes here, in particular the GLINK and SMD edges.
>>>>>>>>>>> Do we really want platform_devices registered for them?
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Bjorn
>>>>>>>>>> thanks for pointing this, can you please suggest the right approach.
>>>>>>>>>>
>>>>>>>>>> This should not impact glink, as that is registering as rproc sub node,
>>>>>>>>>> And we need rproc cooling as child node
>>>>>>>>>>
>>>>>>>>>> of remote proc subsytem to create probe dependency only.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Can we do platform populate for specific child, would that be right
>>>>>>>>>> approach. or we should create rproc cooling as independent of parent ?
>>>>>>>>>>
>>>>>>>>> HI Bjorn,
>>>>>>>>>
>>>>>>>>> I’d like to highlight the impact and details of placement of remoteproc
>>>>>>>>> cooling dt node:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ->As a child of the remote proc subsystem node:
>>>>>>>>>         In this configuration, the cooling device will only be probed once the
>>>>>>>>> corresponding remote proc subsystem itself is probed.
>>>>>>>>>
>>>>>>>>> ->Outside the remote proc subsystem, may be part of soc node:
>>>>>>>>>         In this setup, the cooling device will be probed independently. It will
>>>>>>>>> wait until the remoteproc subsystem is brought up
>>>>>>>>>         before completing cooling registration.
>>>>>>>>>         The drawback here is that if the parent remoteproc subsystem is
>>>>>>>>> disabled, the cooling device will still undergo an
>>>>>>>>>         unnecessary probe, even though it cannot be registered.
>>>>>>>> Bjorns question was different. It wasn't about pushing cooling device
>>>>>>>> outside of the remoteproc node. It is about not registering the devices.
>>>>>>>>
>>>>>>>> Can we follow the approach outlined by qcom_add_smd_subdev() /
>>>>>>>> qcom_add_glink_subdev()?
>>>>>>> Hi Dmitry,
>>>>>>>
>>>>>>> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
>>>>>>> it will receive the
>>>>>>> subsystem up notification directly. Therefore, there’s no need to make it a
>>>>>>> subdev node or
>>>>>>> tie it into the init/reset sequence of remoteproc subsytem.
>>>>>> But you've added a subnode for it (and we are discussing exactly
>>>>>> of_platform_populate()) call. So, you are tying it to the remoteproc
>>>>>> device lifecycle instead of the remoteproc subsys, which seems strange
>>>>>> to me. There is no cooling device if the DSP is not running.
>>>>>
>>>>> For the cooling feature, we don’t need to define it as a subnode. The cooling subsystem becomes relevant only
>>>>> after the remote subsystem is up, at which point it will receive add/delete notifications from the QMI server.
>>>>>
>>>>>
>>>>> If child nodes must be modeled as subnodes for rproc, we can move the CDSP TMD out of the remoteproc and add in soc.
>>>>> Is there currently a way for the remoteproc core layer to call of_platform_populate() without requiring a subnode?
>>>> I think the question is "why can't you register the remoteproc device
>>>> as a cooling_device, with perhaps #cooling-cells = <1>; instead of
>>>> any form of children?"
>>>>
>>>> Konrad
>>>
>>>
>>> thanks Konrad, for the review.
>>>
>>> As each subsystem can expose multiple thermal mitigation devices via the remoteproc TMD service, so need to define child node.
>>
>> I think you're stuck in an XY problem - you keep insisting that adding
>> a subnode is your end goal, while you really want to achieve being able
>> to register multiple cooling devices. Or at least that's how I read your
>> messages since you happen not to give any explanation as to why it's
>> actually necessary.
>>
>> In my previous message, I forgot that cells for cooling devices actually
>> represent the minimum and maximum cooling state allowed. But since the
>> API is just part of the kernel, there's nothing preventing us from
>> evolving it.
>>
>> Currently, we have:
>>
>> Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
>>
>> properties:
>>    "#cooling-cells":
>>      description:
>>        Must be 2, in order to specify minimum and maximum cooling state used in
>>        the cooling-maps reference. The first cell is the minimum cooling state
>>        and the second cell is the maximum cooling state requested.
>>      const: 2
>>
>> But I think it would be perfectly fine to suggest a change such that
>> if cells > 2, the last two cells keep the current behavior and the former
>> ones let you index into a cooling device exposed through a single OF node
> 
> This might be a big change, which probably needs to be coordinated with
> thermal and DT maintainers first.
> 
>>
>> e.g.
>>
>> rproc_xyz: remoteproc {
>> 	compatible = "qcom,rproc-xyz";
>>
>> 	...
>>
>> 	#cooling-device-cells = <3>;
>> };
> 
> Which brings in another topic. In DT we have labels for different DT
> children, which correspond to different handlers on the DSP side. For
> the CDSP we see a "cdsp_sw" only. I think I've asked several times, but
> didn't get an example of the device having more than one, just claims
> that there might be more thane one TMD.
> 
> Do we need different cooling cells here? Or would it be enough to send
> the same max state to all TMDs on the DSP side?
> 

For newer targets, Within the CDSP we have compute core(cdsp-sw),
npu(hmx-sw) core and both have independent dcvs and also dedicated
tsens on each core.

And For Modem also we have multiple mitigation devices based on
different modem tech, for e.g tech level side we have modem-lte,
modem-nr etc and mitigation at different power amplifier side like
modem-pa etc. We have not added modem node for current series target
as it does not support modem.

>>
>> ...
>>
>> thermal-zones {
>> 	super-rproc-therm-a {
>> 		thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_A
>> 				   THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>
>> 		trips { ... } ;
>> 	};
>>
>> 	super-rproc-therm-b {
>> 		thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_B
>> 				   THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>
>> 		trips { ... } ;
>> 	};
>> };
>>
>> This would be resolved by allowing drivers to register an .of_xlate-type
>> function which would take the RPROC_XYZ_COOLING_n argument and e.g. use
>> it as an index into struct thermal_cooling_device cdev[]; within the
>> driver struct.
> 
> 
> 
>>
>> Konrad
>

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Dmitry Baryshkov 1 week, 2 days ago

On Sat, 31 Jan 2026 at 11:12, Gaurav Kohli
<gaurav.kohli@oss.qualcomm.com> wrote:
>
>
>
> On 1/31/2026 1:36 PM, Dmitry Baryshkov wrote:
> > On Fri, Jan 30, 2026 at 10:13:06AM +0100, Konrad Dybcio wrote:
> >> On 1/30/26 8:03 AM, Gaurav Kohli wrote:
> >>>
> >>> On 1/28/2026 3:15 PM, Konrad Dybcio wrote:
> >>>> On 1/28/26 10:39 AM, Gaurav Kohli wrote:
> >>>>> On 1/27/2026 10:11 PM, Dmitry Baryshkov wrote:
> >>>>>> On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
> >>>>>>> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
> >>>>>>>> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
> >>>>>>>>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
> >>>>>>>>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
> >>>>>>>>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
> >>>>>>>>>>>> From: Casey Connolly <casey.connolly@linaro.org>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Generalise the qcom,bam-dmux child node support by probing all
> >>>>>>>>>>>> remoteproc children with of_platform_populate(). This will be used to
> >>>>>>>>>>>> enable support for devices which are best represented as
> >>>>>>>>>>>> subnodes of the
> >>>>>>>>>>>> remoteproc, such as those representing QMI clients.
> >>>>>>>>>>> Please flip this around, start with the description of the problem
> >>>>>>>>>>> you're trying to solve.
> >>>>>>>>>>>
> >>>>>>>>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> >>>>>>>>>>> This must have your signed-off-by, where you certifies the origin of
> >>>>>>>>>>> this patch.
> >>>>>>>>>>>
> >>>>>>>>>>>> ---
> >>>>>>>>>>>>       drivers/remoteproc/qcom_q6v5.c     | 4 ++++
> >>>>>>>>>>>>       drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
> >>>>>>>>>>>>       2 files changed, 4 insertions(+), 8 deletions(-)
> >>>>>>>>>>>>
> >>>>>>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
> >>>>>>>>>>>> b/drivers/remoteproc/qcom_q6v5.c
> >>>>>>>>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
> >>>>>>>>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
> >>>>>>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
> >>>>>>>>>>>> @@ -6,6 +6,7 @@
> >>>>>>>>>>>>        * Copyright (C) 2014 Sony Mobile Communications AB
> >>>>>>>>>>>>        * Copyright (c) 2012-2013, The Linux Foundation. All rights
> >>>>>>>>>>>> reserved.
> >>>>>>>>>>>>        */
> >>>>>>>>>>>> +#include <linux/of_platform.h>
> >>>>>>>>>>>>       #include <linux/kernel.h>
> >>>>>>>>>>>>       #include <linux/platform_device.h>
> >>>>>>>>>>>>       #include <linux/interconnect.h>
> >>>>>>>>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
> >>>>>>>>>>>> struct platform_device *pdev,
> >>>>>>>>>>>>               return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
> >>>>>>>>>>>>                            "failed to acquire interconnect path\n");
> >>>>>>>>>>>>       +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
> >>>>>>>>>>> There are other child nodes here, in particular the GLINK and SMD edges.
> >>>>>>>>>>> Do we really want platform_devices registered for them?
> >>>>>>>>>>>
> >>>>>>>>>>> Regards,
> >>>>>>>>>>> Bjorn
> >>>>>>>>>> thanks for pointing this, can you please suggest the right approach.
> >>>>>>>>>>
> >>>>>>>>>> This should not impact glink, as that is registering as rproc sub node,
> >>>>>>>>>> And we need rproc cooling as child node
> >>>>>>>>>>
> >>>>>>>>>> of remote proc subsytem to create probe dependency only.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Can we do platform populate for specific child, would that be right
> >>>>>>>>>> approach. or we should create rproc cooling as independent of parent ?
> >>>>>>>>>>
> >>>>>>>>> HI Bjorn,
> >>>>>>>>>
> >>>>>>>>> I’d like to highlight the impact and details of placement of remoteproc
> >>>>>>>>> cooling dt node:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> ->As a child of the remote proc subsystem node:
> >>>>>>>>>         In this configuration, the cooling device will only be probed once the
> >>>>>>>>> corresponding remote proc subsystem itself is probed.
> >>>>>>>>>
> >>>>>>>>> ->Outside the remote proc subsystem, may be part of soc node:
> >>>>>>>>>         In this setup, the cooling device will be probed independently. It will
> >>>>>>>>> wait until the remoteproc subsystem is brought up
> >>>>>>>>>         before completing cooling registration.
> >>>>>>>>>         The drawback here is that if the parent remoteproc subsystem is
> >>>>>>>>> disabled, the cooling device will still undergo an
> >>>>>>>>>         unnecessary probe, even though it cannot be registered.
> >>>>>>>> Bjorns question was different. It wasn't about pushing cooling device
> >>>>>>>> outside of the remoteproc node. It is about not registering the devices.
> >>>>>>>>
> >>>>>>>> Can we follow the approach outlined by qcom_add_smd_subdev() /
> >>>>>>>> qcom_add_glink_subdev()?
> >>>>>>> Hi Dmitry,
> >>>>>>>
> >>>>>>> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
> >>>>>>> it will receive the
> >>>>>>> subsystem up notification directly. Therefore, there’s no need to make it a
> >>>>>>> subdev node or
> >>>>>>> tie it into the init/reset sequence of remoteproc subsytem.
> >>>>>> But you've added a subnode for it (and we are discussing exactly
> >>>>>> of_platform_populate()) call. So, you are tying it to the remoteproc
> >>>>>> device lifecycle instead of the remoteproc subsys, which seems strange
> >>>>>> to me. There is no cooling device if the DSP is not running.
> >>>>>
> >>>>> For the cooling feature, we don’t need to define it as a subnode. The cooling subsystem becomes relevant only
> >>>>> after the remote subsystem is up, at which point it will receive add/delete notifications from the QMI server.
> >>>>>
> >>>>>
> >>>>> If child nodes must be modeled as subnodes for rproc, we can move the CDSP TMD out of the remoteproc and add in soc.
> >>>>> Is there currently a way for the remoteproc core layer to call of_platform_populate() without requiring a subnode?
> >>>> I think the question is "why can't you register the remoteproc device
> >>>> as a cooling_device, with perhaps #cooling-cells = <1>; instead of
> >>>> any form of children?"
> >>>>
> >>>> Konrad
> >>>
> >>>
> >>> thanks Konrad, for the review.
> >>>
> >>> As each subsystem can expose multiple thermal mitigation devices via the remoteproc TMD service, so need to define child node.
> >>
> >> I think you're stuck in an XY problem - you keep insisting that adding
> >> a subnode is your end goal, while you really want to achieve being able
> >> to register multiple cooling devices. Or at least that's how I read your
> >> messages since you happen not to give any explanation as to why it's
> >> actually necessary.
> >>
> >> In my previous message, I forgot that cells for cooling devices actually
> >> represent the minimum and maximum cooling state allowed. But since the
> >> API is just part of the kernel, there's nothing preventing us from
> >> evolving it.
> >>
> >> Currently, we have:
> >>
> >> Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
> >>
> >> properties:
> >>    "#cooling-cells":
> >>      description:
> >>        Must be 2, in order to specify minimum and maximum cooling state used in
> >>        the cooling-maps reference. The first cell is the minimum cooling state
> >>        and the second cell is the maximum cooling state requested.
> >>      const: 2
> >>
> >> But I think it would be perfectly fine to suggest a change such that
> >> if cells > 2, the last two cells keep the current behavior and the former
> >> ones let you index into a cooling device exposed through a single OF node
> >
> > This might be a big change, which probably needs to be coordinated with
> > thermal and DT maintainers first.
> >
> >>
> >> e.g.
> >>
> >> rproc_xyz: remoteproc {
> >>      compatible = "qcom,rproc-xyz";
> >>
> >>      ...
> >>
> >>      #cooling-device-cells = <3>;
> >> };
> >
> > Which brings in another topic. In DT we have labels for different DT
> > children, which correspond to different handlers on the DSP side. For
> > the CDSP we see a "cdsp_sw" only. I think I've asked several times, but
> > didn't get an example of the device having more than one, just claims
> > that there might be more thane one TMD.
> >
> > Do we need different cooling cells here? Or would it be enough to send
> > the same max state to all TMDs on the DSP side?
> >
>
> For newer targets, Within the CDSP we have compute core(cdsp-sw),
> npu(hmx-sw) core and both have independent dcvs and also dedicated
> tsens on each core.

Okay, inside the DT bindings please provide this system as an example
rather than using the simplest one.

>
> And For Modem also we have multiple mitigation devices based on
> different modem tech, for e.g tech level side we have modem-lte,
> modem-nr etc and mitigation at different power amplifier side like
> modem-pa etc. We have not added modem node for current series target
> as it does not support modem.

I wrote it several times: there is no reason to limit yourself to
CDSP. Please also add support for ADSP, SLPI and modem.

>
> >>
> >> ...
> >>
> >> thermal-zones {
> >>      super-rproc-therm-a {
> >>              thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_A
> >>                                 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> >>
> >>              trips { ... } ;
> >>      };
> >>
> >>      super-rproc-therm-b {
> >>              thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_B
> >>                                 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> >>
> >>              trips { ... } ;
> >>      };
> >> };
> >>
> >> This would be resolved by allowing drivers to register an .of_xlate-type
> >> function which would take the RPROC_XYZ_COOLING_n argument and e.g. use
> >> it as an index into struct thermal_cooling_device cdev[]; within the
> >> driver struct.
> >
> >
> >
> >>
> >> Konrad
> >
>


-- 
With best wishes
Dmitry

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Gaurav Kohli 1 week, 2 days ago


On 1/31/2026 5:10 PM, Dmitry Baryshkov wrote:
> On Sat, 31 Jan 2026 at 11:12, Gaurav Kohli
> <gaurav.kohli@oss.qualcomm.com> wrote:
>>
>>
>>
>> On 1/31/2026 1:36 PM, Dmitry Baryshkov wrote:
>>> On Fri, Jan 30, 2026 at 10:13:06AM +0100, Konrad Dybcio wrote:
>>>> On 1/30/26 8:03 AM, Gaurav Kohli wrote:
>>>>>
>>>>> On 1/28/2026 3:15 PM, Konrad Dybcio wrote:
>>>>>> On 1/28/26 10:39 AM, Gaurav Kohli wrote:
>>>>>>> On 1/27/2026 10:11 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Tue, Jan 27, 2026 at 09:42:10PM +0530, Gaurav Kohli wrote:
>>>>>>>>> On 1/24/2026 12:33 AM, Dmitry Baryshkov wrote:
>>>>>>>>>> On Fri, Jan 23, 2026 at 07:23:39PM +0530, Gaurav Kohli wrote:
>>>>>>>>>>> On 1/8/2026 12:37 PM, Gaurav Kohli wrote:
>>>>>>>>>>>> On 1/3/2026 8:26 PM, Bjorn Andersson wrote:
>>>>>>>>>>>>> On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
>>>>>>>>>>>>>> From: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Generalise the qcom,bam-dmux child node support by probing all
>>>>>>>>>>>>>> remoteproc children with of_platform_populate(). This will be used to
>>>>>>>>>>>>>> enable support for devices which are best represented as
>>>>>>>>>>>>>> subnodes of the
>>>>>>>>>>>>>> remoteproc, such as those representing QMI clients.
>>>>>>>>>>>>> Please flip this around, start with the description of the problem
>>>>>>>>>>>>> you're trying to solve.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>>>>>>>>>>>>> This must have your signed-off-by, where you certifies the origin of
>>>>>>>>>>>>> this patch.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>        drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>>>>>>>>>>>>>>        drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>>>>>>>>>>>>>>        2 files changed, 4 insertions(+), 8 deletions(-)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>>>>>> b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>>>>>> index 58d5b85e58cd..a02839c7ed8c 100644
>>>>>>>>>>>>>> --- a/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>>>>>>>>>>>>> @@ -6,6 +6,7 @@
>>>>>>>>>>>>>>         * Copyright (C) 2014 Sony Mobile Communications AB
>>>>>>>>>>>>>>         * Copyright (c) 2012-2013, The Linux Foundation. All rights
>>>>>>>>>>>>>> reserved.
>>>>>>>>>>>>>>         */
>>>>>>>>>>>>>> +#include <linux/of_platform.h>
>>>>>>>>>>>>>>        #include <linux/kernel.h>
>>>>>>>>>>>>>>        #include <linux/platform_device.h>
>>>>>>>>>>>>>>        #include <linux/interconnect.h>
>>>>>>>>>>>>>> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5,
>>>>>>>>>>>>>> struct platform_device *pdev,
>>>>>>>>>>>>>>                return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>>>>>>>>>>>>>>                             "failed to acquire interconnect path\n");
>>>>>>>>>>>>>>        +    of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
>>>>>>>>>>>>> There are other child nodes here, in particular the GLINK and SMD edges.
>>>>>>>>>>>>> Do we really want platform_devices registered for them?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>> Bjorn
>>>>>>>>>>>> thanks for pointing this, can you please suggest the right approach.
>>>>>>>>>>>>
>>>>>>>>>>>> This should not impact glink, as that is registering as rproc sub node,
>>>>>>>>>>>> And we need rproc cooling as child node
>>>>>>>>>>>>
>>>>>>>>>>>> of remote proc subsytem to create probe dependency only.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Can we do platform populate for specific child, would that be right
>>>>>>>>>>>> approach. or we should create rproc cooling as independent of parent ?
>>>>>>>>>>>>
>>>>>>>>>>> HI Bjorn,
>>>>>>>>>>>
>>>>>>>>>>> I’d like to highlight the impact and details of placement of remoteproc
>>>>>>>>>>> cooling dt node:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ->As a child of the remote proc subsystem node:
>>>>>>>>>>>          In this configuration, the cooling device will only be probed once the
>>>>>>>>>>> corresponding remote proc subsystem itself is probed.
>>>>>>>>>>>
>>>>>>>>>>> ->Outside the remote proc subsystem, may be part of soc node:
>>>>>>>>>>>          In this setup, the cooling device will be probed independently. It will
>>>>>>>>>>> wait until the remoteproc subsystem is brought up
>>>>>>>>>>>          before completing cooling registration.
>>>>>>>>>>>          The drawback here is that if the parent remoteproc subsystem is
>>>>>>>>>>> disabled, the cooling device will still undergo an
>>>>>>>>>>>          unnecessary probe, even though it cannot be registered.
>>>>>>>>>> Bjorns question was different. It wasn't about pushing cooling device
>>>>>>>>>> outside of the remoteproc node. It is about not registering the devices.
>>>>>>>>>>
>>>>>>>>>> Can we follow the approach outlined by qcom_add_smd_subdev() /
>>>>>>>>>> qcom_add_glink_subdev()?
>>>>>>>>> Hi Dmitry,
>>>>>>>>>
>>>>>>>>> Thanks for the review. Since the remoteproc cooling is a QMI-based driver,
>>>>>>>>> it will receive the
>>>>>>>>> subsystem up notification directly. Therefore, there’s no need to make it a
>>>>>>>>> subdev node or
>>>>>>>>> tie it into the init/reset sequence of remoteproc subsytem.
>>>>>>>> But you've added a subnode for it (and we are discussing exactly
>>>>>>>> of_platform_populate()) call. So, you are tying it to the remoteproc
>>>>>>>> device lifecycle instead of the remoteproc subsys, which seems strange
>>>>>>>> to me. There is no cooling device if the DSP is not running.
>>>>>>>
>>>>>>> For the cooling feature, we don’t need to define it as a subnode. The cooling subsystem becomes relevant only
>>>>>>> after the remote subsystem is up, at which point it will receive add/delete notifications from the QMI server.
>>>>>>>
>>>>>>>
>>>>>>> If child nodes must be modeled as subnodes for rproc, we can move the CDSP TMD out of the remoteproc and add in soc.
>>>>>>> Is there currently a way for the remoteproc core layer to call of_platform_populate() without requiring a subnode?
>>>>>> I think the question is "why can't you register the remoteproc device
>>>>>> as a cooling_device, with perhaps #cooling-cells = <1>; instead of
>>>>>> any form of children?"
>>>>>>
>>>>>> Konrad
>>>>>
>>>>>
>>>>> thanks Konrad, for the review.
>>>>>
>>>>> As each subsystem can expose multiple thermal mitigation devices via the remoteproc TMD service, so need to define child node.
>>>>
>>>> I think you're stuck in an XY problem - you keep insisting that adding
>>>> a subnode is your end goal, while you really want to achieve being able
>>>> to register multiple cooling devices. Or at least that's how I read your
>>>> messages since you happen not to give any explanation as to why it's
>>>> actually necessary.
>>>>
>>>> In my previous message, I forgot that cells for cooling devices actually
>>>> represent the minimum and maximum cooling state allowed. But since the
>>>> API is just part of the kernel, there's nothing preventing us from
>>>> evolving it.
>>>>
>>>> Currently, we have:
>>>>
>>>> Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
>>>>
>>>> properties:
>>>>     "#cooling-cells":
>>>>       description:
>>>>         Must be 2, in order to specify minimum and maximum cooling state used in
>>>>         the cooling-maps reference. The first cell is the minimum cooling state
>>>>         and the second cell is the maximum cooling state requested.
>>>>       const: 2
>>>>
>>>> But I think it would be perfectly fine to suggest a change such that
>>>> if cells > 2, the last two cells keep the current behavior and the former
>>>> ones let you index into a cooling device exposed through a single OF node
>>>
>>> This might be a big change, which probably needs to be coordinated with
>>> thermal and DT maintainers first.
>>>
>>>>
>>>> e.g.
>>>>
>>>> rproc_xyz: remoteproc {
>>>>       compatible = "qcom,rproc-xyz";
>>>>
>>>>       ...
>>>>
>>>>       #cooling-device-cells = <3>;
>>>> };
>>>
>>> Which brings in another topic. In DT we have labels for different DT
>>> children, which correspond to different handlers on the DSP side. For
>>> the CDSP we see a "cdsp_sw" only. I think I've asked several times, but
>>> didn't get an example of the device having more than one, just claims
>>> that there might be more thane one TMD.
>>>
>>> Do we need different cooling cells here? Or would it be enough to send
>>> the same max state to all TMDs on the DSP side?
>>>
>>
>> For newer targets, Within the CDSP we have compute core(cdsp-sw),
>> npu(hmx-sw) core and both have independent dcvs and also dedicated
>> tsens on each core.
> 
> Okay, inside the DT bindings please provide this system as an example
> rather than using the simplest one.
> 
>>
>> And For Modem also we have multiple mitigation devices based on
>> different modem tech, for e.g tech level side we have modem-lte,
>> modem-nr etc and mitigation at different power amplifier side like
>> modem-pa etc. We have not added modem node for current series target
>> as it does not support modem.
> 
> I wrote it several times: there is no reason to limit yourself to
> CDSP. Please also add support for ADSP, SLPI and modem.
> 

Sure, will add.

>>
>>>>
>>>> ...
>>>>
>>>> thermal-zones {
>>>>       super-rproc-therm-a {
>>>>               thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_A
>>>>                                  THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>>>
>>>>               trips { ... } ;
>>>>       };
>>>>
>>>>       super-rproc-therm-b {
>>>>               thermal-sensors = <&rproc_xyz RPROC_XYZ_COOLING_B
>>>>                                  THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>>>
>>>>               trips { ... } ;
>>>>       };
>>>> };
>>>>
>>>> This would be resolved by allowing drivers to register an .of_xlate-type
>>>> function which would take the RPROC_XYZ_COOLING_n argument and e.g. use
>>>> it as an index into struct thermal_cooling_device cdev[]; within the
>>>> driver struct.
>>>
>>>
>>>
>>>>
>>>> Konrad
>>>
>>
> 
>

Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices

Posted by Dmitry Baryshkov 1 month, 2 weeks ago

On Tue, Dec 23, 2025 at 06:02:21PM +0530, Gaurav Kohli wrote:
> From: Casey Connolly <casey.connolly@linaro.org>
> 
> Generalise the qcom,bam-dmux child node support by probing all
> remoteproc children with of_platform_populate(). This will be used to
> enable support for devices which are best represented as subnodes of the
> remoteproc, such as those representing QMI clients.

Bindings first.

> 
> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> ---
>  drivers/remoteproc/qcom_q6v5.c     | 4 ++++
>  drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> index 58d5b85e58cd..a02839c7ed8c 100644
> --- a/drivers/remoteproc/qcom_q6v5.c
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -6,6 +6,7 @@
>   * Copyright (C) 2014 Sony Mobile Communications AB
>   * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
>   */
> +#include <linux/of_platform.h>
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  #include <linux/interconnect.h>
> @@ -351,6 +352,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>  		return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
>  				     "failed to acquire interconnect path\n");
>  
> +	of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);

Missing error handling. Also please use devm_of_platform_populate().

> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(qcom_q6v5_init);
> @@ -361,6 +364,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_init);
>   */
>  void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5)
>  {
> +	of_platform_depopulate(q6v5->dev);
>  	qmp_put(q6v5->qmp);
>  }
>  EXPORT_SYMBOL_GPL(qcom_q6v5_deinit);
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 91940977ca89..d40565c1cc62 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -238,7 +238,6 @@ struct q6v5 {
>  	struct qcom_rproc_pdm pdm_subdev;
>  	struct qcom_rproc_ssr ssr_subdev;
>  	struct qcom_sysmon *sysmon;
> -	struct platform_device *bam_dmux;
>  	bool need_mem_protection;
>  	bool has_alt_reset;
>  	bool has_mba_logs;
> @@ -2029,7 +2028,6 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
>  static int q6v5_probe(struct platform_device *pdev)
>  {
>  	const struct rproc_hexagon_res *desc;
> -	struct device_node *node;
>  	struct q6v5 *qproc;
>  	struct rproc *rproc;
>  	const char *mba_image;
> @@ -2163,10 +2161,6 @@ static int q6v5_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto remove_sysmon_subdev;
>  
> -	node = of_get_compatible_child(pdev->dev.of_node, "qcom,bam-dmux");
> -	qproc->bam_dmux = of_platform_device_create(node, NULL, &pdev->dev);
> -	of_node_put(node);
> -
>  	return 0;
>  
>  remove_sysmon_subdev:
> @@ -2186,8 +2180,6 @@ static void q6v5_remove(struct platform_device *pdev)
>  	struct q6v5 *qproc = platform_get_drvdata(pdev);
>  	struct rproc *rproc = qproc->rproc;
>  
> -	if (qproc->bam_dmux)
> -		of_platform_device_destroy(&qproc->bam_dmux->dev, NULL);
>  	rproc_del(rproc);
>  
>  	qcom_q6v5_deinit(&qproc->q6v5);
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry