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

[Konrad Dybcio]

On 5/25/26 9:49 AM, Bryan O'Donoghue wrote:
> On 25/05/2026 08:06, Jagadeesh Kona wrote:
>>> 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.
>>>
>> Hi Bryan,
>>
>> There’s no standard interface for these bits, and bit definitions/fields vary across PLL types.
>> So, common macros aren’t feasible and would need redefinitions per controller. Since these bits
>> are not reused elsewhere
> 
> - Asking for named bits not common macros
> - Reuse isn't why you name a bit
> 
> , IMO directly using values from the hardware documentation keeps the
>> implementation simpler, avoids unnecessary abstraction, and makes debugging—through direct
>> comparison with the hardware spec easier.
> 
> How are hex values in upstream code easier to debug ?
> 
> Without the spec you can't change or understand hex values in upstream code, which is the whole point I'm making here.

I get the 'understanding' part, but regarding change, as I said
previously, these must remain as-is - any difference for a PLL
impacts every single clock downstream of it. Some of them also
correspond to specific electrical properties, just like with PHY
init sequences. The existing values are a result of tuning and
silicon validation across presumably many, many chip units.

There may be updates (very rarely post the chip going into
production), but I'd assume these would go through the same
testing procedures

Konrad