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

Groups > linux.kernel > #1385498 > unrolled thread

[PATCH 2/8] firmware: qcom: scm: Convert SCM to platform driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/8] firmware: qcom: scm: Convert SCM to platform driver Andy Gross <andy.gross@linaro.org> - 2016-04-23 00:20 +0200
    Re: [PATCH 2/8] firmware: qcom: scm: Convert SCM to platform driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-04-23 01:20 +0200
      Re: [PATCH 2/8] firmware: qcom: scm: Convert SCM to platform driver Andy Gross <andy.gross@linaro.org> - 2016-04-23 06:50 +0200

#1385498 — [PATCH 2/8] firmware: qcom: scm: Convert SCM to platform driver

This patch converts the Qualcomm SCM firmware driver into a platform
driver.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 arch/arm64/Kconfig.platforms |   1 +
 drivers/firmware/qcom_scm.c  | 154 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 147 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index efa77c1..6f0876f 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -76,6 +76,7 @@ config ARCH_MVEBU
 config ARCH_QCOM
 	bool "Qualcomm Platforms"
 	select PINCTRL
+	select QCOM_SCM
 	help
 	  This enables support for the ARMv8 based Qualcomm chipsets.
 
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 45c008d..8e1eeb8 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -10,19 +10,68 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
  */
-
+#include <linux/platform_device.h>
+#include <linux/module.h>
 #include <linux/cpumask.h>
 #include <linux/export.h>
 #include <linux/types.h>
 #include <linux/qcom_scm.h>
+#include <linux/of.h>
+#include <linux/clk.h>
 
 #include "qcom_scm.h"
 
+struct qcom_scm {
+	struct device *dev;
+	struct clk *core_clk;
+	struct clk *iface_clk;
+	struct clk *bus_clk;
+};
+
+static struct qcom_scm *__scm;
+
+static int qcom_scm_clk_enable(void)
+{
+	int ret;
+
+	ret = clk_prepare_enable(__scm->core_clk);
+	if (ret)
+		goto bail;
+
+	if (__scm->iface_clk) {
+		ret = clk_prepare_enable(__scm->iface_clk);
+		if (ret)
+			goto disable_core;
+	}
+
+	if (__scm->bus_clk) {
+		ret = clk_prepare_enable(__scm->bus_clk);
+		if (ret)
+			goto disable_iface;
+	}
+
+	return 0;
+
+disable_iface:
+	if (__scm->iface_clk)
+		clk_disable_unprepare(__scm->iface_clk);
+disable_core:
+	if (__scm->bus_clk)
+		clk_disable_unprepare(__scm->core_clk);
+bail:
+	return ret;
+}
+
+static void qcom_scm_clk_disable(void)
+{
+	clk_disable_unprepare(__scm->core_clk);
+	if (__scm->iface_clk)
+		clk_disable_unprepare(__scm->iface_clk);
+	if (__scm->bus_clk)
+		clk_disable_unprepare(__scm->bus_clk);
+}
+
 /**
  * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
  * @entry: Entry point function for the cpus
@@ -72,11 +121,17 @@ EXPORT_SYMBOL(qcom_scm_cpu_power_down);
  */
 bool qcom_scm_hdcp_available(void)
 {
-	int ret;
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		goto clk_err;
 
 	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_HDCP,
-		QCOM_SCM_CMD_HDCP);
+						QCOM_SCM_CMD_HDCP);
+
+	qcom_scm_clk_disable();
 
