public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: "Luca Weiss" <luca.weiss@fairphone.com>
To: "Bryan O'Donoghue" <bod@kernel.org>,
	"Luca Weiss" <luca.weiss@fairphone.com>,
	"Vladimir Zapolskiy" <vladimir.zapolskiy@linaro.org>
Cc: "Konrad Dybcio" <konradybcio@kernel.org>,
	"Bryan O'Donoghue" <bryan.odonoghue@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Robert Foss" <rfoss@kernel.org>,
	<~postmarketos/upstreaming@lists.sr.ht>,
	<phone-devel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
	<linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@oss.qualcomm.com>,
	"Todor Tomov" <todor.too@gmail.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Bjorn Andersson" <andersson@kernel.org>
Subject: Re: [PATCH v4 0/3] Add CAMSS support for SM6350
Date: Fri, 01 May 2026 12:12:04 +0200	[thread overview]
Message-ID: <DI79CX4PU08J.2M2V0U4PTOVEU@fairphone.com> (raw)
In-Reply-To: <c87d229c-137c-4e59-99cc-a97ef04f6e1b@kernel.org>

Hi Bryan,

On Fri Apr 3, 2026 at 5:07 PM CEST, Bryan O'Donoghue wrote:
> On 03/04/2026 09:09, Luca Weiss wrote:
>> Hi Vladimir,
>> 
>> On Tue Mar 31, 2026 at 12:49 AM CEST, Vladimir Zapolskiy wrote:
>>> Hi Luca,
>>>
>>> On 2/16/26 10:54, Luca Weiss wrote:
>>>> Add bindings, driver and dts to support the Camera Subsystem on the
>>>> SM6350 SoC.
>>>>
>>>> These patches were tested on a Fairphone 4 smartphone with WIP sensor
>>>> drivers (Sony IMX576 and IMX582), the camera pipeline works properly as
>>>> far as I can tell.
>>>>
>>>> Though when stopping the camera stream, the following clock warning
>>>> appears in dmesg. But it does not interfere with any functionality,
>>>> starting and stopping the stream works and debugcc is showing 426.4 MHz
>>>> while the clock is on, and 'off' while it's off.
>>>>
>>>> Any suggestion how to fix this, is appreciated.
>>>
>>> I've looked at CAMCC recently, and I do notice that SM6350 CAMCC does not
>>> set '.use_rpm = true' flag for whatever reason.
>>>
>>> If you find a free minute, can you test the change below?..
>> 
>> Unfortunately that change does not resolve the "gcc_camera_axi_clk
>> status stuck at 'on'" warning.
>> 
>> fairphone-fp4:~$ cat /sys/bus/platform/drivers/sm6350-camcc/ad00000.clock-controller/power/runtime_status
>> active
>> 
>> fairphone-fp4:~$ cat /sys/bus/platform/drivers/sm6350-camcc/ad00000.clock-controller/power/runtime_status
>> suspended
>> 
>>>
>>> ----8<----
>>> diff --git a/drivers/clk/qcom/camcc-sm6350.c b/drivers/clk/qcom/camcc-sm6350.c
>>> index 7df12c1311c6..ba880e4edcaf 100644
>>> --- a/drivers/clk/qcom/camcc-sm6350.c
>>> +++ b/drivers/clk/qcom/camcc-sm6350.c
>>> @@ -1880,6 +1880,7 @@ static const struct qcom_cc_desc camcc_sm6350_desc = {
>>>    	.num_clks = ARRAY_SIZE(camcc_sm6350_clocks),
>>>    	.gdscs = camcc_sm6350_gdscs,
>>>    	.num_gdscs = ARRAY_SIZE(camcc_sm6350_gdscs),
>>> +	.use_rpm = true,
>>>    };
>>>
>>>    static const struct of_device_id camcc_sm6350_match_table[] = {
>>> ----8<----
>>>
>>> This change could be considered to be included in any case, I believe.
>> 
>> I guess this change is now the way to enable pm_runtime, I had this
>> series 3 years ago in February 2023:
>> https://lore.kernel.org/linux-arm-msm/20230213-sm6350-camcc-runtime_pm-v3-0-d35e0d833cc4@fairphone.com/
>> 
>> But I never followed up due to me not understanding pm_runtime well and
>> no direct need for it.
>> 
>> But I guess reviving that with use_rpm = true, add power-domains &
>> required-opps to dt-bindings and sm6350.dtsi should be a good idea?
>> 
>> Regards
>> Luca
>> 
>>>
>>>> [ 5738.590980] ------------[ cut here ]------------
>>>> [ 5738.591009] gcc_camera_axi_clk status stuck at 'on'
>>>> [ 5738.591049] WARNING: CPU: 0 PID: 6918 at drivers/clk/qcom/clk-branch.c:87 clk_branch_toggle+0x170/0x190
>>>> [ 5738.591081] Modules linked in:
>>>> [ 5738.591099] CPU: 0 UID: 10000 PID: 6918 Comm: plasma-camera Tainted: G        W           6.17.0-00057-ge6b67db49622 #71 NONE
>>>> [ 5738.591118] Tainted: [W]=WARN
>>>> [ 5738.591126] Hardware name: Fairphone 4 (DT)
>>>> [ 5738.591136] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>>> [ 5738.591150] pc : clk_branch_toggle+0x170/0x190
>>>> [ 5738.591164] lr : clk_branch_toggle+0x170/0x190
>>>> [ 5738.591177] sp : ffff800086ed3980
>>>> [ 5738.591184] x29: ffff800086ed3990 x28: 0000000000000001 x27: ffff800086ed3cd8
>>>> [ 5738.591208] x26: 0000000000000000 x25: ffffda14fcfbd250 x24: 0000000000000000
>>>> [ 5738.591230] x23: 0000000000000000 x22: ffffda14fc38bce0 x21: 0000000000000000
>>>> [ 5738.591252] x20: ffffda14fd33e618 x19: 0000000000000000 x18: 00000000000064c8
>>>> [ 5738.591274] x17: 0000000000000000 x16: 00001ae003667e9e x15: ffffda14fd2a07b0
>>>> [ 5738.591295] x14: 0000000000000000 x13: 6f27207461206b63 x12: 7574732073757461
>>>> [ 5738.591317] x11: 0000000000000058 x10: 0000000000000018 x9 : ffffda14fd2a0838
>>>> [ 5738.591338] x8 : 0000000000057fa8 x7 : 0000000000000a16 x6 : ffffda14fd2f8838
>>>> [ 5738.591360] x5 : ffff0001f6f59788 x4 : 0000000000000a15 x3 : ffff25ecf9d7e000
>>>> [ 5738.591381] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000baf5c100
>>>> [ 5738.591403] Call trace:
>>>> [ 5738.591412]  clk_branch_toggle+0x170/0x190 (P)
>>>> [ 5738.591429]  clk_branch2_disable+0x1c/0x30
>>>> [ 5738.591445]  clk_core_disable+0x5c/0xb4
>>>> [ 5738.591462]  clk_disable+0x38/0x60
>>>> [ 5738.591478]  camss_disable_clocks+0x44/0x78
>>>> [ 5738.591496]  vfe_put+0x7c/0xc0
>>>> [ 5738.591512]  vfe_set_power+0x40/0x50
>>>> [ 5738.591528]  pipeline_pm_power_one+0x14c/0x150
>>>> [ 5738.591546]  pipeline_pm_power+0x74/0xf4
>>>> [ 5738.591561]  v4l2_pipeline_pm_use+0x54/0x9c
>>>> [ 5738.591577]  v4l2_pipeline_pm_put+0x14/0x40
>>>> [ 5738.591592]  video_unprepare_streaming+0x18/0x24
>>>> [ 5738.591608]  __vb2_queue_cancel+0x4c/0x314
>>>> [ 5738.591626]  vb2_core_streamoff+0x24/0xc8
>>>> [ 5738.591643]  vb2_ioctl_streamoff+0x58/0x98
>>>> [ 5738.591657]  v4l_streamoff+0x24/0x30
>>>> [ 5738.591672]  __video_do_ioctl+0x430/0x4a8
>>>> [ 5738.591689]  video_usercopy+0x2ac/0x680
>>>> [ 5738.591705]  video_ioctl2+0x18/0x40
>>>> [ 5738.591720]  v4l2_ioctl+0x40/0x60
>>>> [ 5738.591734]  __arm64_sys_ioctl+0x90/0xf0
>>>> [ 5738.591750]  invoke_syscall.constprop.0+0x40/0xf0
>>>> [ 5738.591769]  el0_svc_common.constprop.0+0x38/0xd8
>>>> [ 5738.591785]  do_el0_svc+0x1c/0x28
>>>> [ 5738.591801]  el0_svc+0x34/0xe8
>>>> [ 5738.591820]  el0t_64_sync_handler+0xa0/0xe4
>>>> [ 5738.591838]  el0t_64_sync+0x198/0x19c
>>>> [ 5738.591854] ---[ end trace 0000000000000000 ]---
>>>>
>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>> ---
>>>> Changes in v4:
>>>> - Update power-domain-names order (Krzysztof)
>>>> - Make hex numbers lower case in init seq (David)
>>>> - Pick up tags
>>>> - Link to v3: https://lore.kernel.org/r/20260213-sm6350-camss-v3-0-30a845b0b7cc@fairphone.com
>>>
>>> Should find some time myself to issue RBs, sorry for the delay.
>>>
>>>> Changes in v3:
>>>> - Update dt-bindings to include everything related to camss
>>>> - Update regulator names
>>>> - Remove slow_ahb_src
>>>> - Link to v2: https://lore.kernel.org/r/20251114-sm6350-camss-v2-0-d1ff67da33b6@fairphone.com
>>>>
>>>> Changes in v2:
>>>> - Remove prefix from interconnect-names
>>>> - Move 'top' power-domain to the top of list
>>>> - Update regulator supply names
>>>> - Link to v1: https://lore.kernel.org/r/20251024-sm6350-camss-v1-0-63d626638add@fairphone.com
>>>>
>>>> ---
>>>> Luca Weiss (3):
>>>>         dt-bindings: media: camss: Add qcom,sm6350-camss
>>>>         media: qcom: camss: Add SM6350 support
>>>>         arm64: dts: qcom: sm6350: Add CAMSS node
>>>>
>>>>    .../bindings/media/qcom,sm6350-camss.yaml          | 471 +++++++++++++++++++++
>>>>    arch/arm64/boot/dts/qcom/sm6350.dtsi               | 233 ++++++++++
>>>>    .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 125 ++++++
>>>>    drivers/media/platform/qcom/camss/camss-vfe.c      |   2 +
>>>>    drivers/media/platform/qcom/camss/camss.c          | 261 ++++++++++++
>>>>    drivers/media/platform/qcom/camss/camss.h          |   1 +
>>>>    6 files changed, 1093 insertions(+)
>>>> ---
>>>> base-commit: 3daf23347bb5f4a375d0101ed29c97ce1a99721b
>>>> change-id: 20251024-sm6350-camss-9c404bf9cfdd
>>>>
>>>> Best regards,
>> 
>
> What about taking the clock out of hardware gated mode ?
>
> ┌─[deckard@sagittarius-a] - [~/Development/qualcomm/qlt-kernel] - [Fri 
> Apr 03, 16:05]
> └─[$]> git diff
> diff --git a/drivers/clk/qcom/gcc-sm6350.c b/drivers/clk/qcom/gcc-sm6350.c
> index a4d6dff9d0f7f..f98cb35bcd408 100644
> --- a/drivers/clk/qcom/gcc-sm6350.c
> +++ b/drivers/clk/qcom/gcc-sm6350.c
> @@ -909,8 +909,6 @@ static struct clk_branch gcc_camera_ahb_clk = {
>   static struct clk_branch gcc_camera_axi_clk = {
>          .halt_reg = 0x17018,
>          .halt_check = BRANCH_HALT,
> -       .hwcg_reg = 0x17018,
> -       .hwcg_bit = 1,

Unfortunately this change has no effect, still getting the same error

[  192.154311] ------------[ cut here ]------------
[  192.154339] gcc_camera_axi_clk status stuck at 'on'
[  192.154364] WARNING: drivers/clk/qcom/clk-branch.c:87 at clk_branch_toggle+0x170/0x190, CPU#5: CameraManager/5996
[  192.154387] Modules linked in:
[  192.154403] CPU: 5 UID: 10000 PID: 5996 Comm: CameraManager Tainted: G        W           7.0.0-00074-gb9262f98394c-dirty #31 PREEMPTLAZY

Regards
Luca

  reply	other threads:[~2026-05-01 10:12 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-16  8:54 [PATCH v4 0/3] Add CAMSS support for SM6350 Luca Weiss
2026-02-16  8:54 ` [PATCH v4 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss Luca Weiss
2026-03-30 22:57   ` Vladimir Zapolskiy
2026-03-31  6:38     ` Luca Weiss
2026-03-31  8:54       ` Bryan O'Donoghue
2026-03-31  8:50   ` Bryan O'Donoghue
2026-02-16  8:54 ` [PATCH v4 2/3] media: qcom: camss: Add SM6350 support Luca Weiss
2026-03-31 13:20   ` Vladimir Zapolskiy
2026-02-16  8:54 ` [PATCH v4 3/3] arm64: dts: qcom: sm6350: Add CAMSS node Luca Weiss
2026-03-31 13:29   ` Vladimir Zapolskiy
2026-03-30 22:49 ` [PATCH v4 0/3] Add CAMSS support for SM6350 Vladimir Zapolskiy
2026-04-03  8:09   ` Luca Weiss
2026-04-03 10:09     ` Vladimir Zapolskiy
2026-04-03 15:07     ` Bryan O'Donoghue
2026-05-01 10:12       ` Luca Weiss [this message]
2026-05-01 22:32         ` Bryan O'Donoghue
2026-05-04  7:14           ` Luca Weiss
2026-05-04 11:18             ` Bryan O'Donoghue

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=DI79CX4PU08J.2M2V0U4PTOVEU@fairphone.com \
    --to=luca.weiss@fairphone.com \
    --cc=andersson@kernel.org \
    --cc=bod@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzysztof.kozlowski@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=rfoss@kernel.org \
    --cc=robh@kernel.org \
    --cc=todor.too@gmail.com \
    --cc=vladimir.zapolskiy@linaro.org \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /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