On 5/18/22 8:48 AM, Bryan O'Donoghue wrote:
On 18/05/2022 11:35, Vladimir Zapolskiy wrote:
Access to I/O of SM8250 camera clock controller IP depends on enabled
GCC_CAMERA_AHB_CLK clock supplied by global clock controller, the latter
one is inited on subsys level, so, to satisfy the dependency, it would
make sense to deprive the init level of camcc-sm8250 driver.
If both drivers are compiled as built-in, there is a change that a board
won't boot up due to a race, which happens on the same init level.
Fixes: 5d66ca79b58c ("clk: qcom: Add camera clock controller driver
for SM8250")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>
---
drivers/clk/qcom/camcc-sm8250.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/clk/qcom/camcc-sm8250.c
b/drivers/clk/qcom/camcc-sm8250.c
index 439eaafdcc86..ae4e9774f36e 100644
--- a/drivers/clk/qcom/camcc-sm8250.c
+++ b/drivers/clk/qcom/camcc-sm8250.c
@@ -2440,17 +2440,7 @@ static struct platform_driver
cam_cc_sm8250_driver = {
},
};
-static int __init cam_cc_sm8250_init(void)
-{
- return platform_driver_register(&cam_cc_sm8250_driver);
-}
-subsys_initcall(cam_cc_sm8250_init);
-
-static void __exit cam_cc_sm8250_exit(void)
-{
- platform_driver_unregister(&cam_cc_sm8250_driver);
-}
-module_exit(cam_cc_sm8250_exit);
+module_platform_driver(cam_cc_sm8250_driver);
MODULE_DESCRIPTION("QTI CAMCC SM8250 Driver");
MODULE_LICENSE("GPL v2");
So I tried this
- clocks = <&gcc GCC_CAMERA_AHB_CLK>,
- <&rpmhcc RPMH_CXO_CLK>,
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
<&rpmhcc RPMH_CXO_CLK_A>,
<&sleep_clk>;
- clock-names = "iface", "bi_tcxo", "bi_tcxo_ao",
"sleep_clk";
+ clock-names = "bi_tcxo", "bi_tcxo_ao", "sleep_clk";
and the system wouldn't boot @ * 736ee37e2e8e - (tag: next-20220518,
linux-next/master) Add linux-next specific files for 20220518 (2 hours ago)
GCC_CAMERA_AHB_CLK is defined but it isn't actually implemented by the
upstream gcc driver, and the camcc driver doesn't do anything with it
either (I didn't include it in the camcc driver because the gcc driver
didn't implement it, but I have a patch to do things like downstream,
dispcc/gpucc/videocc drivers all have this problem too). Does having it
in the dts like this cause the gcc driver to probe first somehow, even
though the clock isn't used by the camcc driver?
(The sc7180 camcc driver does do something with the "iface" clock, but
the sc7180 gcc driver also doesn't implement GCC_CAMERA_AHB_CLK either..
I guess you get a dummy clock for the unimplemented clocks?)
If we do a grep
grep subsys_init drivers/clk/qcom/camcc-*
drivers/clk/qcom/camcc-sc7180.c:subsys_initcall(cam_cc_sc7180_init);
drivers/clk/qcom/camcc-sc7280.c:subsys_initcall(cam_cc_sc7280_init);
drivers/clk/qcom/camcc-sdm845.c:subsys_initcall(cam_cc_sdm845_init);
drivers/clk/qcom/camcc-sm8250.c:subsys_initcall(cam_cc_sm8250_init);
and
arch/arm64/boot/dts/qcom/sc7180.dtsi: <&gcc
GCC_CAMERA_AHB_CLK>,
arch/arm64/boot/dts/qcom/sm8250.dtsi: clocks = <&gcc
GCC_CAMERA_AHB_CLK>,
I think the sc7180 has this same dependency loop. Probably needs the
same fix.
Also not sure why sdm845 camcc doesn't declare a depends on
GCC_CAMERA_AHB_CLK - should it ?
Recommend applying this same fix to sc718x
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
---
bod
