Re: [PATCH v2 2/4] clk: qcom: camcc-glymur: Add camera clock controller driver

[Konrad Dybcio]

On 4/30/26 9:19 AM, Bryan O'Donoghue wrote:
> On 29/04/2026 15:14, Jagadeesh Kona wrote:
>> +/* 1200.0 MHz Configuration */
>> +static const struct alpha_pll_config cam_cc_pll0_config = {
>> +    .l = 0x3e,
>> +    .alpha = 0x8000,
>> +    .config_ctl_val = 0x25c400e7,
>> +    .config_ctl_hi_val = 0x0a8060e0,
>> +    .config_ctl_hi1_val = 0xf51dea20,
>> +    .user_ctl_val = 0x00008408,
>> +    .user_ctl_hi_val = 0x00000002,
>> +};
> 
> Could we start defining these bits intead of stuffing magic numbers ?
> 
> I can't imagine a PLL setting is commercially sensitive and even if it is..
> 
> There's a difference between someone in the community doing a port of a downstream configuration where the bits aren't documented and a vendor doing upstreaming where it the vendor has control.
> 
> What does 0x0a8060e0 actually mean and - yes its more work but, why can't we define those bits and bit-fields ?

I think this is largely "this is the value that the silicon was tested
with and what the entire frequency plan depends on" - some of these bits
are defined and acted upon in clk-alpha-pll.c

Konrad