Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1511490 > unrolled thread

[PATCH 0/3] Remove clocks dependency from SCM driver

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] Remove clocks dependency from SCM driver Sarangdhar Joshi <spjoshi@codeaurora.org> - 2016-10-29 02:10 +0200
    [PATCH 3/3] firmware: qcom: scm: Return PTR_ERR when devm_clk_get fails Sarangdhar Joshi <spjoshi@codeaurora.org> - 2016-10-29 02:10 +0200
    [PATCH 2/3] firmware: qcom: scm: Remove core, iface and bus clocks dependency Sarangdhar Joshi <spjoshi@codeaurora.org> - 2016-10-29 02:20 +0200
      Re: [PATCH 2/3] firmware: qcom: scm: Remove core, iface and bus  clocks dependency Stephen Boyd <sboyd@codeaurora.org> - 2016-11-02 00:20 +0100
        Re: [PATCH 2/3] firmware: qcom: scm: Remove core, iface and bus  clocks dependency Sarangdhar Joshi <spjoshi@codeaurora.org> - 2016-11-02 20:30 +0100
          Re: [PATCH 2/3] firmware: qcom: scm: Remove core, iface and bus  clocks dependency Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-11-02 21:10 +0100

#1511490 — [PATCH 0/3] Remove clocks dependency from SCM driver

On earlier chiptsets (APQ8064, MSM8660, MSM8690, MSM8916,
APQ8084, MSM8974) crypto operations of TZ were depends on crypto
clocks controlled by users/clients. However on MSM8996 crypto clocks
control is handled internally in TZ itself. The current series of
patches handle this clock dependency in SCM driver.

Sarangdhar Joshi (3):
  dt-bindings: firmware: scm: Add MSM8996 DT bindings
  firmware: qcom: scm: Remove core, iface and bus clocks dependency
  firmware: qcom: scm: Return PTR_ERR when devm_clk_get fails

 .../devicetree/bindings/firmware/qcom,scm.txt      |  2 +
 drivers/firmware/qcom_scm.c                        | 65 +++++++++++++++-------
 2 files changed, 48 insertions(+), 19 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [next] | [standalone]

#1511491 — [PATCH 3/3] firmware: qcom: scm: Return PTR_ERR when devm_clk_get fails

When devm_clk_get fails for core clock, the failure was ignored
and the core_clk was explicitly set to NULL so that other
remaining clocks can be queried. However, now that we have a
clean way of expressing the clock dependency, return failure when
devm_clk_get fails for core clock.

Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
---
 drivers/firmware/qcom_scm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 010a350..5c472da 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -395,10 +395,10 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	if (clks & SCM_HAS_CORE_CLK) {
 		scm->core_clk = devm_clk_get(&pdev->dev, "core");
 		if (IS_ERR(scm->core_clk)) {
-			if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
-				return PTR_ERR(scm->core_clk);
-
-			scm->core_clk = NULL;
+			if (PTR_ERR(scm->core_clk) != -EPROBE_DEFER)
+				dev_err(&pdev->dev,
+					"failed to acquire core clk\n");
+			return PTR_ERR(scm->core_clk);
 		}
 	}
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1511492 — [PATCH 2/3] firmware: qcom: scm: Remove core, iface and bus clocks dependency

Core, iface and bus clocks are not required to be voted from SCM
driver for some of the Qualcomm chipsets. Remove dependency on
these clocks from driver.

Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
---
 drivers/firmware/qcom_scm.c | 65 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 19 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index d79fecd..010a350 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -28,6 +28,10 @@
 
 #include "qcom_scm.h"
 
