Re: [PATCH V2 RESEND 5/6] clk: qcom: camcc-sm8650: Add SM8650 camera clock controller driver
From: Jagadeesh Kona
Date: Mon Apr 22 2024 - 06:58:29 EST
On 4/19/2024 3:00 AM, Vladimir Zapolskiy wrote:
Hello Jagadeesh,
thank you for submitting the clock driver.
On 3/21/24 11:25, Jagadeesh Kona wrote:
Add support for the camera clock controller for camera clients to
be able to request for camcc clocks on SM8650 platform.
Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
---
drivers/clk/qcom/Kconfig | 8 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/camcc-sm8650.c | 3591 +++++++++++++++++++++++++++++++
3 files changed, 3600 insertions(+)
create mode 100644 drivers/clk/qcom/camcc-sm8650.c
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 8ab08e7b5b6c..6257f4a02ec4 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -826,6 +826,14 @@ config SM_CAMCC_8550
Support for the camera clock controller on SM8550 devices.
Say Y if you want to support camera devices and camera
functionality.
+config SM_CAMCC_8650
+ tristate "SM8650 Camera Clock Controller"
+ depends on ARM64 || COMPILE_TEST
+ select SM_GCC_8650
+ help
+ Support for the camera clock controller on SM8650 devices.
+ Say Y if you want to support camera devices and camera
functionality.
+
config SM_DISPCC_6115
tristate "SM6115 Display Clock Controller"
depends on ARM64 || COMPILE_TEST
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index dec5b6db6860..28bffa1eb8dd 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -109,6 +109,7 @@ obj-$(CONFIG_SM_CAMCC_6350) += camcc-sm6350.o
obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o
obj-$(CONFIG_SM_CAMCC_8450) += camcc-sm8450.o
obj-$(CONFIG_SM_CAMCC_8550) += camcc-sm8550.o
+obj-$(CONFIG_SM_CAMCC_8650) += camcc-sm8650.o
obj-$(CONFIG_SM_DISPCC_6115) += dispcc-sm6115.o
obj-$(CONFIG_SM_DISPCC_6125) += dispcc-sm6125.o
obj-$(CONFIG_SM_DISPCC_6350) += dispcc-sm6350.o
diff --git a/drivers/clk/qcom/camcc-sm8650.c
b/drivers/clk/qcom/camcc-sm8650.c
new file mode 100644
index 000000000000..1b28e086e519
--- /dev/null
+++ b/drivers/clk/qcom/camcc-sm8650.c
@@ -0,0 +1,3591 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights
reserved.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,sm8650-camcc.h>
+
+#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 {
+ DT_IFACE,
+ DT_BI_TCXO,
+ DT_BI_TCXO_AO,
+ DT_SLEEP_CLK,
+};
+
+enum {
+ P_BI_TCXO,
+ P_BI_TCXO_AO,
+ P_CAM_CC_PLL0_OUT_EVEN,
+ P_CAM_CC_PLL0_OUT_MAIN,
+ P_CAM_CC_PLL0_OUT_ODD,
+ P_CAM_CC_PLL1_OUT_EVEN,
+ P_CAM_CC_PLL2_OUT_EVEN,
+ P_CAM_CC_PLL2_OUT_MAIN,
+ P_CAM_CC_PLL3_OUT_EVEN,
+ P_CAM_CC_PLL4_OUT_EVEN,
+ P_CAM_CC_PLL5_OUT_EVEN,
+ P_CAM_CC_PLL6_OUT_EVEN,
+ P_CAM_CC_PLL7_OUT_EVEN,
+ P_CAM_CC_PLL8_OUT_EVEN,
+ P_CAM_CC_PLL9_OUT_EVEN,
+ P_CAM_CC_PLL9_OUT_ODD,
+ P_CAM_CC_PLL10_OUT_EVEN,
+ P_SLEEP_CLK,
+};
+
+static const struct pll_vco lucid_ole_vco[] = {
+ { 249600000, 2300000000, 0 },
+};
I've noticed that a downstream Android kernel v6.1.25 defines this clock as
static const struct pll_vco lucid_ole_vco[] = {
{ 249600000, 2100000000, 0 },
};
Do you know any particular reason why here the clock frequencies are
different?
Thanks Vladimir for your review!
The min and max supported frequencies of PLL mentioned above are taken
from the HW specification, and as per the latest HW spec, the maximum
supported frequency for lucid OLE PLL is 2300MHz, hence used 2300MHz above.
+
+static const struct pll_vco rivian_ole_vco[] = {
+ { 777000000, 1285000000, 0 },
+};
+
<snip>
+static struct clk_rcg2 cam_cc_bps_clk_src = {
+ .cmd_rcgr = 0x10050,
+ .mnd_width = 0,
+ .hid_width = 5,
+ .parent_map = cam_cc_parent_map_2,
+ .freq_tbl = ftbl_cam_cc_bps_clk_src,
+ .clkr.hw.init = &(const struct clk_init_data) {
+ .name = "cam_cc_bps_clk_src",
+ .parent_data = cam_cc_parent_data_2,
+ .num_parents = ARRAY_SIZE(cam_cc_parent_data_2),
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_rcg2_shared_ops,
+ },
+};
Please let me ask after Dmitry about your rationale to select
&clk_rcg2_shared_ops here and below for all *_src clocks introduced in
the driver, I do remember you've did it in v1, could you please
elaborate it a bit more?
I have a concern that it's not possible to get an .is_enabled status
of the shared clocks, however at least in this particular case of
camcc clocks it seems to be technically possible.
It might indicate that there is an incompleteness in clk-rcg2.c driver
also, if it's really possible to get is_enabled runtime status at least
for some of the shared clocks.
The recommendation from HW team is to park the RCG's at XO clock source
when RCG is in disabled state and clk_rcg2_shared_ops is the closest
implementation for achieving the same, hence used clk_rcg2_shared_ops
for all the RCG's.
I will check if .is_enabled callback can be added to shared_ops and post
a separate series for it based on the requirement.
+
+static const struct freq_tbl ftbl_cam_cc_camnoc_axi_rt_clk_src[] = {
+ F(300000000, P_CAM_CC_PLL0_OUT_EVEN, 2, 0, 0),
+ F(400000000, P_CAM_CC_PLL0_OUT_ODD, 1, 0, 0),
+ { }
+};
+
<snip>
Other than two my open questions above I don't see any issues with the
driver, if you be kind to provide the answers, please feel free to add
my
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>
Tested-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>
Thanks, sure will add these tags in next series.
Thanks,
Jagadeesh
--
Best wishes,
Vladimir