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
