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

[Dmitry Baryshkov]

On Thu, Jun 11, 2026 at 10:51:21AM +0200, Konrad Dybcio wrote:
> 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

The discussion makes me wonder, should we drop various _mask and _val
bits from the struct alpha_pll_config and use .user_ctl_val as it's done
for the new PLL types? Using both potentially creates inconsistencies.

-- 
With best wishes
Dmitry