Re: [PATCH v2 5/7] clk: qcom: camcc-x1p42100: Add support for camera clock controller

[Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>]

On 3/5/2026 2:35 PM, Konrad Dybcio wrote:
> On 3/5/26 3:18 AM, Bryan O'Donoghue wrote:
>> On 05/03/2026 00:33, Dmitry Baryshkov wrote:
>>> I've cross-checked this against X1E80100 driver. The main changes are a
>>> drop of IFE_1, SPE_0, and two PLLs. However it also:

>>> - uses hw_clk_ctrl for several clocks
>>> - uses rcg2_shared_ops instead of rcg2_ops for several clocks

Above 2 are safe to have recommendations from HW.

>>> - uses hwcg_reg and BRANCH_HALT_VOTED for cam_cc_camnoc_axi_nrt_clk

>>> - uses HW_CTRL_TRIGGER for cam_cc_bps_gdsc and cam_cc_ipe_0_gdsc

These 2 GDSC's have support for HW control mode, so added this flag and
consumer drivers can switch to HW control mode based on their requirement.

>>> - uses non-AO clock for cam_cc_xo_clk_src
>>>

Both XO or non-AO should be fine here. Ideally if CC has any clocks with
CLK_IS_CRITICAL flag, then AO parenting is required to allow XO low power modes.


>>> Are all these changes expected? Are any of them also applicable to X1E?
>>>
>>> At this point, I'm torn between suggesting the merge of this driver into
>>> X1E driver and ack'ing the current form.
>>
>> We can test the diff but, I'm not sure that will really answer the question if it is the right-thing-to-do.
>>
>> OTOH if it ain't broke, don't fix it.
>>
>> Reverse the question - is there any reason to have this driver at all ? Can the x1e CAMCC be used as-is ?
>>
>> If not, then we can accept this patch and potentially look at merging the two drivers later on.
>>
>> I assume the code submitted has a purpose though i.e. its not possible to just use Hamoa and Purwa interchangably.
>>
>> A few community members showed me CAMSS working on Purwa last year in Amsterdam with the x1e code - one error if I recall was a clock splat.
>>
>> So superficially it adds up to me that its not a 1:1 thing with these two parts.
> 
> The difference between 'can/does it work in some simple use case' vs 'is it
> correct' is that the exact match for clock configurations between H and P
> is (according to the computer) 4 clocks (out of 200+ in the camcc topology).
> 
> Most of the changes are small differences in frequency steps or which PLL
> is used for a given OPP etc, which ends up being small in the Linux
> representation of that data since many of the freq tables are reused 3, 4,
> 5 times and many clocks (branches) don't even feature one.
> 
> I would imagine almost all of the points raised by Dmitry probably apply
> (but I'll let the people in the know comment on that), which would greatly
> reduce the effective diff. If they do, the drivers could indeed be merged
> since the delta would be just those couple freq tables and NULLifying 13
> clocks on Purwa
> 

There is frequency table delta for most RCG's since Hamoa has an extra LowSVS_D1 corner,
but along with that, few frequencies like 480MHz for cam_cc_icp_clk_src...etc is derived
from PLL8 on Hamoa, but the same is derived from PLL6 on Purwa. 

To handle above, change is required in cam_cc_parent_map_0, cam_cc_parent_data_0 structures
from X1E and these structures are used by many RCG's and all those RCG's also needs to be
updated, so overall it is significant delta and hence it is good to have this separate driver.

Thanks,
Jagadeesh