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

[Jagadeesh Kona]

On 5/18/2026 1:05 PM, Bryan O'Donoghue wrote:
> On 17/05/2026 18:33, 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,
>> +};
> 
> I'll again push back on these magic numbers.
> 
> At the very least you should be mentioning in the cover letter log why you _aren't_ making that change.
> 
> Just reposting and hoping it slips by the person making the comment isn't too cool.
> 
> Why can't qcom update the python? script that generates this code to enumerate fields instead of magic numbers here ?
> 
> I get you don't want to do it but, just ignoring the review feedback is no OK.
> 
> What gives ?
> 

Hi Bryan,

I haven't ignored your comments & already responded to your earlier comment on why the bit fields are not
defined. Most of these values are static settings we get from PLL HW team and we program them only once
as is during bootup and are never reused again anywhere from PLL code, so these bits are not defined.

Please find the earlier responses for your comments below:
https://lore.kernel.org/all/b92a2cbb-fe8d-4378-aa02-d91e2e4dfff4@xxxxxxxxxxxxxxxx/ 
https://lore.kernel.org/all/009ecdbb-2297-44eb-862d-233e3290691c@xxxxxxxxxxxxxxxx/

Thanks,
Jagadeesh