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

[Bryan O'Donoghue <bod@kernel.org>]

On 18/05/2026 11:23, Jagadeesh Kona wrote:
> 
> 
> 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@oss.qualcomm.com/
> https://lore.kernel.org/all/009ecdbb-2297-44eb-862d-233e3290691c@oss.qualcomm.com/
> 
> Thanks,
> Jagadeesh

That's not in your overview letter so generally I'd advise to include 
things like "did X because Y" - "didn't do Q because Z" anyway, how does 
it make a difference if the values are static ?

They are no less magic numbers that way.

What exactly is the resistance to defining the bits ?

I'll state again - when a vendor is submitting something upstream where 
that vendor 100% controls their own documentation - there's no reason at 
all to be presenting magic hex numbers - even more the case with 
generated code.

Just update the script to enumerate the bit fields, I honestly don't get 
the aversion.

---
bod