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
