Messages in this thread | | | Subject | Re: [Patch v2 1/3] thermal: qcom: lmh: Add support for sm8150 | From | Thara Gopinath <> | Date | Thu, 6 Jan 2022 10:52:49 -0500 |
| |
On 1/5/22 4:52 PM, Bjorn Andersson wrote: > On Wed 15 Dec 08:33 PST 2021, 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) >> + >> 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); > > I think it would be preferable to use of_device_get_match_data() and > assign this to an unsigned long.
Sure.. I will fix this and the comment below and send out v3.
-- Warm Regards Thara (She/Her/Hers) > >> + >> + if (of_id && of_id->data == LMH_ENABLE_ALGOS) { > > Then you don't need to check of_id for NULL here and this would lend > itself nicely to be a bitmask of enabled algorithms if some platform > would need to enable a subset of these. > >> + 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}, > > Make LMH_ENABLE_ALGOS just an integer define and add the explicit (void > *) cast here. > > Regards, > Bjorn > >> + { .compatible = "qcom,sm8150-lmh", }, >> {} >> }; >> MODULE_DEVICE_TABLE(of, lmh_table); >> -- >> 2.25.1 >>
| |