Message-ID: <CAO9ioeVxbAJR-JSVSVOgtZaTmwqaez=2K5Rfxo=+swUJ=YmN3A@mail.gmail.com>
Date: Sat, 31 Jan 2026 13:40:14 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Gaurav Kohli <gaurav.kohli@....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 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.

>
> >>
> >> ...
> >>
> >> 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

Powered by blists - more mailing lists