Re: [PATCH v2 3/3] clk: qcom: camcc: Add camera clock controller driver for SC7180
Thanks for your review Stephen.
On 10/14/2020 7:29 AM, Stephen Boyd wrote:
Quoting Taniya Das (2020-10-13 10:11:50)
diff --git a/drivers/clk/qcom/camcc-sc7180.c b/drivers/clk/qcom/camcc-sc7180.c
new file mode 100644
index 000..e954d21
--- /dev/null
+++ b/drivers/clk/qcom/camcc-sc7180.c
@@ -0,0 +1,1737 @@
[...]
+
+enum {
+ P_BI_TCXO,
+ P_CAM_CC_PLL0_OUT_EVEN,
+ P_CAM_CC_PLL1_OUT_EVEN,
+ P_CAM_CC_PLL2_OUT_AUX,
+ P_CAM_CC_PLL2_OUT_EARLY,
+ P_CAM_CC_PLL3_OUT_MAIN,
+ P_CORE_BI_PLL_TEST_SE,
+};
+
+static struct pll_vco agera_vco[] = {
Can this be const?
+ { 6, 33, 0 },
+};
+
+static struct pll_vco fabia_vco[] = {
Can this be const?
+ { 24960, 20, 0 },
+};
+
[...]
Will take care of the above in the next patch.
+
+static int cam_cc_sc7180_probe(struct platform_device *pdev)
+{
+ struct regmap *regmap;
+ int ret;
+
+ pm_runtime_enable(&pdev->dev);
+ ret = pm_clk_create(&pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = pm_clk_add(&pdev->dev, "xo");
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to acquire XO clock\n");
+ goto disable_pm_runtime;
+ }
+
+ ret = pm_clk_add(&pdev->dev, "iface");
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to acquire iface clock\n");
+ goto disable_pm_runtime;
+ }
+
+ ret = pm_clk_runtime_resume(&pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "pm runtime resume failed\n");
+ goto destroy_pm_clk;
+ }
+
+ regmap = qcom_cc_map(pdev, &cam_cc_sc7180_desc);
+ if (IS_ERR(regmap)) {
+ ret = PTR_ERR(regmap);
+ goto destroy_pm_clk;
+ }
+
+ clk_fabia_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
+ clk_fabia_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
+ clk_agera_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config);
+ clk_fabia_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
+
+ ret = qcom_cc_really_probe(pdev, &cam_cc_sc7180_desc, regmap);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register CAM CC clocks\n");
+ goto suspend_pm_runtime;
ret is non-zero here
+ }
+
+suspend_pm_runtime:
+ ret = pm_clk_runtime_suspend(&pdev->dev);
But then it is overwritten here.
+ if (ret)
+ dev_err(&pdev->dev, "pm runtime suspend failed\n");
+
+ return 0;
And we return 0 when there was a failure to probe the clks?
I will clean the error path in the next patch.
+
+destroy_pm_clk:
+ pm_clk_destroy(&pdev->dev);
+
+disable_pm_runtime:
+ pm_runtime_disable(&pdev->dev);
+
+ return ret;
+}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
Re: [PATCH v2 3/3] clk: qcom: camcc: Add camera clock controller driver for SC7180
Quoting Taniya Das (2020-10-13 10:11:50)
> diff --git a/drivers/clk/qcom/camcc-sc7180.c b/drivers/clk/qcom/camcc-sc7180.c
> new file mode 100644
> index 000..e954d21
> --- /dev/null
> +++ b/drivers/clk/qcom/camcc-sc7180.c
> @@ -0,0 +1,1737 @@
[...]
> +
> +enum {
> + P_BI_TCXO,
> + P_CAM_CC_PLL0_OUT_EVEN,
> + P_CAM_CC_PLL1_OUT_EVEN,
> + P_CAM_CC_PLL2_OUT_AUX,
> + P_CAM_CC_PLL2_OUT_EARLY,
> + P_CAM_CC_PLL3_OUT_MAIN,
> + P_CORE_BI_PLL_TEST_SE,
> +};
> +
> +static struct pll_vco agera_vco[] = {
Can this be const?
> + { 6, 33, 0 },
> +};
> +
> +static struct pll_vco fabia_vco[] = {
Can this be const?
> + { 24960, 20, 0 },
> +};
> +
[...]
> +
> +static int cam_cc_sc7180_probe(struct platform_device *pdev)
> +{
> + struct regmap *regmap;
> + int ret;
> +
> + pm_runtime_enable(&pdev->dev);
> + ret = pm_clk_create(&pdev->dev);
> + if (ret)
> + return ret;
> +
> + ret = pm_clk_add(&pdev->dev, "xo");
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to acquire XO clock\n");
> + goto disable_pm_runtime;
> + }
> +
> + ret = pm_clk_add(&pdev->dev, "iface");
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to acquire iface clock\n");
> + goto disable_pm_runtime;
> + }
> +
> + ret = pm_clk_runtime_resume(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "pm runtime resume failed\n");
> + goto destroy_pm_clk;
> + }
> +
> + regmap = qcom_cc_map(pdev, &cam_cc_sc7180_desc);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + goto destroy_pm_clk;
> + }
> +
> + clk_fabia_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
> + clk_fabia_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
> + clk_agera_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config);
> + clk_fabia_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
> +
> + ret = qcom_cc_really_probe(pdev, &cam_cc_sc7180_desc, regmap);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register CAM CC clocks\n");
> + goto suspend_pm_runtime;
ret is non-zero here
> + }
> +
> +suspend_pm_runtime:
> + ret = pm_clk_runtime_suspend(&pdev->dev);
But then it is overwritten here.
> + if (ret)
> + dev_err(&pdev->dev, "pm runtime suspend failed\n");
> +
> + return 0;
And we return 0 when there was a failure to probe the clks?
> +
> +destroy_pm_clk:
> + pm_clk_destroy(&pdev->dev);
> +
> +disable_pm_runtime:
> + pm_runtime_disable(&pdev->dev);
> +
> + return ret;
> +}
[PATCH v2 3/3] clk: qcom: camcc: Add camera clock controller driver for SC7180
Add support for the camera clock controller found on SC7180 based devices.
This would allow camera drivers to probe and control their clocks.
Signed-off-by: Taniya Das
---
drivers/clk/qcom/Kconfig|9 +
drivers/clk/qcom/Makefile |1 +
drivers/clk/qcom/camcc-sc7180.c | 1737 +++
3 files changed, 1747 insertions(+)
create mode 100644 drivers/clk/qcom/camcc-sc7180.c
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 0583273..7aeebe6 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -290,6 +290,15 @@ config QCS_GCC_404
Say Y if you want to use multimedia devices or peripheral
devices such as UART, SPI, I2C, USB, SD/eMMC, PCIe etc.
+config SC_CAMCC_7180
+ tristate "SC7180 Camera Clock Controller"
+ select SC_GCC_7180
+ help
+ Support for the camera clock controller on Qualcomm Technologies, Inc
+ SC7180 devices.
+ Say Y if you want to support camera devices and functionality such as
+ capturing pictures.
+
config SC_DISPCC_7180
tristate "SC7180 Display Clock Controller"
select SC_GCC_7180
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 9677e76..8e0579f 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
obj-$(CONFIG_QCS_GCC_404) += gcc-qcs404.o
obj-$(CONFIG_QCS_Q6SSTOP_404) += q6sstop-qcs404.o
obj-$(CONFIG_QCS_TURING_404) += turingcc-qcs404.o
+obj-$(CONFIG_SC_CAMCC_7180) += camcc-sc7180.o
obj-$(CONFIG_SC_DISPCC_7180) += dispcc-sc7180.o
obj-$(CONFIG_SC_GCC_7180) += gcc-sc7180.o
obj-$(CONFIG_SC_GPUCC_7180) += gpucc-sc7180.o
diff --git a/drivers/clk/qcom/camcc-sc7180.c b/drivers/clk/qcom/camcc-sc7180.c
new file mode 100644
index 000..e954d21
--- /dev/null
+++ b/drivers/clk/qcom/camcc-sc7180.c
@@ -0,0 +1,1737 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "common.h"
+#include "gdsc.h"
+#include "reset.h"
+
+enum {
+ P_BI_TCXO,
+ P_CAM_CC_PLL0_OUT_EVEN,
+ P_CAM_CC_PLL1_OUT_EVEN,
+ P_CAM_CC_PLL2_OUT_AUX,
+ P_CAM_CC_PLL2_OUT_EARLY,
+ P_CAM_CC_PLL3_OUT_MAIN,
+ P_CORE_BI_PLL_TEST_SE,
+};
+
+static struct pll_vco agera_vco[] = {
+ { 6, 33, 0 },
+};
+
+static struct pll_vco fabia_vco[] = {
+ { 24960, 20, 0 },
+};
+
+/* 600MHz configuration */
+static const struct alpha_pll_config cam_cc_pll0_config = {
+ .l = 0x1F,
+ .alpha = 0x4000,
+ .config_ctl_val = 0x20485699,
+ .config_ctl_hi_val = 0x2067,
+ .test_ctl_val = 0x4000,
+ .user_ctl_hi_val = 0x4805,
+ .user_ctl_val = 0x0001,
+};
+
+static struct clk_alpha_pll cam_cc_pll0 = {
+ .offset = 0x0,
+ .vco_table = fabia_vco,
+ .num_vco = ARRAY_SIZE(fabia_vco),
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
+ .clkr = {
+ .hw.init = &(struct clk_init_data){
+ .name = "cam_cc_pll0",
+ .parent_data = &(const struct clk_parent_data){
+ .fw_name = "bi_tcxo",
+ },
+ .num_parents = 1,
+ .ops = &clk_alpha_pll_fabia_ops,
+ },
+ },
+};
+
+/* 860MHz configuration */
+static const struct alpha_pll_config cam_cc_pll1_config = {
+ .l = 0x2A,
+ .alpha = 0x1555,
+ .config_ctl_val = 0x20485699,
+ .config_ctl_hi_val = 0x2067,
+ .test_ctl_val = 0x4000,
+ .user_ctl_hi_val = 0x4805,
+};
+
+static struct clk_alpha_pll cam_cc_pll1 = {
+ .offset = 0x1000,
+ .vco_table = fabia_vco,
+ .num_vco = ARRAY_SIZE(fabia_vco),
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
+ .clkr = {
+ .hw.init = &(struct clk_init_data){
+ .name = "cam_cc_pll1",
+ .parent_data = &(const struct clk_parent_data){
+ .fw_name = "bi_tcxo",
+ },
+ .num_parents = 1,
+ .ops = &clk_alpha_pll_fabia_ops,
+ },
+ },
+};
+
+/* 1920MHz configuration */
+static const struct alpha_pll_config cam_cc_pll2_config = {
+ .l = 0x64,
+ .config_ctl_val = 0x2800,
+ .config_ctl_hi_val = 0x43D2,
+ .test_ctl_val = 0x04000400,
+ .test_ctl_hi_val = 0x4000,
+ .user_ctl_val = 0x030F,
+};
+
+static struct clk_alpha_pll cam_cc_pll2 = {
+ .offset = 0x2000,
+ .vco_table = agera_vco,
+ .num_vco = ARRAY_SIZE(