+clk_err:
 	return (ret > 0) ? true : false;
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_available);
@@ -91,6 +146,89 @@ EXPORT_SYMBOL(qcom_scm_hdcp_available);
  */
 int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 {
-	return __qcom_scm_hdcp_req(req, req_cnt, resp);
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
+
+	ret = __qcom_scm_hdcp_req(req, req_cnt, resp);
+	qcom_scm_clk_disable();
+	return ret;
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_req);
+
+/**
+ * qcom_scm_is_available() - Checks if SCM is available
+ */
+bool qcom_scm_is_available(void)
+{
+	return !!__scm;
+}
+EXPORT_SYMBOL(qcom_scm_is_available);
+
+static int qcom_scm_probe(struct platform_device *pdev)
+{
+	struct qcom_scm *scm;
+	long rate;
+	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)
+			dev_err(&pdev->dev, "failed to acquire core clk\n");
+		return PTR_ERR(scm->core_clk);
+	}
+
+	if (of_device_is_compatible(pdev->dev.of_node, "qcom,scm-apq8064")) {
+		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);
+		}
+	}
+
+	/* vote for max clk rate for highest performance */
+	rate = clk_round_rate(scm->core_clk, INT_MAX);
+	ret = clk_set_rate(scm->core_clk, rate);
+	if (ret)
+		return ret;
+
+	__scm = scm;
+	__scm->dev = &pdev->dev;
+
+	return 0;
+}
+
+static const struct of_device_id qcom_scm_dt_match[] = {
+	{ .compatible = "qcom,scm-apq8064",},
+	{ .compatible = "qcom,scm-apq8084",},
+	{ .compatible = "qcom,scm-msm8916",},
+	{ .compatible = "qcom,scm-msm8974",},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
+
+static struct platform_driver qcom_scm_driver = {
+	.driver = {
+		.name	= "qcom_scm",
+		.of_match_table = qcom_scm_dt_match,
+	},
+	.probe = qcom_scm_probe,
+};
+
+builtin_platform_driver(qcom_scm_driver);
+MODULE_DESCRIPTION("Qualcomm SCM driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[toc] | [next] | [standalone]

#1385528

On Fri 22 Apr 15:17 PDT 2016, Andy Gross wrote:

[..]
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
[..]
> +static struct qcom_scm *__scm;
> +
> +static int qcom_scm_clk_enable(void)
> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(__scm->core_clk);
> +	if (ret)
> +		goto bail;
> +
> +	if (__scm->iface_clk) {

No need for this check, the clk api accepts NULL.

> +		ret = clk_prepare_enable(__scm->iface_clk);
> +		if (ret)
> +			goto disable_core;
> +	}
> +
> +	if (__scm->bus_clk) {

dito

> +		ret = clk_prepare_enable(__scm->bus_clk);
> +		if (ret)
> +			goto disable_iface;
> +	}
> +
> +	return 0;
> +
> +disable_iface:
> +	if (__scm->iface_clk)

dito

> +		clk_disable_unprepare(__scm->iface_clk);
> +disable_core:
> +	if (__scm->bus_clk)

and here

> +		clk_disable_unprepare(__scm->core_clk);
> +bail:
> +	return ret;
> +}
> +
> +static void qcom_scm_clk_disable(void)
> +{
> +	clk_disable_unprepare(__scm->core_clk);
> +	if (__scm->iface_clk)

and here

> +		clk_disable_unprepare(__scm->iface_clk);
> +	if (__scm->bus_clk)

and here

> +		clk_disable_unprepare(__scm->bus_clk);
> +}
> +
>  /**
>   * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
>   * @entry: Entry point function for the cpus
> @@ -72,11 +121,17 @@ EXPORT_SYMBOL(qcom_scm_cpu_power_down);
>   */
>  bool qcom_scm_hdcp_available(void)
>  {
> -	int ret;
> +	int ret = qcom_scm_clk_enable();
> +
> +	if (ret)
> +		goto clk_err;

Just return here.

>  
>  	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_HDCP,
> -		QCOM_SCM_CMD_HDCP);
> +						QCOM_SCM_CMD_HDCP);
> +
> +	qcom_scm_clk_disable();
>  
> +clk_err:
>  	return (ret > 0) ? true : false;

Unnecessary parenthesis.

>  }
>  EXPORT_SYMBOL(qcom_scm_hdcp_available);
> @@ -91,6 +146,89 @@ EXPORT_SYMBOL(qcom_scm_hdcp_available);
>   */
>  int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
>  {
> -	return __qcom_scm_hdcp_req(req, req_cnt, resp);
> +	int ret = qcom_scm_clk_enable();
> +
> +	if (ret)
> +		return ret;
> +
> +	ret = __qcom_scm_hdcp_req(req, req_cnt, resp);
> +	qcom_scm_clk_disable();
> +	return ret;
>  }
>  EXPORT_SYMBOL(qcom_scm_hdcp_req);
> +
> +/**
> + * qcom_scm_is_available() - Checks if SCM is available
> + */
> +bool qcom_scm_is_available(void)
> +{
> +	return !!__scm;
> +}
> +EXPORT_SYMBOL(qcom_scm_is_available);
> +
> +static int qcom_scm_probe(struct platform_device *pdev)
> +{
> +	struct qcom_scm *scm;
> +	long rate;
> +	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)
> +			dev_err(&pdev->dev, "failed to acquire core clk\n");
> +		return PTR_ERR(scm->core_clk);
> +	}
> +
> +	if (of_device_is_compatible(pdev->dev.of_node, "qcom,scm-apq8064")) {

Shouldn't this be reversed?

> +		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);
> +		}
> +	}
> +
> +	/* vote for max clk rate for highest performance */
> +	rate = clk_round_rate(scm->core_clk, INT_MAX);
> +	ret = clk_set_rate(scm->core_clk, rate);
> +	if (ret)
> +		return ret;
> +
> +	__scm = scm;
> +	__scm->dev = &pdev->dev;
> +
> +	return 0;
> +}
> +