+#define SCM_HAS_CORE_CLK	BIT(0)
+#define SCM_HAS_IFACE_CLK	BIT(1)
+#define SCM_HAS_BUS_CLK		BIT(2)
+
 struct qcom_scm {
 	struct device *dev;
 	struct clk *core_clk;
@@ -380,33 +384,43 @@ EXPORT_SYMBOL(qcom_scm_is_available);
 static int qcom_scm_probe(struct platform_device *pdev)
 {
 	struct qcom_scm *scm;
+	uint64_t clks;
 	int ret;
 
 	scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
 	if (!scm)
 		return -ENOMEM;
 
-	scm->core_clk = devm_clk_get(&pdev->dev, "core");
-	if (IS_ERR(scm->core_clk)) {
-		if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
-			return PTR_ERR(scm->core_clk);
+	clks = (uint64_t)of_device_get_match_data(&pdev->dev);
+	if (clks & SCM_HAS_CORE_CLK) {
+		scm->core_clk = devm_clk_get(&pdev->dev, "core");
+		if (IS_ERR(scm->core_clk)) {
+			if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
+				return PTR_ERR(scm->core_clk);
 
-		scm->core_clk = NULL;
+			scm->core_clk = NULL;
+		}
 	}
 
 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,scm")) {
-		scm->iface_clk = devm_clk_get(&pdev->dev, "iface");
-		if (IS_ERR(scm->iface_clk)) {
-			if (PTR_ERR(scm->iface_clk) != -EPROBE_DEFER)
-				dev_err(&pdev->dev, "failed to acquire iface clk\n");
-			return PTR_ERR(scm->iface_clk);
+		if (clks & SCM_HAS_IFACE_CLK) {
+			scm->iface_clk = devm_clk_get(&pdev->dev, "iface");
+			if (IS_ERR(scm->iface_clk)) {
+				if (PTR_ERR(scm->iface_clk) != -EPROBE_DEFER)
+					dev_err(&pdev->dev,
+						"failed to acquire iface clk\n");
+				return PTR_ERR(scm->iface_clk);
+			}
 		}
 
-		scm->bus_clk = devm_clk_get(&pdev->dev, "bus");
-		if (IS_ERR(scm->bus_clk)) {
-			if (PTR_ERR(scm->bus_clk) != -EPROBE_DEFER)
-				dev_err(&pdev->dev, "failed to acquire bus clk\n");
-			return PTR_ERR(scm->bus_clk);
+		if (clks & SCM_HAS_BUS_CLK) {
+			scm->bus_clk = devm_clk_get(&pdev->dev, "bus");
+			if (IS_ERR(scm->bus_clk)) {
+				if (PTR_ERR(scm->bus_clk) != -EPROBE_DEFER)
+					dev_err(&pdev->dev,
+						"failed to acquire bus clk\n");
+				return PTR_ERR(scm->bus_clk);
+			}
 		}
 	}
 
@@ -429,10 +443,23 @@ static int qcom_scm_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_scm_dt_match[] = {
-	{ .compatible = "qcom,scm-apq8064",},
-	{ .compatible = "qcom,scm-msm8660",},
-	{ .compatible = "qcom,scm-msm8960",},
-	{ .compatible = "qcom,scm",},
+	{ .compatible = "qcom,scm-apq8064",
+	  .data = (void *) SCM_HAS_CORE_CLK,
+	},
+	{ .compatible = "qcom,scm-msm8660",
+	  .data = (void *) SCM_HAS_CORE_CLK,
+	},
+	{ .compatible = "qcom,scm-msm8960",
+	  .data = (void *) SCM_HAS_CORE_CLK,
+	},
+	{ .compatible = "qcom,scm-msm8996",
+	  .data = NULL, /* no clocks */
+	},
+	{ .compatible = "qcom,scm",
+	  .data = (void *) ( SCM_HAS_CORE_CLK
+			    | SCM_HAS_IFACE_CLK
+			    | SCM_HAS_BUS_CLK ),
+	},
 	{}
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1513562 — Re: [PATCH 2/3] firmware: qcom: scm: Remove core, iface and bus clocks dependency

On 10/28, Sarangdhar Joshi wrote:
> @@ -380,33 +384,43 @@ EXPORT_SYMBOL(qcom_scm_is_available);
>  static int qcom_scm_probe(struct platform_device *pdev)
>  {
>  	struct qcom_scm *scm;
> +	uint64_t clks;
>  	int ret;
>  
>  	scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
>  	if (!scm)
>  		return -ENOMEM;
>  
> -	scm->core_clk = devm_clk_get(&pdev->dev, "core");
> -	if (IS_ERR(scm->core_clk)) {
> -		if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
> -			return PTR_ERR(scm->core_clk);
> +	clks = (uint64_t)of_device_get_match_data(&pdev->dev);
> +	if (clks & SCM_HAS_CORE_CLK) {
> +		scm->core_clk = devm_clk_get(&pdev->dev, "core");
> +		if (IS_ERR(scm->core_clk)) {
> +			if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
> +				return PTR_ERR(scm->core_clk);
>  
> -		scm->core_clk = NULL;
> +			scm->core_clk = NULL;
> +		}
>  	}
>  
>  	if (of_device_is_compatible(pdev->dev.of_node, "qcom,scm")) {

Why didn't this also get added to the flags feature? I'd prefer
we either use of_device_is_compatible() for everything, or device
data to figure out what quirks to apply.

> -		scm->iface_clk = devm_clk_get(&pdev->dev, "iface");
> -		if (IS_ERR(scm->iface_clk)) {
> -			if (PTR_ERR(scm->iface_clk) != -EPROBE_DEFER)

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1514094 — Re: [PATCH 2/3] firmware: qcom: scm: Remove core, iface and bus clocks dependency

On 11/01/2016 04:11 PM, Stephen Boyd wrote:
> On 10/28, Sarangdhar Joshi wrote:
>> @@ -380,33 +384,43 @@ EXPORT_SYMBOL(qcom_scm_is_available);
>>  static int qcom_scm_probe(struct platform_device *pdev)
>>  {
>>  	struct qcom_scm *scm;
>> +	uint64_t clks;
>>  	int ret;
>>
>>  	scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
>>  	if (!scm)
>>  		return -ENOMEM;
>>
>> -	scm->core_clk = devm_clk_get(&pdev->dev, "core");
>> -	if (IS_ERR(scm->core_clk)) {
>> -		if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
>> -			return PTR_ERR(scm->core_clk);
>> +	clks = (uint64_t)of_device_get_match_data(&pdev->dev);
>> +	if (clks & SCM_HAS_CORE_CLK) {
>> +		scm->core_clk = devm_clk_get(&pdev->dev, "core");
>> +		if (IS_ERR(scm->core_clk)) {
>> +			if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
>> +				return PTR_ERR(scm->core_clk);
>>
>> -		scm->core_clk = NULL;
>> +			scm->core_clk = NULL;
>> +		}
>>  	}
>>
>>  	if (of_device_is_compatible(pdev->dev.of_node, "qcom,scm")) {
>
> Why didn't this also get added to the flags feature? I'd prefer
> we either use of_device_is_compatible() for everything, or device
> data to figure out what quirks to apply.

You're right. These flags are already added for "qcom,scm" compatible. 
We can modify this to honor data flags only. I hope it's okay to update 
it in the same patch?

Regards,
Sarang

>
>> -		scm->iface_clk = devm_clk_get(&pdev->dev, "iface");
>> -		if (IS_ERR(scm->iface_clk)) {
>> -			if (PTR_ERR(scm->iface_clk) != -EPROBE_DEFER)
>


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1514102 — Re: [PATCH 2/3] firmware: qcom: scm: Remove core, iface and bus clocks dependency

On Wed 02 Nov 12:24 PDT 2016, Sarangdhar Joshi wrote:

> On 11/01/2016 04:11 PM, Stephen Boyd wrote:
> >On 10/28, Sarangdhar Joshi wrote:
> >>@@ -380,33 +384,43 @@ EXPORT_SYMBOL(qcom_scm_is_available);
> >> static int qcom_scm_probe(struct platform_device *pdev)
> >> {
> >> 	struct qcom_scm *scm;
> >>+	uint64_t clks;
> >> 	int ret;
> >>
> >> 	scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
> >> 	if (!scm)
> >> 		return -ENOMEM;
> >>
> >>-	scm->core_clk = devm_clk_get(&pdev->dev, "core");
> >>-	if (IS_ERR(scm->core_clk)) {
> >>-		if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
> >>-			return PTR_ERR(scm->core_clk);
> >>+	clks = (uint64_t)of_device_get_match_data(&pdev->dev);
> >>+	if (clks & SCM_HAS_CORE_CLK) {
> >>+		scm->core_clk = devm_clk_get(&pdev->dev, "core");
> >>+		if (IS_ERR(scm->core_clk)) {
> >>+			if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
> >>+				return PTR_ERR(scm->core_clk);
> >>
> >>-		scm->core_clk = NULL;
> >>+			scm->core_clk = NULL;
> >>+		}
> >> 	}
> >>
> >> 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,scm")) {
> >
> >Why didn't this also get added to the flags feature? I'd prefer
> >we either use of_device_is_compatible() for everything, or device
> >data to figure out what quirks to apply.
> 
> You're right. These flags are already added for "qcom,scm" compatible. We
> can modify this to honor data flags only. I hope it's okay to update it in
> the same patch?
> 

Yes, you're replacing the old conditional with a set of new ones, so it
should go in this patch.

Regards,
Bjorn

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web