* [PATCH v2 0/3] Add CAMSS support for SM6350 @ 2025-11-14 11:15 ` Luca Weiss 2025-11-14 11:15 ` [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss Luca Weiss ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: Luca Weiss @ 2025-11-14 11:15 UTC (permalink / raw) To: Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel, Luca Weiss 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. [ 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 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 | 349 +++++++++++++++++++++ arch/arm64/boot/dts/qcom/sm6350.dtsi | 165 ++++++++++ .../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 | 249 +++++++++++++++ drivers/media/platform/qcom/camss/camss.h | 1 + 6 files changed, 891 insertions(+) --- base-commit: a92c761bcac3d5042559107fa7679470727a4bcb change-id: 20251024-sm6350-camss-9c404bf9cfdd Best regards, -- Luca Weiss <luca.weiss@fairphone.com> ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss 2025-11-14 11:15 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Luca Weiss @ 2025-11-14 11:15 ` Luca Weiss 2025-11-14 12:40 ` Vladimir Zapolskiy 2025-11-14 12:56 ` Rob Herring (Arm) 2025-11-14 11:15 ` [PATCH v2 2/3] media: qcom: camss: Add SM6350 support Luca Weiss ` (2 subsequent siblings) 3 siblings, 2 replies; 20+ messages in thread From: Luca Weiss @ 2025-11-14 11:15 UTC (permalink / raw) To: Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel, Luca Weiss Add bindings for the Camera Subsystem on the SM6350 SoC. Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- .../bindings/media/qcom,sm6350-camss.yaml | 349 +++++++++++++++++++++ 1 file changed, 349 insertions(+) diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml new file mode 100644 index 000000000000..d812b5b50c05 --- /dev/null +++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml @@ -0,0 +1,349 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/qcom,sm6350-camss.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm SM6350 Camera Subsystem (CAMSS) + +maintainers: + - Luca Weiss <luca.weiss@fairphone.com> + +description: + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms. + +properties: + compatible: + const: qcom,sm6350-camss + + reg: + maxItems: 12 + + reg-names: + items: + - const: csid0 + - const: csid1 + - const: csid2 + - const: csid_lite + - const: csiphy0 + - const: csiphy1 + - const: csiphy2 + - const: csiphy3 + - const: vfe0 + - const: vfe1 + - const: vfe2 + - const: vfe_lite + + clocks: + maxItems: 30 + + clock-names: + items: + - const: cam_ahb_clk + - const: cam_axi + - const: soc_ahb + - const: camnoc_axi + - const: core_ahb + - const: cpas_ahb + - const: csiphy0 + - const: csiphy0_timer + - const: csiphy1 + - const: csiphy1_timer + - const: csiphy2 + - const: csiphy2_timer + - const: csiphy3 + - const: csiphy3_timer + - const: slow_ahb_src + - const: vfe0_axi + - const: vfe0 + - const: vfe0_cphy_rx + - const: vfe0_csid + - const: vfe1_axi + - const: vfe1 + - const: vfe1_cphy_rx + - const: vfe1_csid + - const: vfe2_axi + - const: vfe2 + - const: vfe2_cphy_rx + - const: vfe2_csid + - const: vfe_lite + - const: vfe_lite_cphy_rx + - const: vfe_lite_csid + + interrupts: + maxItems: 12 + + interrupt-names: + items: + - const: csid0 + - const: csid1 + - const: csid2 + - const: csid_lite + - const: csiphy0 + - const: csiphy1 + - const: csiphy2 + - const: csiphy3 + - const: vfe0 + - const: vfe1 + - const: vfe2 + - const: vfe_lite + + interconnects: + maxItems: 4 + + interconnect-names: + items: + - const: ahb + - const: hf_mnoc + - const: sf_mnoc + - const: sf_icp_mnoc + + iommus: + maxItems: 4 + + power-domains: + items: + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller. + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller. + - description: IFE2 GDSC - Image Front End, Global Distributed Switch Controller. + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller. + + power-domain-names: + items: + - const: top + - const: ife0 + - const: ife1 + - const: ife2 + + vdd-csiphy-0p9-supply: + description: + Phandle to a 0.9V regulator supply to a PHY. + + vdd-csiphy-1p25-supply: + description: + Phandle to a 1.25V regulator supply to a PHY. + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + description: + CSI input ports. + + patternProperties: + "^port@[0-3]$": + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + + description: + Input port for receiving CSI data from a CSIPHY. + + properties: + endpoint: + $ref: video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + minItems: 1 + maxItems: 4 + + bus-type: + enum: + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY + + required: + - data-lanes + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - interrupts + - interrupt-names + - interconnects + - interconnect-names + - iommus + - power-domains + - power-domain-names + - vdd-csiphy-0p9-supply + - vdd-csiphy-1p25-supply + - ports + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/qcom,gcc-sm6350.h> + #include <dt-bindings/clock/qcom,sm6350-camcc.h> + #include <dt-bindings/interconnect/qcom,icc.h> + #include <dt-bindings/interconnect/qcom,sm6350.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/media/video-interfaces.h> + #include <dt-bindings/power/qcom-rpmpd.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + isp@acb3000 { + compatible = "qcom,sm6350-camss"; + + reg = <0x0 0x0acb3000 0x0 0x1000>, + <0x0 0x0acba000 0x0 0x1000>, + <0x0 0x0acc1000 0x0 0x1000>, + <0x0 0x0acc8000 0x0 0x1000>, + <0x0 0x0ac65000 0x0 0x1000>, + <0x0 0x0ac66000 0x0 0x1000>, + <0x0 0x0ac67000 0x0 0x1000>, + <0x0 0x0ac68000 0x0 0x1000>, + <0x0 0x0acaf000 0x0 0x4000>, + <0x0 0x0acb6000 0x0 0x4000>, + <0x0 0x0acbd000 0x0 0x4000>, + <0x0 0x0acc4000 0x0 0x4000>; + reg-names = "csid0", + "csid1", + "csid2", + "csid_lite", + "csiphy0", + "csiphy1", + "csiphy2", + "csiphy3", + "vfe0", + "vfe1", + "vfe2", + "vfe_lite"; + + clocks = <&gcc GCC_CAMERA_AHB_CLK>, + <&gcc GCC_CAMERA_AXI_CLK>, + <&camcc CAMCC_SOC_AHB_CLK>, + <&camcc CAMCC_CAMNOC_AXI_CLK>, + <&camcc CAMCC_CORE_AHB_CLK>, + <&camcc CAMCC_CPAS_AHB_CLK>, + <&camcc CAMCC_CSIPHY0_CLK>, + <&camcc CAMCC_CSI0PHYTIMER_CLK>, + <&camcc CAMCC_CSIPHY1_CLK>, + <&camcc CAMCC_CSI1PHYTIMER_CLK>, + <&camcc CAMCC_CSIPHY2_CLK>, + <&camcc CAMCC_CSI2PHYTIMER_CLK>, + <&camcc CAMCC_CSIPHY3_CLK>, + <&camcc CAMCC_CSI3PHYTIMER_CLK>, + <&camcc CAMCC_SLOW_AHB_CLK_SRC>, + <&camcc CAMCC_IFE_0_AXI_CLK>, + <&camcc CAMCC_IFE_0_CLK>, + <&camcc CAMCC_IFE_0_CPHY_RX_CLK>, + <&camcc CAMCC_IFE_0_CSID_CLK>, + <&camcc CAMCC_IFE_1_AXI_CLK>, + <&camcc CAMCC_IFE_1_CLK>, + <&camcc CAMCC_IFE_1_CPHY_RX_CLK>, + <&camcc CAMCC_IFE_1_CSID_CLK>, + <&camcc CAMCC_IFE_2_AXI_CLK>, + <&camcc CAMCC_IFE_2_CLK>, + <&camcc CAMCC_IFE_2_CPHY_RX_CLK>, + <&camcc CAMCC_IFE_2_CSID_CLK>, + <&camcc CAMCC_IFE_LITE_CLK>, + <&camcc CAMCC_IFE_LITE_CPHY_RX_CLK>, + <&camcc CAMCC_IFE_LITE_CSID_CLK>; + clock-names = "cam_ahb_clk", + "cam_axi", + "soc_ahb", + "camnoc_axi", + "core_ahb", + "cpas_ahb", + "csiphy0", + "csiphy0_timer", + "csiphy1", + "csiphy1_timer", + "csiphy2", + "csiphy2_timer", + "csiphy3", + "csiphy3_timer", + "slow_ahb_src", + "vfe0_axi", + "vfe0", + "vfe0_cphy_rx", + "vfe0_csid", + "vfe1_axi", + "vfe1", + "vfe1_cphy_rx", + "vfe1_csid", + "vfe2_axi", + "vfe2", + "vfe2_cphy_rx", + "vfe2_csid", + "vfe_lite", + "vfe_lite_cphy_rx", + "vfe_lite_csid"; + + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 717 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 718 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "csid0", + "csid1", + "csid2", + "csid_lite", + "csiphy0", + "csiphy1", + "csiphy2", + "csiphy3", + "vfe0", + "vfe1", + "vfe2", + "vfe_lite"; + + interconnects = <&gem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY + &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>, + <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, + <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, + <&mmss_noc MASTER_CAMNOC_ICP QCOM_ICC_TAG_ALWAYS + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>; + interconnect-names = "ahb", + "hf_mnoc", + "sf_mnoc", + "sf_icp_mnoc"; + + iommus = <&apps_smmu 0x820 0xc0>, + <&apps_smmu 0x840 0x0>, + <&apps_smmu 0x860 0xc0>, + <&apps_smmu 0x880 0x0>; + + power-domains = <&camcc TITAN_TOP_GDSC> + <&camcc IFE_0_GDSC>, + <&camcc IFE_1_GDSC>, + <&camcc IFE_2_GDSC>; + power-domain-names = "top", + "ife0", + "ife1", + "ife2"; + + vdd-csiphy-0p9-supply = <&vreg_l18a>; + vdd-csiphy-1p25-supply = <&vreg_l22a>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + csiphy0_ep: endpoint { + data-lanes = <0 1 2 3>; + bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>; + remote-endpoint = <&sensor_ep>; + }; + }; + }; + }; + }; -- 2.51.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss 2025-11-14 11:15 ` [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss Luca Weiss @ 2025-11-14 12:40 ` Vladimir Zapolskiy 2025-11-14 13:06 ` Luca Weiss 2025-11-14 12:56 ` Rob Herring (Arm) 1 sibling, 1 reply; 20+ messages in thread From: Vladimir Zapolskiy @ 2025-11-14 12:40 UTC (permalink / raw) To: Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel Hi Luca. On 11/14/25 13:15, Luca Weiss wrote: > Add bindings for the Camera Subsystem on the SM6350 SoC. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > .../bindings/media/qcom,sm6350-camss.yaml | 349 +++++++++++++++++++++ > 1 file changed, 349 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml > new file mode 100644 > index 000000000000..d812b5b50c05 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml > @@ -0,0 +1,349 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/qcom,sm6350-camss.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm SM6350 Camera Subsystem (CAMSS) > + > +maintainers: > + - Luca Weiss <luca.weiss@fairphone.com> > + > +description: > + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms. > + > +properties: > + compatible: > + const: qcom,sm6350-camss > + > + reg: > + maxItems: 12 > + > + reg-names: > + items: > + - const: csid0 > + - const: csid1 > + - const: csid2 > + - const: csid_lite > + - const: csiphy0 > + - const: csiphy1 > + - const: csiphy2 > + - const: csiphy3 > + - const: vfe0 > + - const: vfe1 > + - const: vfe2 > + - const: vfe_lite > + > + clocks: > + maxItems: 30 > + > + clock-names: > + items: > + - const: cam_ahb_clk > + - const: cam_axi > + - const: soc_ahb > + - const: camnoc_axi > + - const: core_ahb > + - const: cpas_ahb > + - const: csiphy0 > + - const: csiphy0_timer > + - const: csiphy1 > + - const: csiphy1_timer > + - const: csiphy2 > + - const: csiphy2_timer > + - const: csiphy3 > + - const: csiphy3_timer > + - const: slow_ahb_src > + - const: vfe0_axi > + - const: vfe0 > + - const: vfe0_cphy_rx > + - const: vfe0_csid > + - const: vfe1_axi > + - const: vfe1 > + - const: vfe1_cphy_rx > + - const: vfe1_csid > + - const: vfe2_axi > + - const: vfe2 > + - const: vfe2_cphy_rx > + - const: vfe2_csid > + - const: vfe_lite > + - const: vfe_lite_cphy_rx > + - const: vfe_lite_csid The sorting order of this list does not follow the sorting order accepted in the past. I'm very sorry for the vagueness, but I can not pronounce the accepted sorting order name, because it triggers people. > + > + interrupts: > + maxItems: 12 > + > + interrupt-names: > + items: > + - const: csid0 > + - const: csid1 > + - const: csid2 > + - const: csid_lite > + - const: csiphy0 > + - const: csiphy1 > + - const: csiphy2 > + - const: csiphy3 > + - const: vfe0 > + - const: vfe1 > + - const: vfe2 > + - const: vfe_lite > + > + interconnects: > + maxItems: 4 > + > + interconnect-names: > + items: > + - const: ahb > + - const: hf_mnoc > + - const: sf_mnoc > + - const: sf_icp_mnoc Please remove sf_mnoc and sf_icp_mnoc, they are not needed for enabling IP to produce raw images, and one day you may use them somewhere else. > + > + iommus: > + maxItems: 4 > + > + power-domains: > + items: > + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller. > + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller. > + - description: IFE2 GDSC - Image Front End, Global Distributed Switch Controller. > + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller. > + > + power-domain-names: > + items: > + - const: top > + - const: ife0 > + - const: ife1 > + - const: ife2 Note that the list of items and the list of the item descriptions do not correspond to each other. Titan Top GDSC shall be at the end. > + > + vdd-csiphy-0p9-supply: > + description: > + Phandle to a 0.9V regulator supply to a PHY. > + > + vdd-csiphy-1p25-supply: > + description: > + Phandle to a 1.25V regulator supply to a PHY. > + Please reference to the schematics or SoC TRM, does SM6350 SoC have different pads to get supplies to different CSIPHYx IPs? If so, then please provide hardware properties to get a proper correspondence between supplies and CSIPHYx, and make all these properties optional. > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + description: > + CSI input ports. > + > + patternProperties: > + "^port@[0-3]$": > + $ref: /schemas/graph.yaml#/$defs/port-base > + unevaluatedProperties: false > + > + description: > + Input port for receiving CSI data from a CSIPHY. > + > + properties: > + endpoint: > + $ref: video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: > + minItems: 1 > + maxItems: 4 > + > + bus-type: > + enum: > + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY > + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY > + > + required: > + - data-lanes > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - clock-names > + - interrupts > + - interrupt-names > + - interconnects > + - interconnect-names > + - iommus > + - power-domains > + - power-domain-names > + - vdd-csiphy-0p9-supply > + - vdd-csiphy-1p25-supply When a change to add CSIPHYx specific supplies is done, please remove *-supply properties from the list of the requred ones. > + - ports > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/qcom,gcc-sm6350.h> > + #include <dt-bindings/clock/qcom,sm6350-camcc.h> > + #include <dt-bindings/interconnect/qcom,icc.h> > + #include <dt-bindings/interconnect/qcom,sm6350.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/media/video-interfaces.h> > + #include <dt-bindings/power/qcom-rpmpd.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + isp@acb3000 { > + compatible = "qcom,sm6350-camss"; > + > + reg = <0x0 0x0acb3000 0x0 0x1000>, > + <0x0 0x0acba000 0x0 0x1000>, > + <0x0 0x0acc1000 0x0 0x1000>, > + <0x0 0x0acc8000 0x0 0x1000>, > + <0x0 0x0ac65000 0x0 0x1000>, > + <0x0 0x0ac66000 0x0 0x1000>, > + <0x0 0x0ac67000 0x0 0x1000>, > + <0x0 0x0ac68000 0x0 0x1000>, > + <0x0 0x0acaf000 0x0 0x4000>, > + <0x0 0x0acb6000 0x0 0x4000>, > + <0x0 0x0acbd000 0x0 0x4000>, > + <0x0 0x0acc4000 0x0 0x4000>; > + reg-names = "csid0", > + "csid1", > + "csid2", > + "csid_lite", > + "csiphy0", > + "csiphy1", > + "csiphy2", > + "csiphy3", > + "vfe0", > + "vfe1", > + "vfe2", > + "vfe_lite"; > + > + clocks = <&gcc GCC_CAMERA_AHB_CLK>, I believe this clock is critical, and it is set so in the SM6350 GCC driver, therefore it should not be added here. Multiple CAMCC drivers define some of the clocks as "critical" and always enabled, a misconfiguration in this area may cause the reported warning. > + <&gcc GCC_CAMERA_AXI_CLK>, > + <&camcc CAMCC_SOC_AHB_CLK>, > + <&camcc CAMCC_CAMNOC_AXI_CLK>, > + <&camcc CAMCC_CORE_AHB_CLK>, > + <&camcc CAMCC_CPAS_AHB_CLK>, > + <&camcc CAMCC_CSIPHY0_CLK>, > + <&camcc CAMCC_CSI0PHYTIMER_CLK>, > + <&camcc CAMCC_CSIPHY1_CLK>, > + <&camcc CAMCC_CSI1PHYTIMER_CLK>, > + <&camcc CAMCC_CSIPHY2_CLK>, > + <&camcc CAMCC_CSI2PHYTIMER_CLK>, > + <&camcc CAMCC_CSIPHY3_CLK>, > + <&camcc CAMCC_CSI3PHYTIMER_CLK>, > + <&camcc CAMCC_SLOW_AHB_CLK_SRC>, > + <&camcc CAMCC_IFE_0_AXI_CLK>, > + <&camcc CAMCC_IFE_0_CLK>, > + <&camcc CAMCC_IFE_0_CPHY_RX_CLK>, > + <&camcc CAMCC_IFE_0_CSID_CLK>, > + <&camcc CAMCC_IFE_1_AXI_CLK>, > + <&camcc CAMCC_IFE_1_CLK>, > + <&camcc CAMCC_IFE_1_CPHY_RX_CLK>, > + <&camcc CAMCC_IFE_1_CSID_CLK>, > + <&camcc CAMCC_IFE_2_AXI_CLK>, > + <&camcc CAMCC_IFE_2_CLK>, > + <&camcc CAMCC_IFE_2_CPHY_RX_CLK>, > + <&camcc CAMCC_IFE_2_CSID_CLK>, > + <&camcc CAMCC_IFE_LITE_CLK>, > + <&camcc CAMCC_IFE_LITE_CPHY_RX_CLK>, > + <&camcc CAMCC_IFE_LITE_CSID_CLK>; > + clock-names = "cam_ahb_clk", > + "cam_axi", > + "soc_ahb", > + "camnoc_axi", > + "core_ahb", > + "cpas_ahb", > + "csiphy0", > + "csiphy0_timer", > + "csiphy1", > + "csiphy1_timer", > + "csiphy2", > + "csiphy2_timer", > + "csiphy3", > + "csiphy3_timer", > + "slow_ahb_src", > + "vfe0_axi", > + "vfe0", > + "vfe0_cphy_rx", > + "vfe0_csid", > + "vfe1_axi", > + "vfe1", > + "vfe1_cphy_rx", > + "vfe1_csid", > + "vfe2_axi", > + "vfe2", > + "vfe2_cphy_rx", > + "vfe2_csid", > + "vfe_lite", > + "vfe_lite_cphy_rx", > + "vfe_lite_csid"; > + > + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 717 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 718 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>; Interrupt types shall be IRQ_TYPE_EDGE_RISING. > + interrupt-names = "csid0", > + "csid1", > + "csid2", > + "csid_lite", > + "csiphy0", > + "csiphy1", > + "csiphy2", > + "csiphy3", > + "vfe0", > + "vfe1", > + "vfe2", > + "vfe_lite"; > + > + interconnects = <&gem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY > + &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>, > + <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS > + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, > + <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS > + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, > + <&mmss_noc MASTER_CAMNOC_ICP QCOM_ICC_TAG_ALWAYS > + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>; > + interconnect-names = "ahb", > + "hf_mnoc", > + "sf_mnoc", > + "sf_icp_mnoc"; > + > + iommus = <&apps_smmu 0x820 0xc0>, > + <&apps_smmu 0x840 0x0>, > + <&apps_smmu 0x860 0xc0>, > + <&apps_smmu 0x880 0x0>; > + > + power-domains = <&camcc TITAN_TOP_GDSC> It should be the last one in the list, if the settled practice is followed. > + <&camcc IFE_0_GDSC>, > + <&camcc IFE_1_GDSC>, > + <&camcc IFE_2_GDSC>; > + power-domain-names = "top", > + "ife0", > + "ife1", > + "ife2"; > + > + vdd-csiphy-0p9-supply = <&vreg_l18a>; > + vdd-csiphy-1p25-supply = <&vreg_l22a>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + csiphy0_ep: endpoint { An empty line before a child node is always needed. > + data-lanes = <0 1 2 3>; > + bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>; > + remote-endpoint = <&sensor_ep>; > + }; > + }; > + }; > + }; > + }; > -- Best wishes, Vladimir ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss 2025-11-14 12:40 ` Vladimir Zapolskiy @ 2025-11-14 13:06 ` Luca Weiss 2025-11-14 16:09 ` Bryan O'Donoghue 0 siblings, 1 reply; 20+ messages in thread From: Luca Weiss @ 2025-11-14 13:06 UTC (permalink / raw) To: Vladimir Zapolskiy, Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel Hi Vladimir, On Fri Nov 14, 2025 at 1:40 PM CET, Vladimir Zapolskiy wrote: > Hi Luca. > > On 11/14/25 13:15, Luca Weiss wrote: >> Add bindings for the Camera Subsystem on the SM6350 SoC. >> >> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >> --- >> .../bindings/media/qcom,sm6350-camss.yaml | 349 +++++++++++++++++++++ >> 1 file changed, 349 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml >> new file mode 100644 >> index 000000000000..d812b5b50c05 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml >> @@ -0,0 +1,349 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/qcom,sm6350-camss.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm SM6350 Camera Subsystem (CAMSS) >> + >> +maintainers: >> + - Luca Weiss <luca.weiss@fairphone.com> >> + >> +description: >> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms. >> + >> +properties: >> + compatible: >> + const: qcom,sm6350-camss >> + >> + reg: >> + maxItems: 12 >> + >> + reg-names: >> + items: >> + - const: csid0 >> + - const: csid1 >> + - const: csid2 >> + - const: csid_lite >> + - const: csiphy0 >> + - const: csiphy1 >> + - const: csiphy2 >> + - const: csiphy3 >> + - const: vfe0 >> + - const: vfe1 >> + - const: vfe2 >> + - const: vfe_lite >> + >> + clocks: >> + maxItems: 30 >> + >> + clock-names: >> + items: >> + - const: cam_ahb_clk >> + - const: cam_axi >> + - const: soc_ahb >> + - const: camnoc_axi >> + - const: core_ahb >> + - const: cpas_ahb >> + - const: csiphy0 >> + - const: csiphy0_timer >> + - const: csiphy1 >> + - const: csiphy1_timer >> + - const: csiphy2 >> + - const: csiphy2_timer >> + - const: csiphy3 >> + - const: csiphy3_timer >> + - const: slow_ahb_src >> + - const: vfe0_axi >> + - const: vfe0 >> + - const: vfe0_cphy_rx >> + - const: vfe0_csid >> + - const: vfe1_axi >> + - const: vfe1 >> + - const: vfe1_cphy_rx >> + - const: vfe1_csid >> + - const: vfe2_axi >> + - const: vfe2 >> + - const: vfe2_cphy_rx >> + - const: vfe2_csid >> + - const: vfe_lite >> + - const: vfe_lite_cphy_rx >> + - const: vfe_lite_csid > > The sorting order of this list does not follow the sorting order accepted > in the past. What file should I best reference? > > I'm very sorry for the vagueness, but I can not pronounce the accepted > sorting order name, because it triggers people. > >> + >> + interrupts: >> + maxItems: 12 >> + >> + interrupt-names: >> + items: >> + - const: csid0 >> + - const: csid1 >> + - const: csid2 >> + - const: csid_lite >> + - const: csiphy0 >> + - const: csiphy1 >> + - const: csiphy2 >> + - const: csiphy3 >> + - const: vfe0 >> + - const: vfe1 >> + - const: vfe2 >> + - const: vfe_lite >> + >> + interconnects: >> + maxItems: 4 >> + >> + interconnect-names: >> + items: >> + - const: ahb >> + - const: hf_mnoc >> + - const: sf_mnoc >> + - const: sf_icp_mnoc > > Please remove sf_mnoc and sf_icp_mnoc, they are not needed for enabling > IP to produce raw images, and one day you may use them somewhere else. Ack, will give it a try. > >> + >> + iommus: >> + maxItems: 4 >> + >> + power-domains: >> + items: >> + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller. >> + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller. >> + - description: IFE2 GDSC - Image Front End, Global Distributed Switch Controller. >> + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller. >> + >> + power-domain-names: >> + items: >> + - const: top >> + - const: ife0 >> + - const: ife1 >> + - const: ife2 > > Note that the list of items and the list of the item descriptions do not > correspond to each other. Titan Top GDSC shall be at the end. In the v1 the comment was that top can now be put on top (because a limitation in the driver was fixed). But yes, forgot to modify power-domains description. Will fix. > >> + >> + vdd-csiphy-0p9-supply: >> + description: >> + Phandle to a 0.9V regulator supply to a PHY. >> + >> + vdd-csiphy-1p25-supply: >> + description: >> + Phandle to a 1.25V regulator supply to a PHY. >> + > > Please reference to the schematics or SoC TRM, does SM6350 SoC > have different pads to get supplies to different CSIPHYx IPs? > > If so, then please provide hardware properties to get a proper > correspondence between supplies and CSIPHYx, and make all these > properties optional. I shared the names in replies to v1. * VDD_CAMSS_PLL_0P9 - Camera SS PLL 0.9 V circuits (not referenced in downstream kernel, connected to vreg_s5a in schematics, which is MX) * VDD_A_CSI_x_0P9 - MIPI CSIx 0.9 V circuits With pad names VDD_A_CSI_0_0P9 to VDD_A_CSI_3_0P9 * VDD_A_CSI_x_1P25 - MIPI CSIx 1.25 V circuits With pad names VDD_A_CSI_0_1P25 to VDD_A_CSI_3_1P25 > >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + >> + description: >> + CSI input ports. >> + >> + patternProperties: >> + "^port@[0-3]$": >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + unevaluatedProperties: false >> + >> + description: >> + Input port for receiving CSI data from a CSIPHY. >> + >> + properties: >> + endpoint: >> + $ref: video-interfaces.yaml# >> + unevaluatedProperties: false >> + >> + properties: >> + data-lanes: >> + minItems: 1 >> + maxItems: 4 >> + >> + bus-type: >> + enum: >> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY >> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY >> + >> + required: >> + - data-lanes >> + >> +required: >> + - compatible >> + - reg >> + - reg-names >> + - clocks >> + - clock-names >> + - interrupts >> + - interrupt-names >> + - interconnects >> + - interconnect-names >> + - iommus >> + - power-domains >> + - power-domain-names >> + - vdd-csiphy-0p9-supply >> + - vdd-csiphy-1p25-supply > > When a change to add CSIPHYx specific supplies is done, please remove > *-supply properties from the list of the requred ones. Is this pending some other change that will be posted? Or what do you mean? > >> + - ports >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/qcom,gcc-sm6350.h> >> + #include <dt-bindings/clock/qcom,sm6350-camcc.h> >> + #include <dt-bindings/interconnect/qcom,icc.h> >> + #include <dt-bindings/interconnect/qcom,sm6350.h> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/media/video-interfaces.h> >> + #include <dt-bindings/power/qcom-rpmpd.h> >> + >> + soc { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + isp@acb3000 { >> + compatible = "qcom,sm6350-camss"; >> + >> + reg = <0x0 0x0acb3000 0x0 0x1000>, >> + <0x0 0x0acba000 0x0 0x1000>, >> + <0x0 0x0acc1000 0x0 0x1000>, >> + <0x0 0x0acc8000 0x0 0x1000>, >> + <0x0 0x0ac65000 0x0 0x1000>, >> + <0x0 0x0ac66000 0x0 0x1000>, >> + <0x0 0x0ac67000 0x0 0x1000>, >> + <0x0 0x0ac68000 0x0 0x1000>, >> + <0x0 0x0acaf000 0x0 0x4000>, >> + <0x0 0x0acb6000 0x0 0x4000>, >> + <0x0 0x0acbd000 0x0 0x4000>, >> + <0x0 0x0acc4000 0x0 0x4000>; >> + reg-names = "csid0", >> + "csid1", >> + "csid2", >> + "csid_lite", >> + "csiphy0", >> + "csiphy1", >> + "csiphy2", >> + "csiphy3", >> + "vfe0", >> + "vfe1", >> + "vfe2", >> + "vfe_lite"; >> + >> + clocks = <&gcc GCC_CAMERA_AHB_CLK>, > > I believe this clock is critical, and it is set so in the SM6350 GCC driver, > therefore it should not be added here. True, gcc_camera_ahb_clk has CLK_IS_CRITICAL in gcc-sm6350.c > > Multiple CAMCC drivers define some of the clocks as "critical" and always > enabled, a misconfiguration in this area may cause the reported warning. Will try to remove it then. > >> + <&gcc GCC_CAMERA_AXI_CLK>, >> + <&camcc CAMCC_SOC_AHB_CLK>, >> + <&camcc CAMCC_CAMNOC_AXI_CLK>, >> + <&camcc CAMCC_CORE_AHB_CLK>, >> + <&camcc CAMCC_CPAS_AHB_CLK>, >> + <&camcc CAMCC_CSIPHY0_CLK>, >> + <&camcc CAMCC_CSI0PHYTIMER_CLK>, >> + <&camcc CAMCC_CSIPHY1_CLK>, >> + <&camcc CAMCC_CSI1PHYTIMER_CLK>, >> + <&camcc CAMCC_CSIPHY2_CLK>, >> + <&camcc CAMCC_CSI2PHYTIMER_CLK>, >> + <&camcc CAMCC_CSIPHY3_CLK>, >> + <&camcc CAMCC_CSI3PHYTIMER_CLK>, >> + <&camcc CAMCC_SLOW_AHB_CLK_SRC>, >> + <&camcc CAMCC_IFE_0_AXI_CLK>, >> + <&camcc CAMCC_IFE_0_CLK>, >> + <&camcc CAMCC_IFE_0_CPHY_RX_CLK>, >> + <&camcc CAMCC_IFE_0_CSID_CLK>, >> + <&camcc CAMCC_IFE_1_AXI_CLK>, >> + <&camcc CAMCC_IFE_1_CLK>, >> + <&camcc CAMCC_IFE_1_CPHY_RX_CLK>, >> + <&camcc CAMCC_IFE_1_CSID_CLK>, >> + <&camcc CAMCC_IFE_2_AXI_CLK>, >> + <&camcc CAMCC_IFE_2_CLK>, >> + <&camcc CAMCC_IFE_2_CPHY_RX_CLK>, >> + <&camcc CAMCC_IFE_2_CSID_CLK>, >> + <&camcc CAMCC_IFE_LITE_CLK>, >> + <&camcc CAMCC_IFE_LITE_CPHY_RX_CLK>, >> + <&camcc CAMCC_IFE_LITE_CSID_CLK>; >> + clock-names = "cam_ahb_clk", >> + "cam_axi", >> + "soc_ahb", >> + "camnoc_axi", >> + "core_ahb", >> + "cpas_ahb", >> + "csiphy0", >> + "csiphy0_timer", >> + "csiphy1", >> + "csiphy1_timer", >> + "csiphy2", >> + "csiphy2_timer", >> + "csiphy3", >> + "csiphy3_timer", >> + "slow_ahb_src", >> + "vfe0_axi", >> + "vfe0", >> + "vfe0_cphy_rx", >> + "vfe0_csid", >> + "vfe1_axi", >> + "vfe1", >> + "vfe1_cphy_rx", >> + "vfe1_csid", >> + "vfe2_axi", >> + "vfe2", >> + "vfe2_cphy_rx", >> + "vfe2_csid", >> + "vfe_lite", >> + "vfe_lite_cphy_rx", >> + "vfe_lite_csid"; >> + >> + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 717 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 718 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>; > > Interrupt types shall be IRQ_TYPE_EDGE_RISING. Ack > >> + interrupt-names = "csid0", >> + "csid1", >> + "csid2", >> + "csid_lite", >> + "csiphy0", >> + "csiphy1", >> + "csiphy2", >> + "csiphy3", >> + "vfe0", >> + "vfe1", >> + "vfe2", >> + "vfe_lite"; >> + >> + interconnects = <&gem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY >> + &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>, >> + <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS >> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, >> + <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS >> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, >> + <&mmss_noc MASTER_CAMNOC_ICP QCOM_ICC_TAG_ALWAYS >> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>; >> + interconnect-names = "ahb", >> + "hf_mnoc", >> + "sf_mnoc", >> + "sf_icp_mnoc"; >> + >> + iommus = <&apps_smmu 0x820 0xc0>, >> + <&apps_smmu 0x840 0x0>, >> + <&apps_smmu 0x860 0xc0>, >> + <&apps_smmu 0x880 0x0>; >> + >> + power-domains = <&camcc TITAN_TOP_GDSC> > > It should be the last one in the list, if the settled practice is followed. See above. > >> + <&camcc IFE_0_GDSC>, >> + <&camcc IFE_1_GDSC>, >> + <&camcc IFE_2_GDSC>; >> + power-domain-names = "top", >> + "ife0", >> + "ife1", >> + "ife2"; >> + >> + vdd-csiphy-0p9-supply = <&vreg_l18a>; >> + vdd-csiphy-1p25-supply = <&vreg_l22a>; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + csiphy0_ep: endpoint { > > An empty line before a child node is always needed. Ack > >> + data-lanes = <0 1 2 3>; >> + bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>; >> + remote-endpoint = <&sensor_ep>; >> + }; >> + }; >> + }; >> + }; >> + }; >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss 2025-11-14 13:06 ` Luca Weiss @ 2025-11-14 16:09 ` Bryan O'Donoghue 2025-11-14 17:06 ` Vladimir Zapolskiy 0 siblings, 1 reply; 20+ messages in thread From: Bryan O'Donoghue @ 2025-11-14 16:09 UTC (permalink / raw) To: Luca Weiss, Vladimir Zapolskiy, Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 14/11/2025 13:06, Luca Weiss wrote: > Hi Vladimir, > > On Fri Nov 14, 2025 at 1:40 PM CET, Vladimir Zapolskiy wrote: >> Hi Luca. >> >> On 11/14/25 13:15, Luca Weiss wrote: >>> Add bindings for the Camera Subsystem on the SM6350 SoC. >>> >>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>> --- >>> .../bindings/media/qcom,sm6350-camss.yaml | 349 +++++++++++++++++++++ >>> 1 file changed, 349 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml >>> new file mode 100644 >>> index 000000000000..d812b5b50c05 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml >>> @@ -0,0 +1,349 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/media/qcom,sm6350-camss.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Qualcomm SM6350 Camera Subsystem (CAMSS) >>> + >>> +maintainers: >>> + - Luca Weiss <luca.weiss@fairphone.com> >>> + >>> +description: >>> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms. >>> + >>> +properties: >>> + compatible: >>> + const: qcom,sm6350-camss >>> + >>> + reg: >>> + maxItems: 12 >>> + >>> + reg-names: >>> + items: >>> + - const: csid0 >>> + - const: csid1 >>> + - const: csid2 >>> + - const: csid_lite >>> + - const: csiphy0 >>> + - const: csiphy1 >>> + - const: csiphy2 >>> + - const: csiphy3 >>> + - const: vfe0 >>> + - const: vfe1 >>> + - const: vfe2 >>> + - const: vfe_lite >>> + >>> + clocks: >>> + maxItems: 30 >>> + >>> + clock-names: >>> + items: >>> + - const: cam_ahb_clk >>> + - const: cam_axi >>> + - const: soc_ahb >>> + - const: camnoc_axi >>> + - const: core_ahb >>> + - const: cpas_ahb >>> + - const: csiphy0 >>> + - const: csiphy0_timer >>> + - const: csiphy1 >>> + - const: csiphy1_timer >>> + - const: csiphy2 >>> + - const: csiphy2_timer >>> + - const: csiphy3 >>> + - const: csiphy3_timer >>> + - const: slow_ahb_src >>> + - const: vfe0_axi >>> + - const: vfe0 >>> + - const: vfe0_cphy_rx >>> + - const: vfe0_csid >>> + - const: vfe1_axi >>> + - const: vfe1 >>> + - const: vfe1_cphy_rx >>> + - const: vfe1_csid >>> + - const: vfe2_axi >>> + - const: vfe2 >>> + - const: vfe2_cphy_rx >>> + - const: vfe2_csid >>> + - const: vfe_lite >>> + - const: vfe_lite_cphy_rx >>> + - const: vfe_lite_csid >> >> The sorting order of this list does not follow the sorting order accepted >> in the past. > > What file should I best reference? Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml >> >> I'm very sorry for the vagueness, but I can not pronounce the accepted >> sorting order name, because it triggers people. >> >>> + >>> + interrupts: >>> + maxItems: 12 >>> + >>> + interrupt-names: >>> + items: >>> + - const: csid0 >>> + - const: csid1 >>> + - const: csid2 >>> + - const: csid_lite >>> + - const: csiphy0 >>> + - const: csiphy1 >>> + - const: csiphy2 >>> + - const: csiphy3 >>> + - const: vfe0 >>> + - const: vfe1 >>> + - const: vfe2 >>> + - const: vfe_lite >>> + >>> + interconnects: >>> + maxItems: 4 >>> + >>> + interconnect-names: >>> + items: >>> + - const: ahb >>> + - const: hf_mnoc >>> + - const: sf_mnoc >>> + - const: sf_icp_mnoc >> >> Please remove sf_mnoc and sf_icp_mnoc, they are not needed for enabling >> IP to produce raw images, and one day you may use them somewhere else. > > Ack, will give it a try. Disagree with this. See the Kanaapali patches. I'm asking new submissions to be as complete as possible, instead of limiting the hardware description to the RDI. So listing the ICP noc is the right thing to do. So please include register banks for - bps - cdm - icp - ipe - jpeg - lrme >> >>> + >>> + iommus: >>> + maxItems: 4 >>> + >>> + power-domains: >>> + items: >>> + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller. >>> + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller. >>> + - description: IFE2 GDSC - Image Front End, Global Distributed Switch Controller. >>> + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller. >>> + >>> + power-domain-names: >>> + items: >>> + - const: top >>> + - const: ife0 >>> + - const: ife1 >>> + - const: ife2 >> >> Note that the list of items and the list of the item descriptions do not >> correspond to each other. Titan Top GDSC shall be at the end. > > In the v1 the comment was that top can now be put on top (because a > limitation in the driver was fixed). But yes, forgot to modify > power-domains description. Will fix. > >> >>> + >>> + vdd-csiphy-0p9-supply: >>> + description: >>> + Phandle to a 0.9V regulator supply to a PHY. >>> + >>> + vdd-csiphy-1p25-supply: >>> + description: >>> + Phandle to a 1.25V regulator supply to a PHY. >>> + >> >> Please reference to the schematics or SoC TRM, does SM6350 SoC >> have different pads to get supplies to different CSIPHYx IPs? >> >> If so, then please provide hardware properties to get a proper >> correspondence between supplies and CSIPHYx, and make all these >> properties optional. > > I shared the names in replies to v1. > > * VDD_CAMSS_PLL_0P9 - Camera SS PLL 0.9 V circuits > (not referenced in downstream kernel, connected to vreg_s5a in > schematics, which is MX) > * VDD_A_CSI_x_0P9 - MIPI CSIx 0.9 V circuits > With pad names VDD_A_CSI_0_0P9 to VDD_A_CSI_3_0P9 > * VDD_A_CSI_x_1P25 - MIPI CSIx 1.25 V circuits > With pad names VDD_A_CSI_0_1P25 to VDD_A_CSI_3_1P25 I'm fine with your proposed rail names, they appear to correspond to the voltage values in the docs. > >> >>> + ports: >>> + $ref: /schemas/graph.yaml#/properties/ports >>> + >>> + description: >>> + CSI input ports. >>> + >>> + patternProperties: >>> + "^port@[0-3]$": >>> + $ref: /schemas/graph.yaml#/$defs/port-base >>> + unevaluatedProperties: false >>> + >>> + description: >>> + Input port for receiving CSI data from a CSIPHY. >>> + >>> + properties: >>> + endpoint: >>> + $ref: video-interfaces.yaml# >>> + unevaluatedProperties: false >>> + >>> + properties: >>> + data-lanes: >>> + minItems: 1 >>> + maxItems: 4 >>> + >>> + bus-type: >>> + enum: >>> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY >>> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY >>> + >>> + required: >>> + - data-lanes >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - reg-names >>> + - clocks >>> + - clock-names >>> + - interrupts >>> + - interrupt-names >>> + - interconnects >>> + - interconnect-names >>> + - iommus >>> + - power-domains >>> + - power-domain-names >>> + - vdd-csiphy-0p9-supply >>> + - vdd-csiphy-1p25-supply >> >> When a change to add CSIPHYx specific supplies is done, please remove >> *-supply properties from the list of the requred ones. > > Is this pending some other change that will be posted? Or what do you mean? He means in the current CSIPHY dt its not possible to require these properties. > >> >>> + - ports >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/clock/qcom,gcc-sm6350.h> >>> + #include <dt-bindings/clock/qcom,sm6350-camcc.h> >>> + #include <dt-bindings/interconnect/qcom,icc.h> >>> + #include <dt-bindings/interconnect/qcom,sm6350.h> >>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>> + #include <dt-bindings/media/video-interfaces.h> >>> + #include <dt-bindings/power/qcom-rpmpd.h> >>> + >>> + soc { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + isp@acb3000 { >>> + compatible = "qcom,sm6350-camss"; >>> + >>> + reg = <0x0 0x0acb3000 0x0 0x1000>, >>> + <0x0 0x0acba000 0x0 0x1000>, >>> + <0x0 0x0acc1000 0x0 0x1000>, >>> + <0x0 0x0acc8000 0x0 0x1000>, >>> + <0x0 0x0ac65000 0x0 0x1000>, >>> + <0x0 0x0ac66000 0x0 0x1000>, >>> + <0x0 0x0ac67000 0x0 0x1000>, >>> + <0x0 0x0ac68000 0x0 0x1000>, >>> + <0x0 0x0acaf000 0x0 0x4000>, >>> + <0x0 0x0acb6000 0x0 0x4000>, >>> + <0x0 0x0acbd000 0x0 0x4000>, >>> + <0x0 0x0acc4000 0x0 0x4000>; >>> + reg-names = "csid0", >>> + "csid1", >>> + "csid2", >>> + "csid_lite", >>> + "csiphy0", >>> + "csiphy1", >>> + "csiphy2", >>> + "csiphy3", >>> + "vfe0", >>> + "vfe1", >>> + "vfe2", >>> + "vfe_lite"; >>> + >>> + clocks = <&gcc GCC_CAMERA_AHB_CLK>, >> >> I believe this clock is critical, and it is set so in the SM6350 GCC driver, >> therefore it should not be added here. > > True, gcc_camera_ahb_clk has CLK_IS_CRITICAL in gcc-sm6350.c DT describes hardware, not the happenstance of Linux driver setup. On that basis omitting <&gcc GCC_CAMERA_AHB_CLK> from the clock list is not correct. Because being bornign, can I then reuse this DT in FreeBSD ? No I cannot because it won't describe hardware it will desscirbe Linux-DT which ain't the same thing. >> >> Multiple CAMCC drivers define some of the clocks as "critical" and always >> enabled, a misconfiguration in this area may cause the reported warning. > > Will try to remove it then. I really object to that. DT is a hardware description. Listing the clocks here does no harm and is factually accurate, which again is the point of DT. > >> >>> + <&gcc GCC_CAMERA_AXI_CLK>, >>> + <&camcc CAMCC_SOC_AHB_CLK>, >>> + <&camcc CAMCC_CAMNOC_AXI_CLK>, >>> + <&camcc CAMCC_CORE_AHB_CLK>, >>> + <&camcc CAMCC_CPAS_AHB_CLK>, >>> + <&camcc CAMCC_CSIPHY0_CLK>, >>> + <&camcc CAMCC_CSI0PHYTIMER_CLK>, >>> + <&camcc CAMCC_CSIPHY1_CLK>, >>> + <&camcc CAMCC_CSI1PHYTIMER_CLK>, >>> + <&camcc CAMCC_CSIPHY2_CLK>, >>> + <&camcc CAMCC_CSI2PHYTIMER_CLK>, >>> + <&camcc CAMCC_CSIPHY3_CLK>, >>> + <&camcc CAMCC_CSI3PHYTIMER_CLK>, >>> + <&camcc CAMCC_SLOW_AHB_CLK_SRC>, >>> + <&camcc CAMCC_IFE_0_AXI_CLK>, >>> + <&camcc CAMCC_IFE_0_CLK>, >>> + <&camcc CAMCC_IFE_0_CPHY_RX_CLK>, >>> + <&camcc CAMCC_IFE_0_CSID_CLK>, >>> + <&camcc CAMCC_IFE_1_AXI_CLK>, >>> + <&camcc CAMCC_IFE_1_CLK>, >>> + <&camcc CAMCC_IFE_1_CPHY_RX_CLK>, >>> + <&camcc CAMCC_IFE_1_CSID_CLK>, >>> + <&camcc CAMCC_IFE_2_AXI_CLK>, >>> + <&camcc CAMCC_IFE_2_CLK>, >>> + <&camcc CAMCC_IFE_2_CPHY_RX_CLK>, >>> + <&camcc CAMCC_IFE_2_CSID_CLK>, >>> + <&camcc CAMCC_IFE_LITE_CLK>, >>> + <&camcc CAMCC_IFE_LITE_CPHY_RX_CLK>, >>> + <&camcc CAMCC_IFE_LITE_CSID_CLK>; >>> + clock-names = "cam_ahb_clk", >>> + "cam_axi", >>> + "soc_ahb", >>> + "camnoc_axi", >>> + "core_ahb", >>> + "cpas_ahb", >>> + "csiphy0", >>> + "csiphy0_timer", >>> + "csiphy1", >>> + "csiphy1_timer", >>> + "csiphy2", >>> + "csiphy2_timer", >>> + "csiphy3", >>> + "csiphy3_timer", >>> + "slow_ahb_src", >>> + "vfe0_axi", >>> + "vfe0", >>> + "vfe0_cphy_rx", >>> + "vfe0_csid", >>> + "vfe1_axi", >>> + "vfe1", >>> + "vfe1_cphy_rx", >>> + "vfe1_csid", >>> + "vfe2_axi", >>> + "vfe2", >>> + "vfe2_cphy_rx", >>> + "vfe2_csid", >>> + "vfe_lite", >>> + "vfe_lite_cphy_rx", >>> + "vfe_lite_csid"; >>> + >>> + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 717 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 718 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>; >> >> Interrupt types shall be IRQ_TYPE_EDGE_RISING. > > Ack > >> >>> + interrupt-names = "csid0", >>> + "csid1", >>> + "csid2", >>> + "csid_lite", >>> + "csiphy0", >>> + "csiphy1", >>> + "csiphy2", >>> + "csiphy3", >>> + "vfe0", >>> + "vfe1", >>> + "vfe2", >>> + "vfe_lite"; >>> + >>> + interconnects = <&gem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY >>> + &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>, >>> + <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS >>> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, >>> + <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS >>> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, >>> + <&mmss_noc MASTER_CAMNOC_ICP QCOM_ICC_TAG_ALWAYS >>> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>; >>> + interconnect-names = "ahb", >>> + "hf_mnoc", >>> + "sf_mnoc", >>> + "sf_icp_mnoc"; >>> + >>> + iommus = <&apps_smmu 0x820 0xc0>, >>> + <&apps_smmu 0x840 0x0>, >>> + <&apps_smmu 0x860 0xc0>, >>> + <&apps_smmu 0x880 0x0>; >>> + >>> + power-domains = <&camcc TITAN_TOP_GDSC> >> >> It should be the last one in the list, if the settled practice is followed. > > See above. > >> >>> + <&camcc IFE_0_GDSC>, >>> + <&camcc IFE_1_GDSC>, >>> + <&camcc IFE_2_GDSC>; >>> + power-domain-names = "top", >>> + "ife0", >>> + "ife1", >>> + "ife2"; >>> + >>> + vdd-csiphy-0p9-supply = <&vreg_l18a>; >>> + vdd-csiphy-1p25-supply = <&vreg_l22a>; >>> + >>> + ports { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + port@0 { >>> + reg = <0>; >>> + csiphy0_ep: endpoint { >> >> An empty line before a child node is always needed. > > Ack > >> >>> + data-lanes = <0 1 2 3>; >>> + bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>; >>> + remote-endpoint = <&sensor_ep>; >>> + }; >>> + }; >>> + }; >>> + }; >>> + }; >>> > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss 2025-11-14 16:09 ` Bryan O'Donoghue @ 2025-11-14 17:06 ` Vladimir Zapolskiy 2025-11-16 14:05 ` Bryan O'Donoghue 0 siblings, 1 reply; 20+ messages in thread From: Vladimir Zapolskiy @ 2025-11-14 17:06 UTC (permalink / raw) To: Bryan O'Donoghue, Luca Weiss, Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 11/14/25 18:09, Bryan O'Donoghue wrote: > On 14/11/2025 13:06, Luca Weiss wrote: >> Hi Vladimir, >> >> On Fri Nov 14, 2025 at 1:40 PM CET, Vladimir Zapolskiy wrote: >>> Hi Luca. >>> >>> On 11/14/25 13:15, Luca Weiss wrote: >>>> Add bindings for the Camera Subsystem on the SM6350 SoC. >>>> >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>> --- >>>> .../bindings/media/qcom,sm6350-camss.yaml | 349 +++++++++++++++++++++ >>>> 1 file changed, 349 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml >>>> new file mode 100644 >>>> index 000000000000..d812b5b50c05 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml >>>> @@ -0,0 +1,349 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/media/qcom,sm6350-camss.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Qualcomm SM6350 Camera Subsystem (CAMSS) >>>> + >>>> +maintainers: >>>> + - Luca Weiss <luca.weiss@fairphone.com> >>>> + >>>> +description: >>>> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms. >>>> + >>>> +properties: >>>> + compatible: >>>> + const: qcom,sm6350-camss >>>> + >>>> + reg: >>>> + maxItems: 12 >>>> + >>>> + reg-names: >>>> + items: >>>> + - const: csid0 >>>> + - const: csid1 >>>> + - const: csid2 >>>> + - const: csid_lite >>>> + - const: csiphy0 >>>> + - const: csiphy1 >>>> + - const: csiphy2 >>>> + - const: csiphy3 >>>> + - const: vfe0 >>>> + - const: vfe1 >>>> + - const: vfe2 >>>> + - const: vfe_lite >>>> + >>>> + clocks: >>>> + maxItems: 30 >>>> + >>>> + clock-names: >>>> + items: >>>> + - const: cam_ahb_clk >>>> + - const: cam_axi >>>> + - const: soc_ahb >>>> + - const: camnoc_axi >>>> + - const: core_ahb >>>> + - const: cpas_ahb >>>> + - const: csiphy0 >>>> + - const: csiphy0_timer >>>> + - const: csiphy1 >>>> + - const: csiphy1_timer >>>> + - const: csiphy2 >>>> + - const: csiphy2_timer >>>> + - const: csiphy3 >>>> + - const: csiphy3_timer >>>> + - const: slow_ahb_src >>>> + - const: vfe0_axi >>>> + - const: vfe0 >>>> + - const: vfe0_cphy_rx >>>> + - const: vfe0_csid >>>> + - const: vfe1_axi >>>> + - const: vfe1 >>>> + - const: vfe1_cphy_rx >>>> + - const: vfe1_csid >>>> + - const: vfe2_axi >>>> + - const: vfe2 >>>> + - const: vfe2_cphy_rx >>>> + - const: vfe2_csid >>>> + - const: vfe_lite >>>> + - const: vfe_lite_cphy_rx >>>> + - const: vfe_lite_csid >>> >>> The sorting order of this list does not follow the sorting order accepted >>> in the past. >> >> What file should I best reference? > > Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml > >>> >>> I'm very sorry for the vagueness, but I can not pronounce the accepted >>> sorting order name, because it triggers people. >>> >>>> + >>>> + interrupts: >>>> + maxItems: 12 >>>> + >>>> + interrupt-names: >>>> + items: >>>> + - const: csid0 >>>> + - const: csid1 >>>> + - const: csid2 >>>> + - const: csid_lite >>>> + - const: csiphy0 >>>> + - const: csiphy1 >>>> + - const: csiphy2 >>>> + - const: csiphy3 >>>> + - const: vfe0 >>>> + - const: vfe1 >>>> + - const: vfe2 >>>> + - const: vfe_lite >>>> + >>>> + interconnects: >>>> + maxItems: 4 >>>> + >>>> + interconnect-names: >>>> + items: >>>> + - const: ahb >>>> + - const: hf_mnoc >>>> + - const: sf_mnoc >>>> + - const: sf_icp_mnoc >>> >>> Please remove sf_mnoc and sf_icp_mnoc, they are not needed for enabling >>> IP to produce raw images, and one day you may use them somewhere else. >> >> Ack, will give it a try. > > Disagree with this. > > See the Kanaapali patches. I'm asking new submissions to be as complete > as possible, instead of limiting the hardware description to the RDI. > > So listing the ICP noc is the right thing to do. > > So please include register banks for > > - bps > - cdm > - icp > - ipe > - jpeg > - lrme This suggests to get a DT backward compatibility lock forever, which makes it a very bad advice, which is favourable for just the single interested party, which offers an alternative to the upstream CAMSS. Anybody who wants to get support of any CAMSS ISP functionality above RAW images shall not add anything unrelated and unsupported. The asked inclusion shall be done later on safely, if ever needed. >>> >>>> + >>>> + iommus: >>>> + maxItems: 4 >>>> + >>>> + power-domains: >>>> + items: >>>> + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller. >>>> + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller. >>>> + - description: IFE2 GDSC - Image Front End, Global Distributed Switch Controller. >>>> + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller. >>>> + >>>> + power-domain-names: >>>> + items: >>>> + - const: top >>>> + - const: ife0 >>>> + - const: ife1 >>>> + - const: ife2 >>> >>> Note that the list of items and the list of the item descriptions do not >>> correspond to each other. Titan Top GDSC shall be at the end. >> >> In the v1 the comment was that top can now be put on top (because a >> limitation in the driver was fixed). But yes, forgot to modify >> power-domains description. Will fix. >> >>> >>>> + >>>> + vdd-csiphy-0p9-supply: >>>> + description: >>>> + Phandle to a 0.9V regulator supply to a PHY. >>>> + >>>> + vdd-csiphy-1p25-supply: >>>> + description: >>>> + Phandle to a 1.25V regulator supply to a PHY. >>>> + >>> >>> Please reference to the schematics or SoC TRM, does SM6350 SoC >>> have different pads to get supplies to different CSIPHYx IPs? >>> >>> If so, then please provide hardware properties to get a proper >>> correspondence between supplies and CSIPHYx, and make all these >>> properties optional. >> >> I shared the names in replies to v1. >> >> * VDD_CAMSS_PLL_0P9 - Camera SS PLL 0.9 V circuits >> (not referenced in downstream kernel, connected to vreg_s5a in >> schematics, which is MX) >> * VDD_A_CSI_x_0P9 - MIPI CSIx 0.9 V circuits >> With pad names VDD_A_CSI_0_0P9 to VDD_A_CSI_3_0P9 >> * VDD_A_CSI_x_1P25 - MIPI CSIx 1.25 V circuits >> With pad names VDD_A_CSI_0_1P25 to VDD_A_CSI_3_1P25 > So, if to follow SM8650 CAMSS, the properties shall be: * vdd-csiphy0-0p9-supply, * vdd-csiphy0-1p2-supply, * vdd-csiphy1-0p9-supply, * vdd-csiphy1-1p2-supply, * vdd-csiphy2-0p9-supply, * vdd-csiphy2-1p2-supply, * vdd-csiphy3-0p9-supply, * vdd-csiphy3-1p2-supply, and all of them are optional obviously. This way you may get a future support of proper CSIPHY separation from the already overbloated CAMSS device tree node. > I'm fine with your proposed rail names, they appear to correspond to the > voltage values in the docs. > >> >>> >>>> + ports: >>>> + $ref: /schemas/graph.yaml#/properties/ports >>>> + >>>> + description: >>>> + CSI input ports. >>>> + >>>> + patternProperties: >>>> + "^port@[0-3]$": >>>> + $ref: /schemas/graph.yaml#/$defs/port-base >>>> + unevaluatedProperties: false >>>> + >>>> + description: >>>> + Input port for receiving CSI data from a CSIPHY. >>>> + >>>> + properties: >>>> + endpoint: >>>> + $ref: video-interfaces.yaml# >>>> + unevaluatedProperties: false >>>> + >>>> + properties: >>>> + data-lanes: >>>> + minItems: 1 >>>> + maxItems: 4 >>>> + >>>> + bus-type: >>>> + enum: >>>> + - 1 # MEDIA_BUS_TYPE_CSI2_CPHY >>>> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY >>>> + >>>> + required: >>>> + - data-lanes >>>> + >>>> +required: >>>> + - compatible >>>> + - reg >>>> + - reg-names >>>> + - clocks >>>> + - clock-names >>>> + - interrupts >>>> + - interrupt-names >>>> + - interconnects >>>> + - interconnect-names >>>> + - iommus >>>> + - power-domains >>>> + - power-domain-names >>>> + - vdd-csiphy-0p9-supply >>>> + - vdd-csiphy-1p25-supply >>> >>> When a change to add CSIPHYx specific supplies is done, please remove >>> *-supply properties from the list of the requred ones. >> >> Is this pending some other change that will be posted? Or what do you mean? > > He means in the current CSIPHY dt its not possible to require these > properties. > >> >>> >>>> + - ports >>>> + >>>> +additionalProperties: false >>>> + >>>> +examples: >>>> + - | >>>> + #include <dt-bindings/clock/qcom,gcc-sm6350.h> >>>> + #include <dt-bindings/clock/qcom,sm6350-camcc.h> >>>> + #include <dt-bindings/interconnect/qcom,icc.h> >>>> + #include <dt-bindings/interconnect/qcom,sm6350.h> >>>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>>> + #include <dt-bindings/media/video-interfaces.h> >>>> + #include <dt-bindings/power/qcom-rpmpd.h> >>>> + >>>> + soc { >>>> + #address-cells = <2>; >>>> + #size-cells = <2>; >>>> + >>>> + isp@acb3000 { >>>> + compatible = "qcom,sm6350-camss"; >>>> + >>>> + reg = <0x0 0x0acb3000 0x0 0x1000>, >>>> + <0x0 0x0acba000 0x0 0x1000>, >>>> + <0x0 0x0acc1000 0x0 0x1000>, >>>> + <0x0 0x0acc8000 0x0 0x1000>, >>>> + <0x0 0x0ac65000 0x0 0x1000>, >>>> + <0x0 0x0ac66000 0x0 0x1000>, >>>> + <0x0 0x0ac67000 0x0 0x1000>, >>>> + <0x0 0x0ac68000 0x0 0x1000>, >>>> + <0x0 0x0acaf000 0x0 0x4000>, >>>> + <0x0 0x0acb6000 0x0 0x4000>, >>>> + <0x0 0x0acbd000 0x0 0x4000>, >>>> + <0x0 0x0acc4000 0x0 0x4000>; >>>> + reg-names = "csid0", >>>> + "csid1", >>>> + "csid2", >>>> + "csid_lite", >>>> + "csiphy0", >>>> + "csiphy1", >>>> + "csiphy2", >>>> + "csiphy3", >>>> + "vfe0", >>>> + "vfe1", >>>> + "vfe2", >>>> + "vfe_lite"; >>>> + >>>> + clocks = <&gcc GCC_CAMERA_AHB_CLK>, >>> >>> I believe this clock is critical, and it is set so in the SM6350 GCC driver, >>> therefore it should not be added here. >> >> True, gcc_camera_ahb_clk has CLK_IS_CRITICAL in gcc-sm6350.c > > DT describes hardware, not the happenstance of Linux driver setup. > > On that basis omitting <&gcc GCC_CAMERA_AHB_CLK> from the clock list is > not correct. This has been already discussed, an enumerous amount of Qualcomm/MSM specific resourced like clocks enabled in ABL, Linux etc. are considered critical and not described in the dtb. > > Because being bornign, can I then reuse this DT in FreeBSD ? No I cannot > because it won't describe hardware it will desscirbe Linux-DT which > ain't the same thing. Yes, you can reuse it in FreeBSD, because bug-free FreeBSD sets critical resources as critical and always enabled. >>> >>> Multiple CAMCC drivers define some of the clocks as "critical" and always >>> enabled, a misconfiguration in this area may cause the reported warning. >> >> Will try to remove it then. > > I really object to that. DT is a hardware description. Listing the > clocks here does no harm and is factually accurate, which again is the > point of DT. Here you should argue with linux-arm-msm maintainers, it's not something new. >> >>> >>>> + <&gcc GCC_CAMERA_AXI_CLK>, >>>> + <&camcc CAMCC_SOC_AHB_CLK>, >>>> + <&camcc CAMCC_CAMNOC_AXI_CLK>, >>>> + <&camcc CAMCC_CORE_AHB_CLK>, >>>> + <&camcc CAMCC_CPAS_AHB_CLK>, >>>> + <&camcc CAMCC_CSIPHY0_CLK>, >>>> + <&camcc CAMCC_CSI0PHYTIMER_CLK>, >>>> + <&camcc CAMCC_CSIPHY1_CLK>, >>>> + <&camcc CAMCC_CSI1PHYTIMER_CLK>, >>>> + <&camcc CAMCC_CSIPHY2_CLK>, >>>> + <&camcc CAMCC_CSI2PHYTIMER_CLK>, >>>> + <&camcc CAMCC_CSIPHY3_CLK>, >>>> + <&camcc CAMCC_CSI3PHYTIMER_CLK>, >>>> + <&camcc CAMCC_SLOW_AHB_CLK_SRC>, >>>> + <&camcc CAMCC_IFE_0_AXI_CLK>, >>>> + <&camcc CAMCC_IFE_0_CLK>, >>>> + <&camcc CAMCC_IFE_0_CPHY_RX_CLK>, >>>> + <&camcc CAMCC_IFE_0_CSID_CLK>, >>>> + <&camcc CAMCC_IFE_1_AXI_CLK>, >>>> + <&camcc CAMCC_IFE_1_CLK>, >>>> + <&camcc CAMCC_IFE_1_CPHY_RX_CLK>, >>>> + <&camcc CAMCC_IFE_1_CSID_CLK>, >>>> + <&camcc CAMCC_IFE_2_AXI_CLK>, >>>> + <&camcc CAMCC_IFE_2_CLK>, >>>> + <&camcc CAMCC_IFE_2_CPHY_RX_CLK>, >>>> + <&camcc CAMCC_IFE_2_CSID_CLK>, >>>> + <&camcc CAMCC_IFE_LITE_CLK>, >>>> + <&camcc CAMCC_IFE_LITE_CPHY_RX_CLK>, >>>> + <&camcc CAMCC_IFE_LITE_CSID_CLK>; >>>> + clock-names = "cam_ahb_clk", >>>> + "cam_axi", >>>> + "soc_ahb", >>>> + "camnoc_axi", >>>> + "core_ahb", >>>> + "cpas_ahb", >>>> + "csiphy0", >>>> + "csiphy0_timer", >>>> + "csiphy1", >>>> + "csiphy1_timer", >>>> + "csiphy2", >>>> + "csiphy2_timer", >>>> + "csiphy3", >>>> + "csiphy3_timer", >>>> + "slow_ahb_src", >>>> + "vfe0_axi", >>>> + "vfe0", >>>> + "vfe0_cphy_rx", >>>> + "vfe0_csid", >>>> + "vfe1_axi", >>>> + "vfe1", >>>> + "vfe1_cphy_rx", >>>> + "vfe1_csid", >>>> + "vfe2_axi", >>>> + "vfe2", >>>> + "vfe2_cphy_rx", >>>> + "vfe2_csid", >>>> + "vfe_lite", >>>> + "vfe_lite_cphy_rx", >>>> + "vfe_lite_csid"; >>>> + >>>> + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 717 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 718 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>; >>> >>> Interrupt types shall be IRQ_TYPE_EDGE_RISING. >> >> Ack >> >>> >>>> + interrupt-names = "csid0", >>>> + "csid1", >>>> + "csid2", >>>> + "csid_lite", >>>> + "csiphy0", >>>> + "csiphy1", >>>> + "csiphy2", >>>> + "csiphy3", >>>> + "vfe0", >>>> + "vfe1", >>>> + "vfe2", >>>> + "vfe_lite"; >>>> + >>>> + interconnects = <&gem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY >>>> + &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>, >>>> + <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS >>>> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, >>>> + <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS >>>> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, >>>> + <&mmss_noc MASTER_CAMNOC_ICP QCOM_ICC_TAG_ALWAYS >>>> + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>; >>>> + interconnect-names = "ahb", >>>> + "hf_mnoc", >>>> + "sf_mnoc", >>>> + "sf_icp_mnoc"; >>>> + >>>> + iommus = <&apps_smmu 0x820 0xc0>, >>>> + <&apps_smmu 0x840 0x0>, >>>> + <&apps_smmu 0x860 0xc0>, >>>> + <&apps_smmu 0x880 0x0>; >>>> + >>>> + power-domains = <&camcc TITAN_TOP_GDSC> >>> >>> It should be the last one in the list, if the settled practice is followed. >> >> See above. >> >>> >>>> + <&camcc IFE_0_GDSC>, >>>> + <&camcc IFE_1_GDSC>, >>>> + <&camcc IFE_2_GDSC>; >>>> + power-domain-names = "top", >>>> + "ife0", >>>> + "ife1", >>>> + "ife2"; >>>> + >>>> + vdd-csiphy-0p9-supply = <&vreg_l18a>; >>>> + vdd-csiphy-1p25-supply = <&vreg_l22a>; >>>> + >>>> + ports { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + port@0 { >>>> + reg = <0>; >>>> + csiphy0_ep: endpoint { >>> >>> An empty line before a child node is always needed. >> >> Ack >> >>> >>>> + data-lanes = <0 1 2 3>; >>>> + bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>; >>>> + remote-endpoint = <&sensor_ep>; >>>> + }; >>>> + }; >>>> + }; >>>> + }; >>>> + }; >>>> >> > -- Best wishes, Vladimir ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss 2025-11-14 17:06 ` Vladimir Zapolskiy @ 2025-11-16 14:05 ` Bryan O'Donoghue 0 siblings, 0 replies; 20+ messages in thread From: Bryan O'Donoghue @ 2025-11-16 14:05 UTC (permalink / raw) To: Vladimir Zapolskiy, Bryan O'Donoghue, Luca Weiss, Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 14/11/2025 17:06, Vladimir Zapolskiy wrote: > On 11/14/25 18:09, Bryan O'Donoghue wrote: >> On 14/11/2025 13:06, Luca Weiss wrote: >>> Hi Vladimir, >>> >>> On Fri Nov 14, 2025 at 1:40 PM CET, Vladimir Zapolskiy wrote: >>>> Hi Luca. >>>> >>>> On 11/14/25 13:15, Luca Weiss wrote: >>>>> Add bindings for the Camera Subsystem on the SM6350 SoC. >>>>> >>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>>> --- >>>>> .../bindings/media/qcom,sm6350-camss.yaml | 349 ++++++ >>>>> +++++++++++++++ >>>>> 1 file changed, 349 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350- >>>>> camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350- >>>>> camss.yaml >>>>> new file mode 100644 >>>>> index 000000000000..d812b5b50c05 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml >>>>> @@ -0,0 +1,349 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: http://devicetree.org/schemas/media/qcom,sm6350-camss.yaml# >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>> + >>>>> +title: Qualcomm SM6350 Camera Subsystem (CAMSS) >>>>> + >>>>> +maintainers: >>>>> + - Luca Weiss <luca.weiss@fairphone.com> >>>>> + >>>>> +description: >>>>> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm >>>>> platforms. >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + const: qcom,sm6350-camss >>>>> + >>>>> + reg: >>>>> + maxItems: 12 >>>>> + >>>>> + reg-names: >>>>> + items: >>>>> + - const: csid0 >>>>> + - const: csid1 >>>>> + - const: csid2 >>>>> + - const: csid_lite >>>>> + - const: csiphy0 >>>>> + - const: csiphy1 >>>>> + - const: csiphy2 >>>>> + - const: csiphy3 >>>>> + - const: vfe0 >>>>> + - const: vfe1 >>>>> + - const: vfe2 >>>>> + - const: vfe_lite >>>>> + >>>>> + clocks: >>>>> + maxItems: 30 >>>>> + >>>>> + clock-names: >>>>> + items: >>>>> + - const: cam_ahb_clk >>>>> + - const: cam_axi >>>>> + - const: soc_ahb >>>>> + - const: camnoc_axi >>>>> + - const: core_ahb >>>>> + - const: cpas_ahb >>>>> + - const: csiphy0 >>>>> + - const: csiphy0_timer >>>>> + - const: csiphy1 >>>>> + - const: csiphy1_timer >>>>> + - const: csiphy2 >>>>> + - const: csiphy2_timer >>>>> + - const: csiphy3 >>>>> + - const: csiphy3_timer >>>>> + - const: slow_ahb_src >>>>> + - const: vfe0_axi >>>>> + - const: vfe0 >>>>> + - const: vfe0_cphy_rx >>>>> + - const: vfe0_csid >>>>> + - const: vfe1_axi >>>>> + - const: vfe1 >>>>> + - const: vfe1_cphy_rx >>>>> + - const: vfe1_csid >>>>> + - const: vfe2_axi >>>>> + - const: vfe2 >>>>> + - const: vfe2_cphy_rx >>>>> + - const: vfe2_csid >>>>> + - const: vfe_lite >>>>> + - const: vfe_lite_cphy_rx >>>>> + - const: vfe_lite_csid >>>> >>>> The sorting order of this list does not follow the sorting order >>>> accepted >>>> in the past. >>> >>> What file should I best reference? >> >> Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml >> >>>> >>>> I'm very sorry for the vagueness, but I can not pronounce the accepted >>>> sorting order name, because it triggers people. >>>> >>>>> + >>>>> + interrupts: >>>>> + maxItems: 12 >>>>> + >>>>> + interrupt-names: >>>>> + items: >>>>> + - const: csid0 >>>>> + - const: csid1 >>>>> + - const: csid2 >>>>> + - const: csid_lite >>>>> + - const: csiphy0 >>>>> + - const: csiphy1 >>>>> + - const: csiphy2 >>>>> + - const: csiphy3 >>>>> + - const: vfe0 >>>>> + - const: vfe1 >>>>> + - const: vfe2 >>>>> + - const: vfe_lite >>>>> + >>>>> + interconnects: >>>>> + maxItems: 4 >>>>> + >>>>> + interconnect-names: >>>>> + items: >>>>> + - const: ahb >>>>> + - const: hf_mnoc >>>>> + - const: sf_mnoc >>>>> + - const: sf_icp_mnoc >>>> >>>> Please remove sf_mnoc and sf_icp_mnoc, they are not needed for enabling >>>> IP to produce raw images, and one day you may use them somewhere else. >>> >>> Ack, will give it a try. >> >> Disagree with this. >> >> See the Kanaapali patches. I'm asking new submissions to be as complete >> as possible, instead of limiting the hardware description to the RDI. >> >> So listing the ICP noc is the right thing to do. >> >> So please include register banks for >> >> - bps >> - cdm >> - icp >> - ipe >> - jpeg >> - lrme > > This suggests to get a DT backward compatibility lock forever, I'm not entirely sure what this comment means. The objective here is to get a full and complete DT describing camera hardware that can be consumed by - CAMSS RDI - CAMSS RDI + future goodness - FreeBSD - Any other DT consumer of upstream data perhaps even CamX which > makes it > a very bad advice, which is favourable for just the single interested > party, > which offers an alternative to the upstream CAMSS. > > Anybody who wants to get support of any CAMSS ISP functionality above RAW > images shall not add anything unrelated and unsupported. > > The asked inclusion shall be done later on safely, if ever needed. As I say the objective is to fully describe the hardware in DT, not to describe only the RDI path. >>>> I believe this clock is critical, and it is set so in the SM6350 GCC >>>> driver, >>>> therefore it should not be added here. >>> >>> True, gcc_camera_ahb_clk has CLK_IS_CRITICAL in gcc-sm6350.c >> >> DT describes hardware, not the happenstance of Linux driver setup. >> >> On that basis omitting <&gcc GCC_CAMERA_AHB_CLK> from the clock list is >> not correct. > > This has been already discussed, an enumerous amount of Qualcomm/MSM > specific resourced like clocks enabled in ABL, Linux etc. are considered > critical and not described in the dtb. I still think the argument for that is tenuous wrong in fact but, I know you are right, this is a lost argument. @Luca please _do_ include the full array of registers/clocks/nocs as you find them and yeah drop the AXI clock from that description because reasons. https://lore.kernel.org/linux-arm-msm/20251113-add-support-for-camss-on-kaanapali-v6-1-1e6038785a8e@oss.qualcomm.com/ --- bod ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss 2025-11-14 11:15 ` [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss Luca Weiss 2025-11-14 12:40 ` Vladimir Zapolskiy @ 2025-11-14 12:56 ` Rob Herring (Arm) 1 sibling, 0 replies; 20+ messages in thread From: Rob Herring (Arm) @ 2025-11-14 12:56 UTC (permalink / raw) To: Luca Weiss Cc: Konrad Dybcio, linux-media, linux-arm-msm, ~postmarketos/upstreaming, linux-kernel, Robert Foss, Krzysztof Kozlowski, Bryan O'Donoghue, phone-devel, Bryan O'Donoghue, Mauro Carvalho Chehab, Vladimir Zapolskiy, Conor Dooley, devicetree, Todor Tomov, Bjorn Andersson On Fri, 14 Nov 2025 12:15:24 +0100, Luca Weiss wrote: > Add bindings for the Camera Subsystem on the SM6350 SoC. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > .../bindings/media/qcom,sm6350-camss.yaml | 349 +++++++++++++++++++++ > 1 file changed, 349 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Error: Documentation/devicetree/bindings/media/qcom,sm6350-camss.example.dts:169.33-34 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [scripts/Makefile.dtbs:132: Documentation/devicetree/bindings/media/qcom,sm6350-camss.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1547: dt_binding_check] Error 2 make: *** [Makefile:248: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20251114-sm6350-camss-v2-1-d1ff67da33b6@fairphone.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] media: qcom: camss: Add SM6350 support 2025-11-14 11:15 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Luca Weiss 2025-11-14 11:15 ` [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss Luca Weiss @ 2025-11-14 11:15 ` Luca Weiss 2025-11-14 22:09 ` Konrad Dybcio 2025-11-14 11:15 ` [PATCH v2 3/3] arm64: dts: qcom: sm6350: Add CAMSS node Luca Weiss 2025-11-14 15:51 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Bryan O'Donoghue 3 siblings, 1 reply; 20+ messages in thread From: Luca Weiss @ 2025-11-14 11:15 UTC (permalink / raw) To: Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel, Luca Weiss Add the necessary support for CAMSS on the SM6350 SoC. Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- .../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 | 249 +++++++++++++++++++++ drivers/media/platform/qcom/camss/camss.h | 1 + 4 files changed, 377 insertions(+) diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c index a229ba04b158..3e44b0c8298d 100644 --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c @@ -398,6 +398,126 @@ csiphy_lane_regs lane_regs_sm8250[] = { {0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, }; +/* GEN2 1.2.3 2PH */ +static const struct +csiphy_lane_regs lane_regs_sm6350[] = { + {0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0904, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0910, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0900, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0908, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0904, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x002C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0034, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0010, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x001C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0014, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0028, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x003C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0000, 0x91, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0020, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0008, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0010, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0038, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x005C, 0xC0, 0x00, CSIPHY_SKEW_CAL}, + {0x0060, 0x0D, 0x00, CSIPHY_SKEW_CAL}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0000, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0730, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C84, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C90, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C80, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C88, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C84, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x072C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0734, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0710, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x071C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0714, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0728, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x073C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0700, 0x80, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0704, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0720, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0708, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x070c, 0xFF, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0710, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0738, 0x1F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0000, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0000, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0230, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A10, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A00, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A08, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A04, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x022C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0234, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0210, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x021C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0214, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0228, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x023C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0200, 0x91, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0204, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0220, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0208, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0210, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0238, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x025C, 0xC0, 0x00, CSIPHY_SKEW_CAL}, + {0x0260, 0x0D, 0x00, CSIPHY_SKEW_CAL}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0000, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B10, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B00, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B08, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B04, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x042C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0434, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0410, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x041C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0414, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0428, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x043C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0400, 0x91, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0404, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0420, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0408, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0410, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0438, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x045C, 0xC0, 0x00, CSIPHY_SKEW_CAL}, + {0x0460, 0x0D, 0x00, CSIPHY_SKEW_CAL}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0000, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0630, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C10, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C00, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C08, 0x06, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C04, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x062C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0634, 0x0F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0610, 0x50, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x061C, 0x0A, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0614, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0628, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x063C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0600, 0x91, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0604, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0620, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0608, 0x04, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0610, 0x52, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0638, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x065C, 0xC0, 0x00, CSIPHY_SKEW_CAL}, + {0x0660, 0x0D, 0x00, CSIPHY_SKEW_CAL}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0000, 0x00, 0x00, CSIPHY_DNP_PARAMS}, +}; + /* 14nm 2PH v 2.0.1 2p5Gbps 4 lane DPHY mode */ static const struct csiphy_lane_regs lane_regs_qcm2290[] = { @@ -908,6 +1028,7 @@ static bool csiphy_is_gen2(u32 version) switch (version) { case CAMSS_2290: + case CAMSS_6350: case CAMSS_7280: case CAMSS_8250: case CAMSS_8280XP: @@ -999,6 +1120,10 @@ static int csiphy_init(struct csiphy_device *csiphy) regs->lane_regs = &lane_regs_qcm2290[0]; regs->lane_array_size = ARRAY_SIZE(lane_regs_qcm2290); break; + case CAMSS_6350: + regs->lane_regs = &lane_regs_sm6350[0]; + regs->lane_array_size = ARRAY_SIZE(lane_regs_sm6350); + break; case CAMSS_7280: case CAMSS_8250: regs->lane_regs = &lane_regs_sm8250[0]; diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c index dff8d0a1e8c2..336838b1340b 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe.c +++ b/drivers/media/platform/qcom/camss/camss-vfe.c @@ -339,6 +339,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code, return sink_code; } break; + case CAMSS_6350: case CAMSS_660: case CAMSS_2290: case CAMSS_7280: @@ -1989,6 +1990,7 @@ static int vfe_bpl_align(struct vfe_device *vfe) int ret = 8; switch (vfe->camss->res->version) { + case CAMSS_6350: case CAMSS_7280: case CAMSS_8250: case CAMSS_8280XP: diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c index 2fbcd0e343aa..270f5a1341c6 100644 --- a/drivers/media/platform/qcom/camss/camss.c +++ b/drivers/media/platform/qcom/camss/camss.c @@ -1318,6 +1318,241 @@ static const struct camss_subdev_resources vfe_res_845[] = { } }; +static const struct camss_subdev_resources csiphy_res_6350[] = { + /* CSIPHY0 */ + { + .regulators = { "vdd-csiphy-0p9", "vdd-csiphy-1p25" }, + .clock = { "csiphy0", "csiphy0_timer" }, + .clock_rate = { { 300000000, 384000000, 400000000 }, + { 300000000 } }, + .reg = { "csiphy0" }, + .interrupt = { "csiphy0" }, + .csiphy = { + .id = 0, + .hw_ops = &csiphy_ops_3ph_1_0, + .formats = &csiphy_formats_sdm845 + } + }, + /* CSIPHY1 */ + { + .regulators = { "vdd-csiphy-0p9", "vdd-csiphy-1p25" }, + .clock = { "csiphy1", "csiphy1_timer" }, + .clock_rate = { { 300000000, 384000000, 400000000 }, + { 300000000 } }, + .reg = { "csiphy1" }, + .interrupt = { "csiphy1" }, + .csiphy = { + .id = 1, + .hw_ops = &csiphy_ops_3ph_1_0, + .formats = &csiphy_formats_sdm845 + } + }, + /* CSIPHY2 */ + { + .regulators = { "vdd-csiphy-0p9", "vdd-csiphy-1p25" }, + .clock = { "csiphy2", "csiphy2_timer" }, + .clock_rate = { { 300000000, 384000000, 400000000 }, + { 300000000 } }, + .reg = { "csiphy2" }, + .interrupt = { "csiphy2" }, + .csiphy = { + .id = 2, + .hw_ops = &csiphy_ops_3ph_1_0, + .formats = &csiphy_formats_sdm845 + } + }, + /* CSIPHY3 */ + { + .regulators = { "vdd-csiphy-0p9", "vdd-csiphy-1p25" }, + .clock = { "csiphy3", "csiphy3_timer" }, + .clock_rate = { { 300000000, 384000000, 400000000 }, + { 300000000 } }, + .reg = { "csiphy3" }, + .interrupt = { "csiphy3" }, + .csiphy = { + .id = 3, + .hw_ops = &csiphy_ops_3ph_1_0, + .formats = &csiphy_formats_sdm845 + } + } +}; + +static const struct camss_subdev_resources csid_res_6350[] = { + /* CSID0 */ + { + .regulators = {}, + .clock = { "vfe0_csid", "vfe0_cphy_rx", "vfe0" }, + .clock_rate = { { 300000000, 384000000, 400000000 }, + { 0 }, + { 320000000, 404000000, 480000000, 600000000 } }, + .reg = { "csid0" }, + .interrupt = { "csid0" }, + .csid = { + .hw_ops = &csid_ops_gen2, + .parent_dev_ops = &vfe_parent_dev_ops, + .formats = &csid_formats_gen2 + } + }, + /* CSID1 */ + { + .regulators = {}, + .clock = { "vfe1_csid", "vfe1_cphy_rx", "vfe1" }, + .clock_rate = { { 300000000, 384000000, 400000000 }, + { 0 }, + { 320000000, 404000000, 480000000, 600000000 } }, + .reg = { "csid1" }, + .interrupt = { "csid1" }, + .csid = { + .hw_ops = &csid_ops_gen2, + .parent_dev_ops = &vfe_parent_dev_ops, + .formats = &csid_formats_gen2 + } + }, + /* CSID2 */ + { + .regulators = {}, + .clock = { "vfe2_csid", "vfe2_cphy_rx", "vfe2" }, + .clock_rate = { { 300000000, 384000000, 400000000 }, + { 0 }, + { 320000000, 404000000, 480000000, 600000000 } }, + .reg = { "csid2" }, + .interrupt = { "csid2" }, + .csid = { + .hw_ops = &csid_ops_gen2, + .parent_dev_ops = &vfe_parent_dev_ops, + .formats = &csid_formats_gen2 + } + }, + /* CSID3 (lite) */ + { + .regulators = {}, + .clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite" }, + .clock_rate = { { 300000000, 384000000, 400000000 }, + { 0 }, + { 400000000, 480000000 } }, + .reg = { "csid_lite" }, + .interrupt = { "csid_lite" }, + .csid = { + .is_lite = true, + .hw_ops = &csid_ops_gen2, + .parent_dev_ops = &vfe_parent_dev_ops, + .formats = &csid_formats_gen2 + } + } +}; + +static const struct camss_subdev_resources vfe_res_6350[] = { + /* VFE0 */ + { + .regulators = {}, + .clock = { "slow_ahb_src", "cpas_ahb", + "camnoc_axi", "vfe0", "vfe0_axi", "cam_axi", "soc_ahb" }, + .clock_rate = { { 19200000, 80000000 }, + { 19200000 }, + { 0 }, + { 320000000, 404000000, 480000000, 600000000 }, + { 0 }, + { 0 } }, + .reg = { "vfe0" }, + .interrupt = { "vfe0" }, + .vfe = { + .line_num = 3, + .has_pd = true, + .pd_name = "ife0", + .hw_ops = &vfe_ops_170, + .formats_rdi = &vfe_formats_rdi_845, + .formats_pix = &vfe_formats_pix_845 + } + }, + /* VFE1 */ + { + .regulators = {}, + .clock = { "slow_ahb_src", "cpas_ahb", + "camnoc_axi", "vfe1", "vfe1_axi", "cam_axi", "soc_ahb" }, + .clock_rate = { { 19200000, 80000000 }, + { 19200000 }, + { 0 }, + { 320000000, 404000000, 480000000, 600000000 }, + { 0 }, + { 0 } }, + .reg = { "vfe1" }, + .interrupt = { "vfe1" }, + .vfe = { + .line_num = 3, + .has_pd = true, + .pd_name = "ife1", + .hw_ops = &vfe_ops_170, + .formats_rdi = &vfe_formats_rdi_845, + .formats_pix = &vfe_formats_pix_845 + } + }, + /* VFE2 */ + { + .regulators = {}, + .clock = { "slow_ahb_src", "cpas_ahb", + "camnoc_axi", "vfe2", "vfe2_axi", "cam_axi", "soc_ahb" }, + .clock_rate = { { 19200000, 80000000 }, + { 19200000 }, + { 0 }, + { 320000000, 404000000, 480000000, 600000000 }, + { 0 }, + { 0 } }, + .reg = { "vfe2" }, + .interrupt = { "vfe2" }, + .vfe = { + .line_num = 3, + .has_pd = true, + .pd_name = "ife2", + .hw_ops = &vfe_ops_170, + .formats_rdi = &vfe_formats_rdi_845, + .formats_pix = &vfe_formats_pix_845 + } + }, + /* VFE3 (lite) */ + { + .regulators = {}, + .clock = { "slow_ahb_src", "cpas_ahb", + "camnoc_axi", "vfe_lite", "cam_axi", "soc_ahb" }, + .clock_rate = { { 19200000, 80000000 }, + { 19200000 }, + { 0 }, + { 400000000, 480000000 }, + { 0 } }, + .reg = { "vfe_lite" }, + .interrupt = { "vfe_lite" }, + .vfe = { + .is_lite = true, + .line_num = 4, + .hw_ops = &vfe_ops_170, + .formats_rdi = &vfe_formats_rdi_845, + .formats_pix = &vfe_formats_pix_845 + } + }, +}; + +static const struct resources_icc icc_res_sm6350[] = { + { + .name = "ahb", + .icc_bw_tbl.avg = 0, + .icc_bw_tbl.peak = 300000, + }, + { + .name = "hf_mnoc", + .icc_bw_tbl.avg = 2097152, + .icc_bw_tbl.peak = 2097152, + }, + { + .name = "sf_mnoc", + .icc_bw_tbl.avg = 2097152, + .icc_bw_tbl.peak = 2097152, + }, + { + .name = "sf_icp_mnoc", + .icc_bw_tbl.avg = 2097152, + .icc_bw_tbl.peak = 2097152, + }, +}; + static const struct camss_subdev_resources csiphy_res_8250[] = { /* CSIPHY0 */ { @@ -4398,6 +4633,19 @@ static const struct camss_resources sdm845_resources = { .vfe_num = ARRAY_SIZE(vfe_res_845), }; +static const struct camss_resources sm6350_resources = { + .version = CAMSS_6350, + .pd_name = "top", + .csiphy_res = csiphy_res_6350, + .csid_res = csid_res_6350, + .vfe_res = vfe_res_6350, + .icc_res = icc_res_sm6350, + .icc_path_num = ARRAY_SIZE(icc_res_sm6350), + .csiphy_num = ARRAY_SIZE(csiphy_res_6350), + .csid_num = ARRAY_SIZE(csid_res_6350), + .vfe_num = ARRAY_SIZE(vfe_res_6350), +}; + static const struct camss_resources sm8250_resources = { .version = CAMSS_8250, .pd_name = "top", @@ -4478,6 +4726,7 @@ static const struct of_device_id camss_dt_match[] = { { .compatible = "qcom,sdm660-camss", .data = &sdm660_resources }, { .compatible = "qcom,sdm670-camss", .data = &sdm670_resources }, { .compatible = "qcom,sdm845-camss", .data = &sdm845_resources }, + { .compatible = "qcom,sm6350-camss", .data = &sm6350_resources }, { .compatible = "qcom,sm8250-camss", .data = &sm8250_resources }, { .compatible = "qcom,sm8550-camss", .data = &sm8550_resources }, { .compatible = "qcom,x1e80100-camss", .data = &x1e80100_resources }, diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h index a70fbc78ccc3..ae456ff4e612 100644 --- a/drivers/media/platform/qcom/camss/camss.h +++ b/drivers/media/platform/qcom/camss/camss.h @@ -77,6 +77,7 @@ enum pm_domain { }; enum camss_version { + CAMSS_6350, CAMSS_660, CAMSS_2290, CAMSS_7280, -- 2.51.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] media: qcom: camss: Add SM6350 support 2025-11-14 11:15 ` [PATCH v2 2/3] media: qcom: camss: Add SM6350 support Luca Weiss @ 2025-11-14 22:09 ` Konrad Dybcio 0 siblings, 0 replies; 20+ messages in thread From: Konrad Dybcio @ 2025-11-14 22:09 UTC (permalink / raw) To: Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 11/14/25 12:15 PM, Luca Weiss wrote: > Add the necessary support for CAMSS on the SM6350 SoC. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- [...] > + .regulators = {}, > + .clock = { "slow_ahb_src", "cpas_ahb", Drop the _src clock, we do ratesetting on it through its dear child who you have described right next to it Konrad ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] arm64: dts: qcom: sm6350: Add CAMSS node 2025-11-14 11:15 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Luca Weiss 2025-11-14 11:15 ` [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss Luca Weiss 2025-11-14 11:15 ` [PATCH v2 2/3] media: qcom: camss: Add SM6350 support Luca Weiss @ 2025-11-14 11:15 ` Luca Weiss 2025-11-14 15:51 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Bryan O'Donoghue 3 siblings, 0 replies; 20+ messages in thread From: Luca Weiss @ 2025-11-14 11:15 UTC (permalink / raw) To: Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel, Luca Weiss Add a node for the CAMSS on the SM6350 SoC. Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- arch/arm64/boot/dts/qcom/sm6350.dtsi | 165 +++++++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi index ca6f65e8e267..2784b4541771 100644 --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi @@ -2123,6 +2123,171 @@ cci1_i2c0: i2c-bus@0 { /* SM6350 seems to have cci1_i2c1 on gpio2 & gpio3 but unused downstream */ }; + camss: isp@acb3000 { + compatible = "qcom,sm6350-camss"; + + reg = <0x0 0x0acb3000 0x0 0x1000>, + <0x0 0x0acba000 0x0 0x1000>, + <0x0 0x0acc1000 0x0 0x1000>, + <0x0 0x0acc8000 0x0 0x1000>, + <0x0 0x0ac65000 0x0 0x1000>, + <0x0 0x0ac66000 0x0 0x1000>, + <0x0 0x0ac67000 0x0 0x1000>, + <0x0 0x0ac68000 0x0 0x1000>, + <0x0 0x0acaf000 0x0 0x4000>, + <0x0 0x0acb6000 0x0 0x4000>, + <0x0 0x0acbd000 0x0 0x4000>, + <0x0 0x0acc4000 0x0 0x4000>; + reg-names = "csid0", + "csid1", + "csid2", + "csid_lite", + "csiphy0", + "csiphy1", + "csiphy2", + "csiphy3", + "vfe0", + "vfe1", + "vfe2", + "vfe_lite"; + + clocks = <&gcc GCC_CAMERA_AHB_CLK>, + <&gcc GCC_CAMERA_AXI_CLK>, + <&camcc CAMCC_SOC_AHB_CLK>, + <&camcc CAMCC_CAMNOC_AXI_CLK>, + <&camcc CAMCC_CORE_AHB_CLK>, + <&camcc CAMCC_CPAS_AHB_CLK>, + <&camcc CAMCC_CSIPHY0_CLK>, + <&camcc CAMCC_CSI0PHYTIMER_CLK>, + <&camcc CAMCC_CSIPHY1_CLK>, + <&camcc CAMCC_CSI1PHYTIMER_CLK>, + <&camcc CAMCC_CSIPHY2_CLK>, + <&camcc CAMCC_CSI2PHYTIMER_CLK>, + <&camcc CAMCC_CSIPHY3_CLK>, + <&camcc CAMCC_CSI3PHYTIMER_CLK>, + <&camcc CAMCC_SLOW_AHB_CLK_SRC>, + <&camcc CAMCC_IFE_0_AXI_CLK>, + <&camcc CAMCC_IFE_0_CLK>, + <&camcc CAMCC_IFE_0_CPHY_RX_CLK>, + <&camcc CAMCC_IFE_0_CSID_CLK>, + <&camcc CAMCC_IFE_1_AXI_CLK>, + <&camcc CAMCC_IFE_1_CLK>, + <&camcc CAMCC_IFE_1_CPHY_RX_CLK>, + <&camcc CAMCC_IFE_1_CSID_CLK>, + <&camcc CAMCC_IFE_2_AXI_CLK>, + <&camcc CAMCC_IFE_2_CLK>, + <&camcc CAMCC_IFE_2_CPHY_RX_CLK>, + <&camcc CAMCC_IFE_2_CSID_CLK>, + <&camcc CAMCC_IFE_LITE_CLK>, + <&camcc CAMCC_IFE_LITE_CPHY_RX_CLK>, + <&camcc CAMCC_IFE_LITE_CSID_CLK>; + clock-names = "cam_ahb_clk", + "cam_axi", + "soc_ahb", + "camnoc_axi", + "core_ahb", + "cpas_ahb", + "csiphy0", + "csiphy0_timer", + "csiphy1", + "csiphy1_timer", + "csiphy2", + "csiphy2_timer", + "csiphy3", + "csiphy3_timer", + "slow_ahb_src", + "vfe0_axi", + "vfe0", + "vfe0_cphy_rx", + "vfe0_csid", + "vfe1_axi", + "vfe1", + "vfe1_cphy_rx", + "vfe1_csid", + "vfe2_axi", + "vfe2", + "vfe2_cphy_rx", + "vfe2_csid", + "vfe_lite", + "vfe_lite_cphy_rx", + "vfe_lite_csid"; + + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 717 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 718 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "csid0", + "csid1", + "csid2", + "csid_lite", + "csiphy0", + "csiphy1", + "csiphy2", + "csiphy3", + "vfe0", + "vfe1", + "vfe2", + "vfe_lite"; + + interconnects = <&gem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY + &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>, + <&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, + <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>, + <&mmss_noc MASTER_CAMNOC_ICP QCOM_ICC_TAG_ALWAYS + &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>; + interconnect-names = "ahb", + "hf_mnoc", + "sf_mnoc", + "sf_icp_mnoc"; + + iommus = <&apps_smmu 0x820 0xc0>, + <&apps_smmu 0x840 0x0>, + <&apps_smmu 0x860 0xc0>, + <&apps_smmu 0x880 0x0>; + + power-domains = <&camcc TITAN_TOP_GDSC>, + <&camcc IFE_0_GDSC>, + <&camcc IFE_1_GDSC>, + <&camcc IFE_2_GDSC>; + power-domain-names = "top", + "ife0", + "ife1", + "ife2"; + + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + }; + + port@1 { + reg = <1>; + }; + + port@2 { + reg = <2>; + }; + + port@3 { + reg = <3>; + }; + }; + }; + camcc: clock-controller@ad00000 { compatible = "qcom,sm6350-camcc"; reg = <0x0 0x0ad00000 0x0 0x16000>; -- 2.51.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-14 11:15 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Luca Weiss ` (2 preceding siblings ...) 2025-11-14 11:15 ` [PATCH v2 3/3] arm64: dts: qcom: sm6350: Add CAMSS node Luca Weiss @ 2025-11-14 15:51 ` Bryan O'Donoghue 2025-11-14 15:59 ` Luca Weiss 3 siblings, 1 reply; 20+ messages in thread From: Bryan O'Donoghue @ 2025-11-14 15:51 UTC (permalink / raw) To: Luca Weiss, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 14/11/2025 11:15, 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. > > [ 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 Do you have a full and complete kernel tree we could look at here ? --- bod ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-14 15:51 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Bryan O'Donoghue @ 2025-11-14 15:59 ` Luca Weiss 2025-11-16 14:30 ` Bryan O'Donoghue 0 siblings, 1 reply; 20+ messages in thread From: Luca Weiss @ 2025-11-14 15:59 UTC (permalink / raw) To: Bryan O'Donoghue, Luca Weiss, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bryan O'Donoghue, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On Fri Nov 14, 2025 at 4:51 PM CET, Bryan O'Donoghue wrote: > On 14/11/2025 11:15, 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. >> >> [ 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 > > Do you have a full and complete kernel tree we could look at here ? Sure, this branch has everything in: https://github.com/sm6350-mainline/linux/tree/sm6350-6.17.y/ For further refence, at least two other people have tested this branch in postmarketOS, nothing particularly exciting to report from there, apart from that the sdm-skin-thermal thermal zone (thermistor right next to SoC) is currently configured with 55 degC as critical trip, which is quickly achieved when starting a video recording, but that's not really an issue with camss, but will need some tweaking regardless. https://gitlab.postmarketos.org/postmarketOS/pmaports/-/merge_requests/7281 Regards Luca > > --- > bod ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-14 15:59 ` Luca Weiss @ 2025-11-16 14:30 ` Bryan O'Donoghue 2025-11-17 12:53 ` Konrad Dybcio 0 siblings, 1 reply; 20+ messages in thread From: Bryan O'Donoghue @ 2025-11-16 14:30 UTC (permalink / raw) To: Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 14/11/2025 15:59, Luca Weiss wrote: > On Fri Nov 14, 2025 at 4:51 PM CET, Bryan O'Donoghue wrote: >> On 14/11/2025 11:15, 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. >>> >>> [ 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 >> >> Do you have a full and complete kernel tree we could look at here ? > > Sure, this branch has everything in: > > https://github.com/sm6350-mainline/linux/tree/sm6350-6.17.y/ > > For further refence, at least two other people have tested this branch > in postmarketOS, nothing particularly exciting to report from there, > apart from that the sdm-skin-thermal thermal zone (thermistor right next > to SoC) is currently configured with 55 degC as critical trip, which is > quickly achieved when starting a video recording, but that's not really > an issue with camss, but will need some tweaking regardless. > > https://gitlab.postmarketos.org/postmarketOS/pmaports/-/merge_requests/7281 diff --git a/drivers/clk/qcom/gcc-sm6350.c b/drivers/clk/qcom/gcc-sm6350.c index a4d6dff9d0f7f..229629ef82809 100644 --- a/drivers/clk/qcom/gcc-sm6350.c +++ b/drivers/clk/qcom/gcc-sm6350.c @@ -908,9 +908,7 @@ 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, + .halt_check = BRANCH_VOTED, .clkr = { .enable_reg = 0x17018, .enable_mask = BIT(0), ? --- bod ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-16 14:30 ` Bryan O'Donoghue @ 2025-11-17 12:53 ` Konrad Dybcio 2025-11-18 9:33 ` Bryan O'Donoghue 0 siblings, 1 reply; 20+ messages in thread From: Konrad Dybcio @ 2025-11-17 12:53 UTC (permalink / raw) To: Bryan O'Donoghue, Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 11/16/25 3:30 PM, Bryan O'Donoghue wrote: > On 14/11/2025 15:59, Luca Weiss wrote: >> On Fri Nov 14, 2025 at 4:51 PM CET, Bryan O'Donoghue wrote: >>> On 14/11/2025 11:15, 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. >>>> >>>> [ 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 >>> >>> Do you have a full and complete kernel tree we could look at here ? >> >> Sure, this branch has everything in: >> >> https://github.com/sm6350-mainline/linux/tree/sm6350-6.17.y/ >> >> For further refence, at least two other people have tested this branch >> in postmarketOS, nothing particularly exciting to report from there, >> apart from that the sdm-skin-thermal thermal zone (thermistor right next >> to SoC) is currently configured with 55 degC as critical trip, which is >> quickly achieved when starting a video recording, but that's not really >> an issue with camss, but will need some tweaking regardless. >> >> https://gitlab.postmarketos.org/postmarketOS/pmaports/-/merge_requests/7281 > > diff --git a/drivers/clk/qcom/gcc-sm6350.c b/drivers/clk/qcom/gcc-sm6350.c > index a4d6dff9d0f7f..229629ef82809 100644 > --- a/drivers/clk/qcom/gcc-sm6350.c > +++ b/drivers/clk/qcom/gcc-sm6350.c > @@ -908,9 +908,7 @@ 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, No reason to drop the hwcg description > + .halt_check = BRANCH_VOTED, It'd be useful to explain why we should ignore the hw feedback in this case > .clkr = { > .enable_reg = 0x17018, > .enable_mask = BIT(0), Konrad ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-17 12:53 ` Konrad Dybcio @ 2025-11-18 9:33 ` Bryan O'Donoghue 2025-11-18 9:43 ` Luca Weiss 2025-11-18 10:06 ` Konrad Dybcio 0 siblings, 2 replies; 20+ messages in thread From: Bryan O'Donoghue @ 2025-11-18 9:33 UTC (permalink / raw) To: Konrad Dybcio, Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 17/11/2025 12:53, Konrad Dybcio wrote: > On 11/16/25 3:30 PM, Bryan O'Donoghue wrote: >> On 14/11/2025 15:59, Luca Weiss wrote: >>> On Fri Nov 14, 2025 at 4:51 PM CET, Bryan O'Donoghue wrote: >>>> On 14/11/2025 11:15, 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. >>>>> >>>>> [ 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 >>>> >>>> Do you have a full and complete kernel tree we could look at here ? >>> >>> Sure, this branch has everything in: >>> >>> https://github.com/sm6350-mainline/linux/tree/sm6350-6.17.y/ >>> >>> For further refence, at least two other people have tested this branch >>> in postmarketOS, nothing particularly exciting to report from there, >>> apart from that the sdm-skin-thermal thermal zone (thermistor right next >>> to SoC) is currently configured with 55 degC as critical trip, which is >>> quickly achieved when starting a video recording, but that's not really >>> an issue with camss, but will need some tweaking regardless. >>> >>> https://gitlab.postmarketos.org/postmarketOS/pmaports/-/merge_requests/7281 >> >> diff --git a/drivers/clk/qcom/gcc-sm6350.c b/drivers/clk/qcom/gcc-sm6350.c >> index a4d6dff9d0f7f..229629ef82809 100644 >> --- a/drivers/clk/qcom/gcc-sm6350.c >> +++ b/drivers/clk/qcom/gcc-sm6350.c >> @@ -908,9 +908,7 @@ 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, > > No reason to drop the hwcg description > >> + .halt_check = BRANCH_VOTED, > > It'd be useful to explain why we should ignore the hw feedback in this case > >> .clkr = { >> .enable_reg = 0x17018, >> .enable_mask = BIT(0), > > Konrad vfe170 is what we have on sdm845 So I'm just asking Luca to try the sdm845 method of waggling this clock since what we have doesn't work. --- bod ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-18 9:33 ` Bryan O'Donoghue @ 2025-11-18 9:43 ` Luca Weiss 2025-11-18 10:06 ` Konrad Dybcio 1 sibling, 0 replies; 20+ messages in thread From: Luca Weiss @ 2025-11-18 9:43 UTC (permalink / raw) To: Bryan O'Donoghue, Konrad Dybcio, Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel Hi Bryan, On Tue Nov 18, 2025 at 10:33 AM CET, Bryan O'Donoghue wrote: > On 17/11/2025 12:53, Konrad Dybcio wrote: >> On 11/16/25 3:30 PM, Bryan O'Donoghue wrote: >>> On 14/11/2025 15:59, Luca Weiss wrote: >>>> On Fri Nov 14, 2025 at 4:51 PM CET, Bryan O'Donoghue wrote: >>>>> On 14/11/2025 11:15, 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. >>>>>> >>>>>> [ 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 >>>>> >>>>> Do you have a full and complete kernel tree we could look at here ? >>>> >>>> Sure, this branch has everything in: >>>> >>>> https://github.com/sm6350-mainline/linux/tree/sm6350-6.17.y/ >>>> >>>> For further refence, at least two other people have tested this branch >>>> in postmarketOS, nothing particularly exciting to report from there, >>>> apart from that the sdm-skin-thermal thermal zone (thermistor right next >>>> to SoC) is currently configured with 55 degC as critical trip, which is >>>> quickly achieved when starting a video recording, but that's not really >>>> an issue with camss, but will need some tweaking regardless. >>>> >>>> https://gitlab.postmarketos.org/postmarketOS/pmaports/-/merge_requests/7281 >>> >>> diff --git a/drivers/clk/qcom/gcc-sm6350.c b/drivers/clk/qcom/gcc-sm6350.c >>> index a4d6dff9d0f7f..229629ef82809 100644 >>> --- a/drivers/clk/qcom/gcc-sm6350.c >>> +++ b/drivers/clk/qcom/gcc-sm6350.c >>> @@ -908,9 +908,7 @@ 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, >> >> No reason to drop the hwcg description >> >>> + .halt_check = BRANCH_VOTED, >> >> It'd be useful to explain why we should ignore the hw feedback in this case >> >>> .clkr = { >>> .enable_reg = 0x17018, >>> .enable_mask = BIT(0), >> >> Konrad > > vfe170 is what we have on sdm845 > > So I'm just asking Luca to try the sdm845 method of waggling this clock > since what we have doesn't work. Yes, your change to gcc-sm6350.c does remove the warning and everything seems to work as expected. Opening and closing the plasma-camera application, switching between cameras works without any visible issues. Regards Luca > > --- > bod ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-18 9:33 ` Bryan O'Donoghue 2025-11-18 9:43 ` Luca Weiss @ 2025-11-18 10:06 ` Konrad Dybcio 2025-11-18 11:08 ` Bryan O'Donoghue 1 sibling, 1 reply; 20+ messages in thread From: Konrad Dybcio @ 2025-11-18 10:06 UTC (permalink / raw) To: Bryan O'Donoghue, Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 11/18/25 10:33 AM, Bryan O'Donoghue wrote: > On 17/11/2025 12:53, Konrad Dybcio wrote: >> On 11/16/25 3:30 PM, Bryan O'Donoghue wrote: >>> On 14/11/2025 15:59, Luca Weiss wrote: >>>> On Fri Nov 14, 2025 at 4:51 PM CET, Bryan O'Donoghue wrote: >>>>> On 14/11/2025 11:15, 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. >>>>>> >>>>>> [ 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 >>>>> >>>>> Do you have a full and complete kernel tree we could look at here ? >>>> >>>> Sure, this branch has everything in: >>>> >>>> https://github.com/sm6350-mainline/linux/tree/sm6350-6.17.y/ >>>> >>>> For further refence, at least two other people have tested this branch >>>> in postmarketOS, nothing particularly exciting to report from there, >>>> apart from that the sdm-skin-thermal thermal zone (thermistor right next >>>> to SoC) is currently configured with 55 degC as critical trip, which is >>>> quickly achieved when starting a video recording, but that's not really >>>> an issue with camss, but will need some tweaking regardless. >>>> >>>> https://gitlab.postmarketos.org/postmarketOS/pmaports/-/merge_requests/7281 >>> >>> diff --git a/drivers/clk/qcom/gcc-sm6350.c b/drivers/clk/qcom/gcc-sm6350.c >>> index a4d6dff9d0f7f..229629ef82809 100644 >>> --- a/drivers/clk/qcom/gcc-sm6350.c >>> +++ b/drivers/clk/qcom/gcc-sm6350.c >>> @@ -908,9 +908,7 @@ 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, >> >> No reason to drop the hwcg description >> >>> + .halt_check = BRANCH_VOTED, >> >> It'd be useful to explain why we should ignore the hw feedback in this case >> >>> .clkr = { >>> .enable_reg = 0x17018, >>> .enable_mask = BIT(0), >> >> Konrad > > vfe170 is what we have on sdm845 > > So I'm just asking Luca to try the sdm845 method of waggling this clock since what we have doesn't work. It's of course going to work because this way you're not calling the code that throws this error I was curious whether you know the actual reason why this is being done in some other GCC drivers Konrad ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-18 10:06 ` Konrad Dybcio @ 2025-11-18 11:08 ` Bryan O'Donoghue 2025-11-18 11:50 ` Konrad Dybcio 0 siblings, 1 reply; 20+ messages in thread From: Bryan O'Donoghue @ 2025-11-18 11:08 UTC (permalink / raw) To: Konrad Dybcio, Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 18/11/2025 10:06, Konrad Dybcio wrote: >>> Konrad >> vfe170 is what we have on sdm845 >> >> So I'm just asking Luca to try the sdm845 method of waggling this clock since what we have doesn't work. > It's of course going to work because this way you're not calling the > code that throws this error > > I was curious whether you know the actual reason why this is being > done in some other GCC drivers > > Konrad No notion at all, perhaps as a workaround to this very problem. --- bod ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] Add CAMSS support for SM6350 2025-11-18 11:08 ` Bryan O'Donoghue @ 2025-11-18 11:50 ` Konrad Dybcio 0 siblings, 0 replies; 20+ messages in thread From: Konrad Dybcio @ 2025-11-18 11:50 UTC (permalink / raw) To: Bryan O'Donoghue, Luca Weiss, Bryan O'Donoghue, Robert Foss, Todor Tomov, Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio, Taniya Das Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, linux-media, devicetree, linux-kernel On 11/18/25 12:08 PM, Bryan O'Donoghue wrote: > On 18/11/2025 10:06, Konrad Dybcio wrote: >>>> Konrad >>> vfe170 is what we have on sdm845 >>> >>> So I'm just asking Luca to try the sdm845 method of waggling this clock since what we have doesn't work. >> It's of course going to work because this way you're not calling the >> code that throws this error >> >> I was curious whether you know the actual reason why this is being >> done in some other GCC drivers >> >> Konrad > > No notion at all, perhaps as a workaround to this very problem. I tried digging it up, but only managed to find that there is a signal between titan and gcc to request it being enabled (perhaps that's a fancy description of hwcg) Maybe +Taniya would know? (context: sm6350 gcc_camera_axi_clk stuck at 'on' when disabling) Konrad ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-11-18 11:50 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <3ph8XeidoxkUIsK7qiOH29pde94sdwa3ReWKVVrPabgS5enIAmwVAC5plyFnBMJGKQBnxFB6df6j69OMFIeavw==@protonmail.internalid>
2025-11-14 11:15 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Luca Weiss
2025-11-14 11:15 ` [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss Luca Weiss
2025-11-14 12:40 ` Vladimir Zapolskiy
2025-11-14 13:06 ` Luca Weiss
2025-11-14 16:09 ` Bryan O'Donoghue
2025-11-14 17:06 ` Vladimir Zapolskiy
2025-11-16 14:05 ` Bryan O'Donoghue
2025-11-14 12:56 ` Rob Herring (Arm)
2025-11-14 11:15 ` [PATCH v2 2/3] media: qcom: camss: Add SM6350 support Luca Weiss
2025-11-14 22:09 ` Konrad Dybcio
2025-11-14 11:15 ` [PATCH v2 3/3] arm64: dts: qcom: sm6350: Add CAMSS node Luca Weiss
2025-11-14 15:51 ` [PATCH v2 0/3] Add CAMSS support for SM6350 Bryan O'Donoghue
2025-11-14 15:59 ` Luca Weiss
2025-11-16 14:30 ` Bryan O'Donoghue
2025-11-17 12:53 ` Konrad Dybcio
2025-11-18 9:33 ` Bryan O'Donoghue
2025-11-18 9:43 ` Luca Weiss
2025-11-18 10:06 ` Konrad Dybcio
2025-11-18 11:08 ` Bryan O'Donoghue
2025-11-18 11:50 ` Konrad Dybcio
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).