Regards,
Bjorn

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

#1385575

On Fri, Apr 22, 2016 at 04:11:11PM -0700, Bjorn Andersson wrote:
> On Fri 22 Apr 15:17 PDT 2016, Andy Gross wrote:
> 
> [..]
> > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> [..]
> > +static struct qcom_scm *__scm;
> > +
> > +static int qcom_scm_clk_enable(void)
> > +{
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(__scm->core_clk);
> > +	if (ret)
> > +		goto bail;
> > +
> > +	if (__scm->iface_clk) {
> 
> No need for this check, the clk api accepts NULL.

Ah ok.  I wondered about that and then didn't check to see.

> > +		ret = clk_prepare_enable(__scm->iface_clk);
> > +		if (ret)
> > +			goto disable_core;
> > +	}
> > +
> > +	if (__scm->bus_clk) {
> 
> dito
> 
> > +		ret = clk_prepare_enable(__scm->bus_clk);
> > +		if (ret)
> > +			goto disable_iface;
> > +	}
> > +
> > +	return 0;
> > +
> > +disable_iface:
> > +	if (__scm->iface_clk)
> 
> dito
> 
> > +		clk_disable_unprepare(__scm->iface_clk);
> > +disable_core:
> > +	if (__scm->bus_clk)
> 
> and here
> 
> > +		clk_disable_unprepare(__scm->core_clk);
> > +bail:
> > +	return ret;
> > +}
> > +
> > +static void qcom_scm_clk_disable(void)
> > +{
> > +	clk_disable_unprepare(__scm->core_clk);
> > +	if (__scm->iface_clk)
> 
> and here
> 
> > +		clk_disable_unprepare(__scm->iface_clk);
> > +	if (__scm->bus_clk)
> 
> and here
> 
> > +		clk_disable_unprepare(__scm->bus_clk);
> > +}
> > +
> >  /**
> >   * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
> >   * @entry: Entry point function for the cpus
> > @@ -72,11 +121,17 @@ EXPORT_SYMBOL(qcom_scm_cpu_power_down);
> >   */
> >  bool qcom_scm_hdcp_available(void)
> >  {
> > -	int ret;
> > +	int ret = qcom_scm_clk_enable();
> > +
> > +	if (ret)
> > +		goto clk_err;
> 
> Just return here.

right.

> 
> >  
> >  	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_HDCP,
> > -		QCOM_SCM_CMD_HDCP);
> > +						QCOM_SCM_CMD_HDCP);
> > +
> > +	qcom_scm_clk_disable();
> >  
> > +clk_err:
> >  	return (ret > 0) ? true : false;
> 
> Unnecessary parenthesis.

will fix that.

> 
> >  }
> >  EXPORT_SYMBOL(qcom_scm_hdcp_available);
> > @@ -91,6 +146,89 @@ EXPORT_SYMBOL(qcom_scm_hdcp_available);
> >   */
> >  int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
> >  {
> > -	return __qcom_scm_hdcp_req(req, req_cnt, resp);
> > +	int ret = qcom_scm_clk_enable();
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = __qcom_scm_hdcp_req(req, req_cnt, resp);
> > +	qcom_scm_clk_disable();
> > +	return ret;
> >  }
> >  EXPORT_SYMBOL(qcom_scm_hdcp_req);
> > +
> > +/**
> > + * qcom_scm_is_available() - Checks if SCM is available
> > + */
> > +bool qcom_scm_is_available(void)
> > +{
> > +	return !!__scm;
> > +}
> > +EXPORT_SYMBOL(qcom_scm_is_available);
> > +
> > +static int qcom_scm_probe(struct platform_device *pdev)
> > +{
> > +	struct qcom_scm *scm;
> > +	long rate;
> > +	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)
> > +			dev_err(&pdev->dev, "failed to acquire core clk\n");
> > +		return PTR_ERR(scm->core_clk);
> > +	}
> > +
> > +	if (of_device_is_compatible(pdev->dev.of_node, "qcom,scm-apq8064")) {
> 
> Shouldn't this be reversed?

Right, I inverted this.  Will fix.

> 
> > +		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);
> > +		}
> > +	}
> > +
> > +	/* vote for max clk rate for highest performance */
> > +	rate = clk_round_rate(scm->core_clk, INT_MAX);
> > +	ret = clk_set_rate(scm->core_clk, rate);
> > +	if (ret)
> > +		return ret;
> > +
> > +	__scm = scm;
> > +	__scm->dev = &pdev->dev;
> > +
> > +	return 0;
> > +}
> > +

Thanks for reviewing!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web