Re: [PATCH v3 1/8] dt-bindings: remoteproc: qcom,pas: add thermal mitigation properties

[Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>]

On 6/16/26 00:15, Dmitry Baryshkov wrote:
> On Mon, Jun 15, 2026 at 05:33:15PM +0200, Daniel Lezcano wrote:
>>
>>
>> Le 15/06/2026 à 17:14, Dmitry Baryshkov a écrit :
>>> On Mon, Jun 15, 2026 at 04:33:38PM +0200, Daniel Lezcano wrote:
>>>>
>>>>
>>>> Le 15/06/2026 à 16:11, Dmitry Baryshkov a écrit :
>>>>> On Mon, Jun 15, 2026 at 02:30:49PM +0200, Daniel Lezcano wrote:
>>>>>> Hi Gaurav,
>>>>>>
>>>>>> Le 15/06/2026 à 14:12, Gaurav Kohli a écrit :
>>>>>>>
>>>>>>>
>>>>>>> On 6/15/2026 4:04 PM, Daniel Lezcano wrote:
>>>>>>>> On 6/13/26 13:05, Gaurav Kohli wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 6/13/2026 1:11 PM, Krzysztof Kozlowski wrote:
>>>>>>>>>> On 12/06/2026 15:52, Gaurav Kohli wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 6/11/2026 5:53 PM, Krzysztof Kozlowski wrote:
>>>>>>>>>>>> On 11/06/2026 13:12, Gaurav Kohli wrote:
>>>>>>>>>>>>>> Why? And where is this generic property defined? You cannot just
>>>>>>>>>>>>>> sprinkle generic properties in random bindings.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Ack, will add why part.
>>>>>>>>>>>>> These names are matched with the thermal
>>>>>>>>>>>>> mitigation device identifiers
>>>>>>>>>>>>> populated by remote firmware over QMI and define
>>>>>>>>>>>>> mitigation devices are
>>>>>>>>>>>>> exposed as cooling devices.
>>>>>>>>>>>>
>>>>>>>>>>>> No, -names correspond to values passed via DT, not
>>>>>>>>>>>> some remote firmware.
>>>>>>>>>>>> The remote firmware should give you interface which
>>>>>>>>>>>> is explicit and does
>>>>>>>>>>>> not need such properties.
>>>>>>>>>>>
>>>>>>>>>>> thanks Krzysztof for review, We need tmd-names because
>>>>>>>>>>> of following reasons:
>>>>>>>>>>>
>>>>>>>>>>> Following Daniel's series [1], the thermal framework supports
>>>>>>>>>>> mapping multiple cooling devices per remoteproc/device via indexed
>>>>>>>>>>> cooling-cells.
>>>>>>>>>>>
>>>>>>>>>>> 1) The thermal framework's cooling-maps reference
>>>>>>>>>>> cooling devices by index (for #cooling-cells = <3>).
>>>>>>>>>>> Without tmd- names,
>>>>>>>>>>> there's no way to know which index corresponds to which
>>>>>>>>>>> TMD, as firmware
>>>>>>>>>>> may return tmd-names in any order.
>>>>>>>>>>>
>>>>>>>>>>> below are the changes post new thermal mapping changes:
>>>>>>>>>>> DT: tmd-names = "cdsp_sw", "xyz";
>>>>>>>>>>> Firmware: ["cdsp_sw", "xyz1", "xyz2",]
>>>>>>>>>>> Driver registers: Only "cdsp_sw" (index 0) and "xyz" (index 1)
>>>>>>>>>>
>>>>>>>>>> names property are not to instruct drivers to register or not to
>>>>>>>>>> register something.
>>>>>>>>>>
>>>>>>>>>> I don't understand the problem and explanation in the binding is
>>>>>>>>>> basically non-existing.
>>>>>>>>>>
>>>>>>>>>> Remember that all lists and indices ARE FIXED, so driver knows exactly
>>>>>>>>>> which index means what.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> thanks for review, shall i use driver data, which is basically
>>>>>>>>> pas data structure like below:
>>>>>>>>>
>>>>>>>>> static const struct qcom_pas_data {
>>>>>>>>>         .crash_reason_smem = 601,
>>>>>>>>>         .firmware_name = "cdsp.mdt",
>>>>>>>>>         .tmd_names = (const char *[]){"xyz", NULL},
>>>>>>>>>         .num_tmds = 1,
>>>>>>>>>
>>>>>>>>> Is something like above acceptable? and this will also help to
>>>>>>>>> filter tmd names as well?
>>>>>>>>
>>>>>>>>
>>>>>>>> How the thermal framework will bind the thermal zone with the TMD ?
>>>>>>>> (node pointer, id) ?
>>>>>>>>
>>>>>>>
>>>>>>> Hi Daniel,
>>>>>>>
>>>>>>> thanks for review.
>>>>>>>
>>>>>>> With id only, in this case instead of taking tmd names from device tree,
>>>>>>> qmi_tmd will take tmd name from pas_data(driver) and register with the
>>>>>>> cooling framework with id only. Please let us know if this looks fine.
>>>>>> May be I'm missing something but:
>>>>>>
>>>>>>     - The QMI TMD returns a list of names, not ids
>>>>>>     - The QMI TMD may return the list in different order than assumed
>>>>>>     - The cooling map index points to the name of the TMD in the DT
>>>>>>     - This name is used to match the name in the aformentionned list
>>>>>>     - The index in the list and the id in the DT can differ
>>>>>
>>>>> Would it be better if we define standard indices for the standard names?
>>>>> This way we decouple the actual firmware strings from the DT.
>>>>
>>>> I don't think so, it seems to me too fragile and prone to error.
>>>>
>>>> It is a remote proc, an external subsystem. The contract between the client
>>>> and the server is the protocol. The protocol specifies the identifier as
>>>> named strings, the TMD names, not numerical identifiers.
>>>>
>>>> When asking for the list of TMDs, we get a list of strings. But as it is an
>>>> external subsystems, may be tomorrow someone decide to send list ordered
>>>> alphabetically, or per number of states, or whatever.
>>>>
>>>> With hardcoded id the QMI TMD clients break
>>>
>>> I was thinking about something like:
>>>
>>> #define QCOM_TMD_DSP	0
>>> #define QCOM_TMD_PA	1
>>
>> Ah ok, it is correct if:
>>
>> tmd-names = "dsp", "pa"
>>
>> Or
>>
>> #define QCOM_TMD_PA	0
>> #define QCOM_TMD_DSP	1
>>
>> tmd-names = "pa", "dsp"
> 
> I was more inclined about having the standard indices for the standard
> mitigations.
> 
> BTW, I checked, which mitigations are being returned by the DSPs. Few
> examples, just to provide some context.

I don't know if you are missing my point or if I'm missing yours :)

The QMI TMD protocol identifies the TMD with strings. There is no 
guarantee the ordering is kept if there is a firmware upgrade.

The ID is to connect the cooling map with the remote proc node index. 
Its only a thermal description not related to the TMD itself.

> SC8280XP, X13s:
> 
> TMD service: instance=0x01 (adsp) node=5 port=9
>    1 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
> 
> TMD service: instance=0x53 (slpi) node=9 port=9
>    1 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
> 
> TMD service: instance=0x43 (cdsp) node=10 port=8
>    3 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
>      [ 1] cdsp_hw                  max_mitigation_level=1
>      [ 2] cdsp_sw                  max_mitigation_level=7
> 
> SM6115, RB2:
> 
> TMD service: instance=0x00 (modem) node=0 port=20
>    9 mitigation device(s):
>      [ 0] pa                       max_mitigation_level=3
>      [ 1] modem                    max_mitigation_level=3
>      [ 2] cpuv_restriction_cold    max_mitigation_level=1
>      [ 3] modem_current            max_mitigation_level=3
>      [ 4] vbatt_low                max_mitigation_level=3
>      [ 5] modem_skin               max_mitigation_level=3
>      [ 6] modem_bw                 max_mitigation_level=5
>      [ 7] wlan                     max_mitigation_level=1
>      [ 8] wlan_bw                  max_mitigation_level=1
> 
> TMD service: instance=0x01 (adsp) node=5 port=8
>    1 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
> 
> TMD service: instance=0x43 (cdsp) node=10 port=8
>    3 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
>      [ 1] cdsp_hw                  max_mitigation_level=1
>      [ 2] cdsp_sw                  max_mitigation_level=5
> 
> 
> SM8350, HDK:
> 
> TMD service: instance=0x00 (modem) node=0 port=22
>    28 mitigation device(s):
>      [ 0] pa                       max_mitigation_level=3
>      [ 1] pa_fr1                   max_mitigation_level=3
>      [ 2] modem                    max_mitigation_level=3
>      [ 3] cpuv_restriction_cold    max_mitigation_level=1
>      [ 4] modem_current            max_mitigation_level=3
>      [ 5] vbatt_low                max_mitigation_level=3
>      [ 6] charge_state             max_mitigation_level=3
>      [ 7] modem_skin               max_mitigation_level=3
>      [ 8] modem_bw                 max_mitigation_level=5
>      [ 9] mmw0                     max_mitigation_level=3
>      [10] mmw1                     max_mitigation_level=3
>      [11] mmw2                     max_mitigation_level=3
>      [12] mmw3                     max_mitigation_level=3
>      [13] mmw_skin0                max_mitigation_level=3
>      [14] mmw_skin1                max_mitigation_level=3
>      [15] mmw_skin2                max_mitigation_level=3
>      [16] mmw_skin3                max_mitigation_level=3
>      [17] mmw_skin0_dsc            max_mitigation_level=15
>      [18] mmw_skin1_dsc            max_mitigation_level=15
>      [19] mmw_skin2_dsc            max_mitigation_level=15
>      [20] mmw_skin3_dsc            max_mitigation_level=15
>      [21] wlan                     max_mitigation_level=4
>      [22] wlan_bw                  max_mitigation_level=1
>      [23] modem_skin_lte_dsc       max_mitigation_level=255
>      [24] modem_skin_nr_dsc        max_mitigation_level=255
>      [25] pa_dsc                   max_mitigation_level=255
>      [26] pa_fr1_dsc               max_mitigation_level=255
>      [27] cpr_cold                 max_mitigation_level=3
> 
> TMD service: instance=0x01 (adsp) node=5 port=9
>    1 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
> 
> TMD service: instance=0x43 (cdsp) node=10 port=9
>    3 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
>      [ 1] cdsp_hw                  max_mitigation_level=1
>      [ 2] cdsp_sw                  max_mitigation_level=7
> 
> SM8150, HDK:
> 
> TMD service: instance=0x00 (modem) node=0 port=21
>    6 mitigation device(s):
>      [ 0] pa                       max_mitigation_level=3
>      [ 1] modem                    max_mitigation_level=3
>      [ 2] cpuv_restriction_cold    max_mitigation_level=1
>      [ 3] modem_current            max_mitigation_level=3
>      [ 4] vbatt_low                max_mitigation_level=3
>      [ 5] modem_skin               max_mitigation_level=3
> 
> TMD service: instance=0x01 node=5 port=8
>    1 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
> 
> TMD service: instance=0x53 node=9 port=8
>    1 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
> 
> TMD service: instance=0x43 (cdsp) node=10 port=8
>    1 mitigation device(s):
>      [ 0] cpuv_restriction_cold    max_mitigation_level=1
> 
>