Re: [PATCH v4 3/4] thermal/qcom/lmh: support SDM670 and its CPU clusters

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 3/30/26 12:59 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 30, 2026 at 12:32:29PM +0200, Konrad Dybcio wrote:
>> On 3/29/26 12:44 PM, Dmitry Baryshkov wrote:
>>> On Fri, Mar 27, 2026 at 09:40:40PM -0400, Richard Acayan wrote:
>>>> The LMh driver was made for Qualcomm SoCs with clusters of 4 CPUs, but
>>>> some SoCs divide the CPUs into different sizes of clusters. In SDM670,
>>>> the first 6 CPUs are in the little cluster and the next 2 are in the big
>>>> cluster. Define the clusters in the match data and define the different
>>>> cluster configuration for SDM670.
>>>>
>>>> Currently, this only supports 8 CPUs and tolerates linking to any CPU in
>>>> the cluster.
>>>>
>>>> Signed-off-by: Richard Acayan <mailingradian@gmail.com>
>>>> ---
>>>>  drivers/thermal/qcom/lmh.c | 69 +++++++++++++++++++++++++++++++-------
>>>>  1 file changed, 56 insertions(+), 13 deletions(-)
>>>>
>>>> +static const struct lmh_soc_data sdm670_lmh_data = {
>>>> +	.enable_algos = true,
>>>> +	.node_ids = {
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +	},
>>>> +};
>>>> +
>>>> +static const struct lmh_soc_data sdm845_lmh_data = {
>>>> +	.enable_algos = true,
>>>> +	.node_ids = {
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +	},
>>>> +};
>>>
>>> These tables made me wonder, can we determine this information from the
>>> DT? For example, by reading the qcom,freq-domain property. But...
>>>
>>>> +
>>>> +static const struct lmh_soc_data sm8150_lmh_data = {
>>>> +	.enable_algos = false,
>>>> +	.node_ids = {
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER0_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +		LMH_CLUSTER1_NODE_ID,
>>>> +	},
>>>> +};
>>>
>>> ... this might be problematic, unless this entry is broken. On SM8150 we
>>> have three freq domains, but up to now we were programming two clustern
>>> nodes. Of course it is possible to define that node_id is 0 for freq
>>> domain 0 and 1 for domains 1 and 2.
>>
>> The third cluster situation on 8150 is not super good - we e.g. only have
>> a single LMH irq that's shared between the big and prime cores. That
>> was fixed with later SoCs (which is why it's not wired up in the DT today)
> 
> Thanks!
> 
> Anyway, from your point of view, would it be better to define mappings
> in the driver (like it's done with this patch) or parse the DT?

Well, we can spend a lot of time trying to be smart about it and handle
the odd edge case, or add a simple comparison!

Konrad