Re: [PATCH 2/2] arm64: dts: qcom: sm8550: Additionally manage MXC power domain in camcc

[Taniya Das <taniya.das@oss.qualcomm.com>]

On 10/21/2025 8:26 PM, Luca Weiss wrote:
> On Tue Oct 21, 2025 at 4:24 PM CEST, Luca Weiss wrote:
>> Hi Taniya,
>>
>> On Tue Oct 21, 2025 at 1:12 PM CEST, Taniya Das wrote:
>>>
>>>
>>> On 10/21/2025 3:28 PM, Luca Weiss wrote:
>>>> Hi Vladimir,
>>>>
>>>> On Tue Oct 21, 2025 at 11:48 AM CEST, Vladimir Zapolskiy wrote:
>>>>> Hi Luca.
>>>>>
>>>>> On 10/17/25 17:05, Luca Weiss wrote:
>>>>>> Hi Taniya,
>>>>>>
>>>>>> On Thu Mar 13, 2025 at 12:57 PM CET, Taniya Das wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 3/13/2025 1:22 PM, Luca Weiss wrote:
>>>>>>>> Hi Taniya,
>>>>>>>>
>>>>>>>> On Thu Mar 13, 2025 at 5:39 AM CET, Taniya Das wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 3/4/2025 2:10 PM, Dmitry Baryshkov wrote:
>>>>>>>>>> On Tue, 4 Mar 2025 at 09:37, Vladimir Zapolskiy
>>>>>>>>>> <vladimir.zapolskiy@linaro.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 3/4/25 01:53, Dmitry Baryshkov wrote:
>>>>>>>>>>>> On Tue, Mar 04, 2025 at 12:55:21AM +0200, Vladimir Zapolskiy wrote:
>>>>>>>>>>>>> SM8550 Camera Clock Controller shall enable both MXC and MMCX power
>>>>>>>>>>>>> domains.
>>>>>>>>>>>>
>>>>>>>>>>>> Are those really required to access the registers of the cammcc? Or is
>>>>>>>>>>>> one of those (MXC?) required to setup PLLs? Also, is this applicable
>>>>>>>>>>>> only to sm8550 or to other similar clock controllers?
>>>>>>>>>>>
>>>>>>>>>>> Due to the described problem I experience a fatal CPU stall on SM8550-QRD,
>>>>>>>>>>> not on any SM8450 or SM8650 powered board for instance, however it does
>>>>>>>>>>> not exclude an option that the problem has to be fixed for other clock
>>>>>>>>>>> controllers, but it's Qualcomm to confirm any other touched platforms,
>>>>>>>>>>
>>>>>>>>>> Please work with Taniya to identify used power domains.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> CAMCC requires both MMCX and MXC to be functional.
>>>>>>>>
>>>>>>>> Could you check whether any clock controllers on SM6350/SM7225 (Bitra)
>>>>>>>> need multiple power domains, or in general which clock controller uses
>>>>>>>> which power domain.
>>>>>>>>
>>>>>>>> That SoC has camcc, dispcc, gcc, gpucc, npucc and videocc.
>>>>>>>>
>>>>>>>> That'd be highly appreciated since I've been hitting weird issues there
>>>>>>>> that could be explained by some missing power domains.
>>>>>>>>
>>>>>>>
>>>>>>> Hi Luca,
>>>>>>>
>>>>>>> The targets you mentioned does not have any have multiple rail
>>>>>>> dependency, but could you share the weird issues with respect to clock
>>>>>>> controller I can take a look.
>>>>>>
>>>>>> Coming back to this, I've taken a shot at camera on SM6350 (Fairphone 4)
>>>>>> again, but again hitting some clock issues.
>>>>>>
>>>>>> For reference, I am testing with following change:
>>>>>> https://lore.kernel.org/linux-arm-msm/20250911011218.861322-3-vladimir.zapolskiy@linaro.org/
>>>>>>
>>>>>> Trying to enable CAMCC_MCLK1_CLK - wired up to the IMX576 camera sensor
>>>>>> on this phone - results in following error.
>>>>>>
>>>>>> [    3.140232] ------------[ cut here ]------------
>>>>>> [    3.141264] camcc_mclk1_clk status stuck at 'off'
>>>>>> [    3.141276] WARNING: CPU: 6 PID: 12 at drivers/clk/qcom/clk-branch.c:87 clk_branch_toggle+0x170/0x190
>>>>>>
>>>>>> Checking the driver against downstream driver, it looks like the RCGs
>>>>>> should be using clk_rcg2_shared_ops because of enable_safe_config in
>>>>>> downstream, but changing that doesn't really improve the situation, but
>>>>>> it does change the error message to this:
>>>>>>
>>>>>> [    2.933254] ------------[ cut here ]------------
>>>>>> [    2.933961] camcc_mclk1_clk_src: rcg didn't update its configuration.
>>>>>> [    2.933970] WARNING: CPU: 7 PID: 12 at drivers/clk/qcom/clk-rcg2.c:136 update_config+0xd4/0xe4
>>>>>>
>>>>>> I've also noticed that some camcc drivers take in GCC_CAMERA_AHB_CLK as
>>>>>> iface clk, could something like this be missing on sm6350?
>>>>>>
>>>>>> I'd appreciate any help or tips for resolving this.
>>>>>>
>>>>>
>>>>> Recently one particular problem related to MCLK was identified by me on
>>>>> QRB5165/RB5, and it was reported to Bjorn over IRC, namely it's not possible
>>>>> to toggle MCLK clock enable/disable state, when TITAN GDSC power domain is
>>>>> set off. I'm working on fixing the issue (a change under clk/qcom), since
>>>>> it's of an importance for a customer as well.
>>>>>
>>>>> I can't be totally sure that it's right the same problem as the one reported
>>>>> by you above, but it looks very similar, as a fast workaround please consider
>>>>> to set an ALWAYS_ON flag of TITAN GDSC, and at least a report from you that
>>>>> this actually helps would be nice to get.
>>>>
>>>> Unfortunately that doesn't seem to help on sm6350.
>>>>
>>>> diff --git a/drivers/clk/qcom/camcc-sm6350.c b/drivers/clk/qcom/camcc-sm6350.c
>>>> index 12a469ce7e2f..cf87ad55d318 100644
>>>> --- a/drivers/clk/qcom/camcc-sm6350.c
>>>> +++ b/drivers/clk/qcom/camcc-sm6350.c
>>>> @@ -1767,6 +1767,7 @@ static struct gdsc titan_top_gdsc = {
>>>>  		.name = "titan_top_gdsc",
>>>>  	},
>>>>  	.pwrsts = PWRSTS_OFF_ON,
>>>> +	.flags = ALWAYS_ON,
>>>>  };
>>>>  
>>>>  static struct clk_hw *camcc_sm6350_hws[] = {
>>>>
>>>>
>>>> $ cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>>>> [...]
>>>> titan_top_gdsc                  on                              0
>>>>                                                 bps_gdsc, ipe_0_gdsc, ife_0_gdsc, ife_1_gdsc, ife_2_gdsc
>>>>     ac4a000.cci                     suspended                   0           SW
>>>>     ac4b000.cci                     suspended                   0           SW
>>>>     genpd:3:acb3000.camss           suspended                   0           SW
>>>> [...]
>>>>
>>>> but still the same clock stuck warning...
>>>>
>>>> [    3.093431] ------------[ cut here ]------------
>>>> [    3.094614] camcc_mclk1_clk status stuck at 'off'
>>>> [    3.094629] WARNING: CPU: 6 PID: 65 at drivers/clk/qcom/clk-branch.c:87 clk_branch_toggle+0x170/0x190
>>>>
>>>> Thanks for the suggestion though.
>>>>
>>>
>>> Hi Luca,
>>>
>>> Seems like the CAMCC_PLL2_OUT_EARLY output could be OFF here, which is
>>> sourcing the mclk RCG's.
>>>
>>> The user_ctl is not properly configured to enable the PLL early output.
>>> Can you please try below change and check if it helps?
>>>
>>> diff --git a/drivers/clk/qcom/camcc-sm6350.c
>>> b/drivers/clk/qcom/camcc-sm6350.c
>>> index 8aac97d29ce3..d33db530b7c9 100644
>>> --- a/drivers/clk/qcom/camcc-sm6350.c
>>> +++ b/drivers/clk/qcom/camcc-sm6350.c
>>> @@ -154,6 +154,7 @@ static const struct alpha_pll_config
>>> camcc_pll2_config = {
>>>         .config_ctl_hi_val = 0x400003d2,
>>>         .test_ctl_val = 0x04000400,
>>>         .test_ctl_hi_val = 0x00004000,
>>> +       .user_ctl_val = 0x0000030F,
>>> };
>>
>> Yes! Indeed that makes the clock not be stuck, and debugcc is also
>> correctly reporting ~24MHz from that clock when it's enabled!
>>
>>   cam_cc_mclk1_clk: 23.999592MHz (23999592Hz)
>>
>> Is this PLL also missing a value for .user_ctl_hi_val? The other PLLs
>> have that set as well.
>>
>> Out of curiousity, where's this magic value from? Only some sort of HPG
>> doc, or is this also somewhere referenced in downstream? Curious why
>> this is not set there for this PLL.
> 

These are part of recommended PLL settings from design.

> Ah, just saw this part in downstream
> https://git.codelinaro.org/clo/la/kernel/msm-4.19/-/blob/LA.UM.9.12.c25-02800-SMxx50.QSSI13c28.0/drivers/clk/qcom/clk-alpha-pll.c#L2443-2463
> 
> That's quite different to the simple
> 
> 	clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll),
> 							config->user_ctl_val);
> 
> that's in upstream.
> 

yes, we can set all the required configs in the user_ctl_val.

> Also explains makes it clear that .user_ctl_hi_val is not applicable to
> this agera PLL.
> 

Yes you are correct.

> From looking at camcc-sm7150, I guess they have the same problem over
> there. A bunch of post_div and _mask's set, but no .user_ctl_val set for
> PLL2. But camcc-sc7180 is fine though.
> 

In SC7180, we agreed to take care of all configurations as part of the
user_ctl_val.

>>
>> This is one major step toward camss & camera support for this phone!
>>
>> Regards
>> Luca
> 

-- 
Thanks,
Taniya Das