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

[Bryan O'Donoghue]

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

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

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

---
bod