Re: [PATCH v2 2/2] clk: qcom: Add support for the camera clock controller (CAMCC) on Hawi
From: sashiko-bot@kernel.org
Date: 2026-09-09 06:32:47
Also in:
linux-clk
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Missing post-divider definition and mismatched clock parent mapping for `cam_cc_pll7_out_even`. - [Low] Redundant and identical parent map and parent data definitions. -- commit 94962e26b9035957a94cf2180f7c86517eb17167 Author: Vivek Aknurwar [off-list ref] clk: qcom: Add support for the camera clock controller (CAMCC) on Hawi This commit adds support for the camera clock controller (CAMCC) on the Qualcomm Hawi SoC. It introduces the clock definitions and routing for camera operations, reusing the MCLK driver from Kaanapali.
quoted hunk ↗ jump to hunk
diff --git a/drivers/clk/qcom/camcc-hawi.c b/drivers/clk/qcom/camcc-hawi.c new file mode 100644 index 0000000000000..5b004a627fdde --- /dev/null +++ b/drivers/clk/qcom/camcc-hawi.c
[ ... ]
+static const struct parent_map cam_cc_parent_map_1[] = {
+ { P_BI_TCXO, 0 },
+ { P_CAM_CC_PLL0_OUT_MAIN, 1 },
+ { P_CAM_CC_PLL0_OUT_EVEN, 2 },
+ { P_CAM_CC_PLL0_OUT_ODD, 3 },
+ { P_CAM_CC_PLL6_OUT_ODD, 4 },
+ { P_CAM_CC_PLL6_OUT_EVEN, 5 },
+ { P_CAM_CC_PLL6_OUT_MAIN, 6 },
+};
+
+static const struct clk_parent_data cam_cc_parent_data_1[] = {
+ { .index = DT_BI_TCXO },
+ { .hw = &cam_cc_pll0.clkr.hw },
+ { .hw = &cam_cc_pll0_out_even.clkr.hw },
+ { .hw = &cam_cc_pll0_out_odd.clkr.hw },
+ { .hw = &cam_cc_pll6_out_odd.clkr.hw },
+ { .hw = &cam_cc_pll6_out_even.clkr.hw },
+ { .hw = &cam_cc_pll6.clkr.hw },
+};[Severity: Low] Are cam_cc_parent_map_1 and cam_cc_parent_data_1 intended to be completely identical to the _0 variants defined just above this block? This isn't a functional issue, but could we reuse the _0 map and data structures to reduce code duplication and simplify the driver? [ ... ]
+static const struct parent_map cam_cc_parent_map_2[] = {
+ { P_BI_TCXO, 0 },
+ { P_CAM_CC_PLL0_OUT_MAIN, 1 },
+ { P_CAM_CC_PLL0_OUT_EVEN, 2 },
+ { P_CAM_CC_PLL0_OUT_ODD, 3 },
+ { P_CAM_CC_PLL7_OUT_EVEN, 5 },
+ { P_CAM_CC_PLL7_OUT_MAIN, 6 },
+};
+
+static const struct clk_parent_data cam_cc_parent_data_2[] = {
+ { .index = DT_BI_TCXO },
+ { .hw = &cam_cc_pll0.clkr.hw },
+ { .hw = &cam_cc_pll0_out_even.clkr.hw },
+ { .hw = &cam_cc_pll0_out_odd.clkr.hw },
+ { .hw = &cam_cc_pll7.clkr.hw },
+ { .hw = &cam_cc_pll7.clkr.hw },
+};[Severity: Medium] Does this misconfigure the clock parent mapping for cam_cc_pll7_out_even? The parent map array specifies P_CAM_CC_PLL7_OUT_EVEN at index 5, but the corresponding parent data array maps it to the undivided main PLL (&cam_cc_pll7.clkr.hw) instead of a post-divider hw struct. If the bootloader leaves the hardware mux at index 5, could this cause the framework to miscalculate the frequency as the undivided main PLL rate instead of the divided rate, leading to incorrect downstream clock rates? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260908-camcc-hawi-v2-0-f892ba1d118e@oss.qualcomm.com?part=2