Re: [PATCH v2 5/6] clk: qcom: Add camera clock controller driver for SM8150

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

 




On 7/15/2024 8:31 PM, Bryan O'Donoghue wrote:
On 15/07/2024 11:38, Dmitry Baryshkov wrote:
Does it apply to SM8150? For example, on SM8250 RCG2s are not parked.

Yes, it applies to SM8150.
Should the same logic be applied to other chipsets supported upstream?
If this is the case, which chipsets?

If you are representing the "top" GDSC inside of the CCF instead of doing this

+    /* Keep the critical clock always-on */
+    qcom_branch_set_clk_en(regmap, 0xc1e4); /* cam_cc_gdsc_clk */

then the clock should be parked else you'll find the GDSC doesn't come out of reset.

and... as I look at it now we have a logical conflict in drivers/clk/qcom/camcc-sc8280xp.c

static struct clk_branch camcc_gdsc_clk = {
        .halt_reg = 0xc1e4,
        .halt_check = BRANCH_HALT,
        .clkr = {
                .enable_reg = 0xc1e4,
                .enable_mask = BIT(0),
                .hw.init = &(struct clk_init_data){
                        .name = "camcc_gdsc_clk",
                        .parent_hws = (const struct clk_hw*[]){
                                &camcc_xo_clk_src.clkr.hw,
                        },
                        .num_parents = 1,
                        .flags = CLK_SET_RATE_PARENT,
                        .ops = &clk_branch2_ops,
                },
        },
};

Patch sent.

https://lore.kernel.org/linux-arm-msm/20240715-linux-next-24-07-13-sc8280xp-camcc-fixes-v1-1-fadb5d9445c1@xxxxxxxxxx/T/#u


If the clock is modelled, it can get disabled during the late init call, when the CCF disables the unused clocks. But, it is a PoR ON  clock and expectation from design team is to keep it always-on for GDSC functionality.

So, we should not model this, instead just keep it always on from probe.


In the round I think we should avoid these horrific hard-coded always-on writes where possible.


We have been suggested to make such clocks always ON from probe on previous upstream discussions. This is also recently acknowledged by maintainers, please see [1] and [2].


[1] https://lore.kernel.org/linux-clk/6286a410-6faa-4996-8a9e-dc334dd9421f@xxxxxxxxxx/


[2] https://lore.kernel.org/linux-clk/664cca91-8615-d3f6-7525-15b9b6725cce@xxxxxxxxxxx/


Personally I think parking is better than always-on specifically because you define the clock and "see" it in the clock tree.

---
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