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

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

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"

> cooling-maps {
> 	map0 {
> 		cooling-device = <&remoteproc_cdsp QCOM_TMD_DSP 0 2>;
> 	};
> 	map1 {
> 		cooling-device = <&remoteproc_mpss QCOM_TMD_DSP 0 2>;
> 	};
> 	map2 {
> 		cooling-device = <&remoteproc_mpss QCOM_TMD_PA 0 2>;
> 	};
> };
> 
> 
>>
>>>> Krzysztof , I don't get why having the TMD names as properties is wrong,
>>>> they describes the existing TMDs on the system and the cooling maps index
>>>> points to the one to be connected with thermal zone.
>>>
>>
>