diff mbox

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

Message ID 1461625725-32425-3-git-send-email-andy.gross@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Andy Gross April 25, 2016, 11:08 p.m. UTC
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  | 163 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 155 insertions(+), 9 deletions(-)

Comments

Stephen Boyd April 26, 2016, 1:29 a.m. UTC | #1
On 04/25, Andy Gross wrote:
> 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  | 163 ++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 155 insertions(+), 9 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

So far we've left this selection up to the consumer drivers of
the qcom_scm_*() APIs. Any reason why that's changing here? I
don't see mention in the commit text.

>  	help
>  	  This enables support for the ARMv8 based Qualcomm chipsets.
>  
> +
> +/**
> + * qcom_scm_is_available() - Checks if SCM is available
> + */
> +bool qcom_scm_is_available(void)
> +{
> +	return !!__scm;
> +}
> +EXPORT_SYMBOL(qcom_scm_is_available);

What's the planned user of this? If we need it can we bury it
inside the qcom_scm_*() functions?

> +
> +static int qcom_scm_probe(struct platform_device *pdev)
> +{
[...]
> +
> +	/* 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);

You can just do clk_set_rate(scm->core_clk, INT_MAX) and it will
round internally for you and do the right thing. 

> +	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",},
> +	{},

Nitpick: drop , here because it's always going to be the last
entry.

> +};
> +
> +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);
> +
> +static int __init qcom_scm_init(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_node_by_name(NULL, "firmware");
> +	if (!np)
> +		return -ENODEV;
> +
> +	return of_platform_populate(np, qcom_scm_dt_match, NULL, NULL);
> +

Weird newline and also we need an of_node_put() on the firmware
node at the end of this function.
Andy Gross April 29, 2016, 7:25 p.m. UTC | #2
On Mon, Apr 25, 2016 at 06:29:35PM -0700, Stephen Boyd wrote:
> On 04/25, Andy Gross wrote:
> > 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  | 163 ++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 155 insertions(+), 9 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
> 
> So far we've left this selection up to the consumer drivers of
> the qcom_scm_*() APIs. Any reason why that's changing here? I
> don't see mention in the commit text.

We can leave it that way.
> 
> >  	help
> >  	  This enables support for the ARMv8 based Qualcomm chipsets.
> >  
> > +
> > +/**
> > + * qcom_scm_is_available() - Checks if SCM is available
> > + */
> > +bool qcom_scm_is_available(void)
> > +{
> > +	return !!__scm;
> > +}
> > +EXPORT_SYMBOL(qcom_scm_is_available);
> 
> What's the planned user of this? If we need it can we bury it
> inside the qcom_scm_*() functions?

Hmmmm doing a little searching, I don't see this being used anymore.  I'll drop
it for now.

> > +
> > +static int qcom_scm_probe(struct platform_device *pdev)
> > +{
> [...]
> > +
> > +	/* 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);
> 
> You can just do clk_set_rate(scm->core_clk, INT_MAX) and it will
> round internally for you and do the right thing. 

I'll change this to do that.

> > +	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",},
> > +	{},
> 
> Nitpick: drop , here because it's always going to be the last
> entry.

will fix.

> > +};
> > +
> > +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);
> > +
> > +static int __init qcom_scm_init(void)
> > +{
> > +	struct device_node *np;
> > +
> > +	np = of_find_node_by_name(NULL, "firmware");
> > +	if (!np)
> > +		return -ENODEV;
> > +
> > +	return of_platform_populate(np, qcom_scm_dt_match, NULL, NULL);
> > +
> 
> Weird newline and also we need an of_node_put() on the firmware
> node at the end of this function.

Ah thanks for catching that.


Regards,

Andy
diff mbox

Patch

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..d4e9145 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -10,19 +10,61 @@ 
  * 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/of_platform.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;
+
+	ret = clk_prepare_enable(__scm->iface_clk);
+	if (ret)
+		goto disable_core;
+
+	ret = clk_prepare_enable(__scm->bus_clk);
+	if (ret)
+		goto disable_iface;
+
+	return 0;
+
+disable_iface:
+	clk_disable_unprepare(__scm->iface_clk);
+disable_core:
+	clk_disable_unprepare(__scm->core_clk);
+bail:
+	return ret;
+}
+
+static void qcom_scm_clk_disable(void)
+{
+	clk_disable_unprepare(__scm->core_clk);
+	clk_disable_unprepare(__scm->iface_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,12 +114,17 @@  EXPORT_SYMBOL(qcom_scm_cpu_power_down);
  */
 bool qcom_scm_hdcp_available(void)
 {
-	int ret;
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
 
 	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_HDCP,
-		QCOM_SCM_CMD_HDCP);
+						QCOM_SCM_CMD_HDCP);
+
+	qcom_scm_clk_disable();
 
-	return (ret > 0) ? true : false;
+	return ret > 0 ? true : false;
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_available);
 
@@ -91,6 +138,104 @@  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);
+
+static int __init qcom_scm_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_node_by_name(NULL, "firmware");
+	if (!np)
+		return -ENODEV;
+
+	return of_platform_populate(np, qcom_scm_dt_match, NULL, NULL);
+
+}
+
+arch_initcall(qcom_scm_init);
+
+MODULE_DESCRIPTION("Qualcomm SCM driver");
+MODULE_LICENSE("GPL v2");