Re: [PATCH] clk: qcom: camcc-sm8250: Fix halt on boot by reducing driver's init level

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux