2019-09-12 14:18:12

by Jorge Ramirez-Ortiz

Subject: [PATCH v2 5/5] clk: qcom: apcs-msm8916: get parent clock names from DT

Allow accessing the parent clock names required for the driver
operation by using the device tree node.

This permits extending the driver to other platforms without having to
modify its source code.

For backwards compatibility leave previous values as default.

Co-developed-by: Niklas Cassel <[email protected]>
Signed-off-by: Niklas Cassel <[email protected]>
Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
---
drivers/clk/qcom/apcs-msm8916.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/apcs-msm8916.c b/drivers/clk/qcom/apcs-msm8916.c
index a6c89a310b18..099b028dbc20 100644
--- a/drivers/clk/qcom/apcs-msm8916.c
+++ b/drivers/clk/qcom/apcs-msm8916.c
@@ -19,7 +19,7 @@

static const u32 gpll0_a53cc_map[] = { 4, 5 };

-static const char * const gpll0_a53cc[] = {
+static const char *gpll0_a53cc[] = {
"gpll0_vote",
"a53pll",
};
@@ -50,6 +50,7 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
struct regmap *regmap;
struct clk_init_data init = { };
int ret = -ENODEV;
+ const char *parents[2];

regmap = dev_get_regmap(parent, NULL);
if (!regmap) {
@@ -61,6 +62,9 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
if (!a53cc)
return -ENOMEM;

+ if (of_clk_parent_fill(parent->of_node, parents, 2) == 2)
+ memcpy(gpll0_a53cc, parents, sizeof(parents));
+
init.name = "a53mux";
init.parent_names = gpll0_a53cc;
init.num_parents = ARRAY_SIZE(gpll0_a53cc);
@@ -76,10 +80,11 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
a53cc->src_shift = 8;
a53cc->parent_map = gpll0_a53cc_map;

- a53cc->pclk = devm_clk_get(parent, NULL);
+ a53cc->pclk = of_clk_get(parent->of_node, 0);
if (IS_ERR(a53cc->pclk)) {
ret = PTR_ERR(a53cc->pclk);
- dev_err(dev, "failed to get clk: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "failed to get clk: %d\n", ret);
return ret;
}

@@ -87,6 +92,7 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
ret = clk_notifier_register(a53cc->pclk, &a53cc->clk_nb);
if (ret) {
dev_err(dev, "failed to register clock notifier: %d\n", ret);
+ clk_put(a53cc->pclk);
return ret;
}

@@ -109,6 +115,8 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)

err:
clk_notifier_unregister(a53cc->pclk, &a53cc->clk_nb);
+ clk_put(a53cc->pclk);
+
return ret;
}

@@ -117,6 +125,7 @@ static int qcom_apcs_msm8916_clk_remove(struct platform_device *pdev)
struct clk_regmap_mux_div *a53cc = platform_get_drvdata(pdev);

clk_notifier_unregister(a53cc->pclk, &a53cc->clk_nb);
+ clk_put(a53cc->pclk);

return 0;
}
--
2.23.0

2019-11-07 22:20:40

by Stephen Boyd

Subject: Re: [PATCH v2 5/5] clk: qcom: apcs-msm8916: get parent clock names from DT

Quoting Jorge Ramirez-Ortiz (2019-09-12 07:15:34)
> @@ -61,6 +62,9 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
> if (!a53cc)
> return -ENOMEM;
>
> + if (of_clk_parent_fill(parent->of_node, parents, 2) == 2)
> + memcpy(gpll0_a53cc, parents, sizeof(parents));
> +
> init.name = "a53mux";
> init.parent_names = gpll0_a53cc;
> init.num_parents = ARRAY_SIZE(gpll0_a53cc);

Why can't we use new way of specifying parents in this driver too?

> @@ -76,10 +80,11 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
> a53cc->src_shift = 8;
> a53cc->parent_map = gpll0_a53cc_map;
>
> - a53cc->pclk = devm_clk_get(parent, NULL);
> + a53cc->pclk = of_clk_get(parent->of_node, 0);

And then leave this one alone? clk_get() with a NULL id should use a DT
index of 0 from what I can tell.

> if (IS_ERR(a53cc->pclk)) {
> ret = PTR_ERR(a53cc->pclk);
> - dev_err(dev, "failed to get clk: %d\n", ret);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "failed to get clk: %d\n", ret);
> return ret;
> }
>