On 18/05/2022 20:46, Jonathan Marek wrote:
GCC_CAMERA_AHB_CLK is defined but it isn't actually implemented by the
upstream gcc driver
*facepalm*
drivers/clk/qcom/gcc-sc8180x.c: * GCC_VIDEO_AHB_CLK,
GCC_CAMERA_AHB_CLK, GCC_DISP_AHB_CLK,
drivers/clk/qcom/gcc-sm8250.c: * GCC_VIDEO_AHB_CLK, GCC_CAMERA_AHB_CLK,
GCC_DISP_AHB_CLK,
drivers/clk/qcom/gcc-sm6350.c: [GCC_CAMERA_AHB_CLK] =
&gcc_camera_ahb_clk.clkr,
you're right - and we have this too don't we
/*
* Keep the clocks always-ON
* GCC_VIDEO_AHB_CLK, GCC_CAMERA_AHB_CLK, GCC_DISP_AHB_CLK,
* GCC_CPUSS_DVM_BUS_CLK, GCC_GPU_CFG_AHB_CLK,
* GCC_SYS_NOC_CPUSS_AHB_CLK
*/
regmap_update_bits(regmap, 0x0b004, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x0b008, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x0b00c, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x4818c, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x52000, BIT(0), BIT(0));
drivers/clk/qcom/gcc-sm8250.c - so defining or not defining
GCC_CAMERA_AHB_CLK should be a nop.
And yep we have this on sc7280
/*
* Keep the clocks always-ON
* GCC_CPUSS_GNOC_CLK, GCC_VIDEO_AHB_CLK, GCC_CAMERA_AHB_CLK,
* GCC_DISP_AHB_CLK, GCC_GPU_CFG_AHB_CLK
*/
regmap_update_bits(regmap, 0x48004, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x0b004, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x0b008, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x0b00c, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x0b02c, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x0b028, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x0b030, BIT(0), BIT(0));
regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
at least the bug is consistent :)
, and the camcc driver doesn't do anything with it
yep - it has an interconnect with a hopeful name "cam_ahb" we don't have
an obvious clock on the AP side for that, perhaps the rpmh is doing
something with the AHB clock because of an interconnect call.
We will have to do a deep dive to find out.
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?)
Yep, I missed that.
Meh. Ok we know we have a bug, we know its replicated on sm8250 and
sc7280 and we know it doesn't make sense.
My guess is rpmh is switching on the clock. In any case we clearly
haven't captured the clock dependency right upstream.
---
bod