Subject: Re: [Patch v2 1/3] thermal: qcom: lmh: Add support for sm8150
To:     Daniel Lezcano <daniel.lezcano@linaro.org>, agross@kernel.org,
        bjorn.andersson@linaro.org, rafael@kernel.org, rui.zhang@intel.com,
        robh+dt@kernel.org
Cc:     linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20211215163400.33349-1-thara.gopinath@linaro.org>
 <20211215163400.33349-2-thara.gopinath@linaro.org>
 <62f98846-0126-86ee-9bbd-f7abd03a4791@linaro.org>
From:   Thara Gopinath <thara.gopinath@linaro.org>
Message-ID: <f5eae932-8f8a-6c7f-b953-0d4da99a0d17@linaro.org>
Date:   Wed, 5 Jan 2022 14:18:05 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <62f98846-0126-86ee-9bbd-f7abd03a4791@linaro.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/20/21 7:04 AM, Daniel Lezcano wrote:
> On 15/12/2021 17:33, Thara Gopinath wrote:
>> Add compatible to support LMh for sm8150 SoC.
>> sm8150 does not require explicit enabling for various LMh subsystems.
>> Add a variable indicating the same as match data which is set for sdm845.
>> Execute the piece of code enabling various LMh subsystems only if
>> enable algorithm match data is present.
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>> ---
>>
>> v1->v2:
>> 	- Added LMH_ENABLE_ALGOS of_device_id match data to indicate
>> 	  whether LMh subsytems need explicit enabling or not.
>>
>>   drivers/thermal/qcom/lmh.c | 62 +++++++++++++++++++++-----------------
>>   1 file changed, 35 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/thermal/qcom/lmh.c b/drivers/thermal/qcom/lmh.c
>> index eafa7526eb8b..80d26d043498 100644
>> --- a/drivers/thermal/qcom/lmh.c
>> +++ b/drivers/thermal/qcom/lmh.c
>> @@ -28,6 +28,8 @@
>>   
>>   #define LMH_REG_DCVS_INTR_CLR		0x8
>>   
>> +#define LMH_ENABLE_ALGOS		((void *)1)
> 
> It will be nicer a probe function here

Hello Daniel,

As we discussed, there are SoCs for which all the algorithms need not be 
enabled. So introducing a separate probe function for each will be 
clumsy. The idea here is to use flags (currently just one to specify 
whether the algorithms need to be enabled or not) to specify which 
algorithms to be enabled.


-- 
Warm Regards
Thara (She/Her/Hers)
> 
>> +
>>   struct lmh_hw_data {
>>   	void __iomem *base;
>>   	struct irq_domain *domain;
>> @@ -87,6 +89,7 @@ static int lmh_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct device_node *np = dev->of_node;
>> +	const struct of_device_id *of_id;
>>   	struct device_node *cpu_node;
>>   	struct lmh_hw_data *lmh_data;
>>   	int temp_low, temp_high, temp_arm, cpu_id, ret;
>> @@ -141,32 +144,36 @@ static int lmh_probe(struct platform_device *pdev)
>>   	if (!qcom_scm_lmh_dcvsh_available())
>>   		return -EINVAL;
>>   
>> -	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1,
>> -				 LMH_NODE_DCVS, node_id, 0);
>> -	if (ret)
>> -		dev_err(dev, "Error %d enabling current subfunction\n", ret);
>> -
>> -	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1,
>> -				 LMH_NODE_DCVS, node_id, 0);
>> -	if (ret)
>> -		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
>> -
>> -	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1,
>> -				 LMH_NODE_DCVS, node_id, 0);
>> -	if (ret)
>> -		dev_err(dev, "Error %d enabling BCL subfunction\n", ret);
>> -
>> -	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1,
>> -				 LMH_NODE_DCVS, node_id, 0);
>> -	if (ret) {
>> -		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
>> -		return ret;
>> -	}
>> -
>> -	ret = qcom_scm_lmh_profile_change(0x1);
>> -	if (ret) {
>> -		dev_err(dev, "Error %d changing profile\n", ret);
>> -		return ret;
>> +	of_id = of_match_device(dev->driver->of_match_table, dev);
>> +
>> +	if (of_id && of_id->data == LMH_ENABLE_ALGOS) {
>> +		ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1,
>> +					 LMH_NODE_DCVS, node_id, 0);
>> +		if (ret)
>> +			dev_err(dev, "Error %d enabling current subfunction\n", ret);
>> +
>> +		ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1,
>> +					 LMH_NODE_DCVS, node_id, 0);
>> +		if (ret)
>> +			dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
>> +
>> +		ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1,
>> +					 LMH_NODE_DCVS, node_id, 0);
>> +		if (ret)
>> +			dev_err(dev, "Error %d enabling BCL subfunction\n", ret);
>> +
>> +		ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1,
>> +					 LMH_NODE_DCVS, node_id, 0);
>> +		if (ret) {
>> +			dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
>> +			return ret;
>> +		}
>> +
>> +		ret = qcom_scm_lmh_profile_change(0x1);
>> +		if (ret) {
>> +			dev_err(dev, "Error %d changing profile\n", ret);
>> +			return ret;
>> +		}
>>   	}
>>   
>>   	/* Set default thermal trips */
>> @@ -213,7 +220,8 @@ static int lmh_probe(struct platform_device *pdev)
>>   }
>>   
>>   static const struct of_device_id lmh_table[] = {
>> -	{ .compatible = "qcom,sdm845-lmh", },
>> +	{ .compatible = "qcom,sdm845-lmh", .data = LMH_ENABLE_ALGOS},
>> +	{ .compatible = "qcom,sm8150-lmh", },
>>   	{}
>>   };
>>   MODULE_DEVICE_TABLE(of, lmh_table);
>>
> 
>