Message-ID: <dfdb2f4f-9c94-4d44-8dd7-67e8f4e025a4@oss.qualcomm.com>
Date: Sat, 31 Jan 2026 17:15:57 +0530
From: Gaurav Kohli <gaurav.kohli@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Bjorn Andersson <andersson@...nel.org>, mathieu.poirier@...aro.org,
        robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
        rafael@...nel.org, daniel.lezcano@...aro.org, rui.zhang@...el.com,
        lukasz.luba@....com, konradybcio@...nel.org, amitk@...nel.org,
        mani@...nel.org, casey.connolly@...aro.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        manaf.pallikunhi@....qualcomm.com
Subject: Re: [PATCH v1 2/8] remoteproc: qcom: probe all child devices



On 1/31/2026 5:10 PM, Dmitry Baryshkov wrote:
> On Sat, 31 Jan 2026 at 11:12, Gaurav Kohli
> <gaurav.kohli@....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@...aro.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@...aro.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
>>>
>>
> 
> 

Powered by blists - more mailing lists