public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: "Luca Weiss" <luca.weiss@fairphone.com>
To: "Taniya Das" <taniya.das@oss.qualcomm.com>,
	"Luca Weiss" <luca.weiss@fairphone.com>,
	"Vladimir Zapolskiy" <vladimir.zapolskiy@linaro.org>
Cc: "Bjorn Andersson" <andersson@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Jagadeesh Kona" <quic_jkona@quicinc.com>,
	"Bryan O'Donoghue" <bryan.odonoghue@linaro.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	<linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-clk@vger.kernel.org>,
	"Dmitry Baryshkov" <dmitry.baryshkov@oss.qualcomm.com>
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8550: Additionally manage MXC power domain in camcc
Date: Tue, 21 Oct 2025 16:24:58 +0200	[thread overview]
Message-ID: <DDO2HYG8H2XJ.1MZLPJH36PR85@fairphone.com> (raw)
In-Reply-To: <dca13de5-b027-4938-a854-2538fce52b7c@oss.qualcomm.com>

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.

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

Regards
Luca

  reply	other threads:[~2025-10-21 14:25 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-03 22:55 [PATCH 0/2] arm64: dts: qcom: sm8550: camcc: Manage MMCX and MXC Vladimir Zapolskiy
2025-03-03 22:55 ` [PATCH 1/2] dt-bindings: clock: qcom: sm8450-camcc: Allow to specify two power domains Vladimir Zapolskiy
2025-03-03 23:51   ` Dmitry Baryshkov
2025-03-04  0:31   ` Rob Herring (Arm)
2025-03-03 22:55 ` [PATCH 2/2] arm64: dts: qcom: sm8550: Additionally manage MXC power domain in camcc Vladimir Zapolskiy
2025-03-03 23:53   ` Dmitry Baryshkov
2025-03-04  8:30     ` Vladimir Zapolskiy
2025-03-04  8:37     ` Vladimir Zapolskiy
2025-03-04  8:40       ` Dmitry Baryshkov
2025-03-13  4:39         ` Taniya Das
2025-03-13  7:26           ` Dmitry Baryshkov
2025-03-13  7:52           ` Luca Weiss
2025-03-13 11:57             ` Taniya Das
2025-10-17 14:05               ` Luca Weiss
2025-10-20 12:21                 ` Konrad Dybcio
2025-10-20 13:00                   ` Bryan O'Donoghue
2025-10-21 10:07                     ` Luca Weiss
2025-10-21 10:36                   ` Luca Weiss
2025-10-21 10:39                     ` Konrad Dybcio
2025-10-21  9:48                 ` Vladimir Zapolskiy
2025-10-21  9:58                   ` Luca Weiss
2025-10-21 11:12                     ` Taniya Das
2025-10-21 14:24                       ` Luca Weiss [this message]
2025-10-21 14:56                         ` Luca Weiss
2025-10-22  6:27                           ` Taniya Das
2025-03-12 21:00     ` Bryan O'Donoghue
2025-03-13  4:39     ` Taniya Das
2025-03-13  9:10       ` Bryan O'Donoghue
2025-03-13  9:25         ` Bryan O'Donoghue
2025-03-13 11:32           ` Taniya Das
2025-03-04  1:38 ` [PATCH 0/2] arm64: dts: qcom: sm8550: camcc: Manage MMCX and MXC Rob Herring (Arm)
2025-03-13 11:38 ` Taniya Das
2025-03-26 11:46   ` Jagadeesh Kona

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DDO2HYG8H2XJ.1MZLPJH36PR85@fairphone.com \
    --to=luca.weiss@fairphone.com \
    --cc=andersson@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=quic_jkona@quicinc.com \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=taniya.das@oss.qualcomm.com \
    --cc=vladimir.zapolskiy@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox