[PATCH 0/4] Add camss to SM8250 dtsi

[PATCH 0/4] Add camss to SM8250 dtsi

[Bryan O'Donoghue]

Add in necessary CAMSS, pin and CCI definitions to the SM8250 dtsi.

This is the SoC specific stuff, the platform enablement will be done in a
separate drop later.

On the RB5 platform we have validated ~ 30 FPS @ 4K using the SRGGB10P
pixel format provided by a Sony IMX577 sensor.

Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=br-v5.19b%2brb5-dts%2bsensors

Bryan O'Donoghue (4):
  arm64: dts: qcom: sm8250: Add camcc DT node
  arm64: dts: qcom: sm8250: camss: Add CAMSS block definition
  arm64: dts: qcom: sm8250: camss: Add downstream camera pin definitions
  arm64: dts: qcom: sm8250: camss: Add CCI definitions

 arch/arm64/boot/dts/qcom/sm8250.dtsi | 721 +++++++++++++++++++++++++++
 1 file changed, 721 insertions(+)

-- 
2.35.1

[PATCH 1/4] arm64: dts: qcom: sm8250: Add camcc DT node

[Bryan O'Donoghue]

Add the camcc DT node for the Camera Clock Controller on sm8250.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index af8f22636436..906bc8ed25b7 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -19,6 +19,7 @@
 #include <dt-bindings/sound/qcom,q6afe.h>
 #include <dt-bindings/thermal/thermal.h>
 #include <dt-bindings/clock/qcom,videocc-sm8250.h>
+#include <dt-bindings/clock/qcom,camcc-sm8250.h>
 
 / {
 	interrupt-parent = <&intc>;
@@ -3149,6 +3150,20 @@ videocc: clock-controller@abf0000 {
 			#power-domain-cells = <1>;
 		};
 
+		camcc: clock-controller@ad00000 {
+			compatible = "qcom,sm8250-camcc";
+			reg = <0 0x0ad00000 0 0x10000>;
+			clocks = <&gcc GCC_VIDEO_AHB_CLK>,
+				 <&rpmhcc RPMH_CXO_CLK>,
+				 <&rpmhcc RPMH_CXO_CLK_A>;
+			clock-names = "iface", "bi_tcxo", "bi_tcxo_ao";
+			power-domains = <&rpmhpd SM8250_MMCX>;
+			required-opps = <&rpmhpd_opp_low_svs>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			#power-domain-cells = <1>;
+		};
+
 		mdss: mdss@ae00000 {
 			compatible = "qcom,sm8250-mdss";
 			reg = <0 0x0ae00000 0 0x1000>;
-- 
2.35.1

[PATCH 2/4] arm64: dts: qcom: sm8250: camss: Add CAMSS block definition

[Bryan O'Donoghue]

Adds a CAMSS definition block.

Co-developed-by: Julian Grahsl <jgrahsl@snap.com>
Signed-off-by: Julian Grahsl <jgrahsl@snap.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 153 +++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 906bc8ed25b7..3d6c617de045 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3150,6 +3150,159 @@ videocc: clock-controller@abf0000 {
 			#power-domain-cells = <1>;
 		};
 
+		camss: camss@ac6a000 {
+			compatible = "qcom,sm8250-camss";
+			status = "disabled";
+
+			reg = <0 0xac6a000 0 0x2000>,
+			      <0 0xac6c000 0 0x2000>,
+			      <0 0xac6e000 0 0x1000>,
+			      <0 0xac70000 0 0x1000>,
+			      <0 0xac72000 0 0x1000>,
+			      <0 0xac74000 0 0x1000>,
+			      <0 0xacb4000 0 0xd000>,
+			      <0 0xacc3000 0 0xd000>,
+			      <0 0xacd9000 0 0x2200>,
+			      <0 0xacdb200 0 0x2200>;
+			reg-names = "csiphy0",
+				    "csiphy1",
+				    "csiphy2",
+				    "csiphy3",
+				    "csiphy4",
+				    "csiphy5",
+				    "vfe0",
+				    "vfe1",
+				    "vfe_lite0",
+				    "vfe_lite1";
+
+			interrupts = <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "csiphy0",
+					  "csiphy1",
+					  "csiphy2",
+					  "csiphy3",
+					  "csiphy4",
+					  "csiphy5",
+					  "csid0",
+					  "csid1",
+					  "csid2",
+					  "csid3",
+					  "vfe0",
+					  "vfe1",
+					  "vfe_lite0",
+					  "vfe_lite1";
+
+			power-domains = <&camcc IFE_0_GDSC>,
+					<&camcc IFE_1_GDSC>,
+					<&camcc TITAN_TOP_GDSC>;
+
+			clocks = <&gcc GCC_CAMERA_AHB_CLK>,
+				 <&gcc GCC_CAMERA_HF_AXI_CLK>,
+				 <&gcc GCC_CAMERA_SF_AXI_CLK>,
+				 <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+				 <&camcc CAM_CC_CAMNOC_AXI_CLK_SRC>,
+				 <&camcc CAM_CC_CORE_AHB_CLK>,
+				 <&camcc CAM_CC_CPAS_AHB_CLK>,
+				 <&camcc CAM_CC_CSIPHY0_CLK>,
+				 <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY1_CLK>,
+				 <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY2_CLK>,
+				 <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY3_CLK>,
+				 <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY4_CLK>,
+				 <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY5_CLK>,
+				 <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
+				 <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
+				 <&camcc CAM_CC_IFE_0_AHB_CLK>,
+				 <&camcc CAM_CC_IFE_0_AXI_CLK>,
+				 <&camcc CAM_CC_IFE_0_CLK>,
+				 <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
+				 <&camcc CAM_CC_IFE_0_CSID_CLK>,
+				 <&camcc CAM_CC_IFE_0_AREG_CLK>,
+				 <&camcc CAM_CC_IFE_1_AHB_CLK>,
+				 <&camcc CAM_CC_IFE_1_AXI_CLK>,
+				 <&camcc CAM_CC_IFE_1_CLK>,
+				 <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
+				 <&camcc CAM_CC_IFE_1_CSID_CLK>,
+				 <&camcc CAM_CC_IFE_1_AREG_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_AXI_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
+
+			clock-names = "cam_ahb_clk",
+				      "cam_hf_axi",
+				      "cam_sf_axi",
+				      "camnoc_axi",
+				      "camnoc_axi_src",
+				      "core_ahb",
+				      "cpas_ahb",
+				      "csiphy0",
+				      "csiphy0_timer",
+				      "csiphy1",
+				      "csiphy1_timer",
+				      "csiphy2",
+				      "csiphy2_timer",
+				      "csiphy3",
+				      "csiphy3_timer",
+				      "csiphy4",
+				      "csiphy4_timer",
+				      "csiphy5",
+				      "csiphy5_timer",
+				      "slow_ahb_src",
+				      "vfe0_ahb",
+				      "vfe0_axi",
+				      "vfe0",
+				      "vfe0_cphy_rx",
+				      "vfe0_csid",
+				      "vfe0_areg",
+				      "vfe1_ahb",
+				      "vfe1_axi",
+				      "vfe1",
+				      "vfe1_cphy_rx",
+				      "vfe1_csid",
+				      "vfe1_areg",
+				      "vfe_lite_ahb",
+				      "vfe_lite_axi",
+				      "vfe_lite",
+				      "vfe_lite_cphy_rx",
+				      "vfe_lite_csid";
+
+			iommus = <&apps_smmu 0x800 0x400>,
+				 <&apps_smmu 0x801 0x400>,
+				 <&apps_smmu 0x840 0x400>,
+				 <&apps_smmu 0x841 0x400>,
+				 <&apps_smmu 0xC00 0x400>,
+				 <&apps_smmu 0xC01 0x400>,
+				 <&apps_smmu 0xC40 0x400>,
+				 <&apps_smmu 0xC41 0x400>;
+
+			interconnects = <&gem_noc MASTER_AMPSS_M0 &config_noc SLAVE_CAMERA_CFG>,
+					<&mmss_noc MASTER_CAMNOC_HF &mc_virt SLAVE_EBI_CH0>,
+					<&mmss_noc MASTER_CAMNOC_SF &mc_virt SLAVE_EBI_CH0>,
+					<&mmss_noc MASTER_CAMNOC_ICP &mc_virt SLAVE_EBI_CH0>;
+			interconnect-names = "cam_ahb",
+					     "cam_hf_0_mnoc",
+					     "cam_sf_0_mnoc",
+					     "cam_sf_icp_mnoc";
+		};
+
 		camcc: clock-controller@ad00000 {
 			compatible = "qcom,sm8250-camcc";
 			reg = <0 0x0ad00000 0 0x10000>;
-- 
2.35.1

[PATCH 3/4] arm64: dts: qcom: sm8250: camss: Add downstream camera pin definitions

[Bryan O'Donoghue]

There are a number of pin mux descriptions for the CAMSS block which are
defined downstream.

Bring in the definitions now. Subsequent changes will optionally activate
those muxes for different sm8250 based boards.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 471 +++++++++++++++++++++++++++
 1 file changed, 471 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 3d6c617de045..91ed079edbf7 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3687,6 +3687,477 @@ tlmm: pinctrl@f100000 {
 			gpio-ranges = <&tlmm 0 0 181>;
 			wakeup-parent = <&pdc>;
 
+			cam_sensor_mclk0_active: cam_sensor_mclk0_active {
+				/* MCLK0 */
+				mux {
+					pins = "gpio94";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio94";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk0_suspend: cam_sensor_mclk0_suspend {
+				/* MCLK0 */
+				mux {
+					pins = "gpio94";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio94";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk1_active: cam_sensor_mclk1_active {
+				/* MCLK1 */
+				mux {
+					pins = "gpio95";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio95";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk1_suspend: cam_sensor_mclk1_suspend {
+				/* MCLK1 */
+				mux {
+					pins = "gpio95";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio95";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk2_active: cam_sensor_mclk2_active {
+				/* MCLK2 */
+				mux {
+					pins = "gpio96";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio96";
+					bias-disable;		/* No PULL */
+					drive-strength = <2>;	/* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk2_suspend: cam_sensor_mclk2_suspend {
+				/* MCLK2 */
+				mux {
+					pins = "gpio96";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio96";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk3_active: cam_sensor_mclk3_active {
+				/* MCLK3 */
+				mux {
+					pins = "gpio97";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio97";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk3_suspend: cam_sensor_mclk3_suspend {
+				/* MCLK3 */
+				mux {
+					pins = "gpio97";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio97";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk4_active: cam_sensor_mclk4_active {
+				/* MCLK4 */
+				mux {
+					pins = "gpio98";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio98";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk4_suspend: cam_sensor_mclk4_suspend {
+				/* MCLK4 */
+				mux {
+					pins = "gpio98";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio98";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk5_active: cam_sensor_mclk5_active {
+				/* MCLK5 */
+				mux {
+					pins = "gpio99";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio99";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk5_suspend: cam_sensor_mclk5_suspend {
+				/* MCLK5 */
+				mux {
+					pins = "gpio99";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio99";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk6_active: cam_sensor_mclk6_active {
+				/* MCLK6 */
+				mux {
+					pins = "gpio100";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio100";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_mclk6_suspend: cam_sensor_mclk6_suspend {
+				/* MCLK6 */
+				mux {
+					pins = "gpio100";
+					function = "cam_mclk";
+				};
+
+				config {
+					pins = "gpio100";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_active_rear: cam_sensor_active_rear {
+				/* RESET REAR */
+				mux {
+					pins = "gpio93";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio93";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_suspend_rear: cam_sensor_suspend_rear {
+				/* RESET REAR */
+				mux {
+					pins = "gpio93";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio93";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+					output-low;
+				};
+			};
+
+			cam_sensor_active_rear_aux: cam_sensor_active_rear_aux {
+				/* RESET REARAUX */
+				mux {
+					pins = "gpio92";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio92";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_suspend_rear_aux: cam_sensor_suspend_rear_aux {
+				/* RESET REARAUX */
+				mux {
+					pins = "gpio92";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio92";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+					output-low;
+				};
+			};
+
+			cam_sensor_active_rst2: cam_sensor_active_rst2 {
+				/* RESET 2 */
+				mux {
+					pins = "gpio78";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio78";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_suspend_rst2: cam_sensor_suspend_rst2 {
+				/* RESET 2 */
+				mux {
+					pins = "gpio78";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio78";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+					output-low;
+				};
+			};
+
+			cam_sensor_active_3: cam_sensor_active_3 {
+				/* RESET 3 */
+				mux {
+					pins = "gpio109";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio109";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_suspend_3: cam_sensor_suspend_3 {
+				/* RESET 3 */
+				mux {
+					pins = "gpio109";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio109";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+					output-low;
+				};
+			};
+
+			cam_sensor_active_4: cam_sensor_active_4 {
+				/* RESET 4 */
+				mux {
+					pins = "gpio130";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio130";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_suspend_4: cam_sensor_suspend_4 {
+				/* RESET 4 */
+				mux {
+					pins = "gpio130";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio130";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+					output-low;
+				};
+			};
+
+			cam_sensor_active_5: cam_sensor_active_5 {
+				/* RESET 5 */
+				mux {
+					pins = "gpio131";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio131";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_suspend_5: cam_sensor_suspend_5 {
+				/* RESET 5 */
+				mux {
+					pins = "gpio131";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio131";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+					output-low;
+				};
+			};
+
+			cam_sensor_active_6: cam_sensor_active_6 {
+				/* RESET 6 */
+				mux {
+					pins = "gpio114";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio114";
+					bias-disable; /* No PULL */
+					drive-strength = <2>; /* 2 MA */
+				};
+			};
+
+			cam_sensor_suspend_6: cam_sensor_suspend_6 {
+				/* RESET 6 */
+				mux {
+					pins = "gpio114";
+					function = "gpio";
+				};
+
+				config {
+					pins = "gpio114";
+					bias-pull-down; /* PULL DOWN */
+					drive-strength = <2>; /* 2 MA */
+					output-low;
+				};
+			};
+
+			cci0_default: cci0-default {
+				/* SDA, SCL */
+				pins = "gpio101", "gpio102";
+				function = "cci_i2c";
+
+				bias-pull-up;
+				drive-strength = <2>; /* 2 mA */
+			};
+
+			cci0_sleep: cci0-sleep {
+				/* SDA, SCL */
+				pins = "gpio101", "gpio102";
+				function = "cci_i2c";
+
+				drive-strength = <2>; /* 2 mA */
+				bias-pull-down;
+			};
+
+			cci1_default: cci1-default {
+				/* SDA, SCL */
+				pins = "gpio103", "gpio104";
+				function = "cci_i2c";
+
+				bias-pull-up;
+				drive-strength = <2>; /* 2 mA */
+			};
+
+			cci1_sleep: cci1-sleep {
+				/* SDA, SCL */
+				pins = "gpio103", "gpio104";
+				function = "cci_i2c";
+
+				drive-strength = <2>; /* 2 mA */
+				bias-pull-down;
+			};
+
+			cci2_default: cci2-default {
+				/* SDA, SCL */
+				pins = "gpio105","gpio106";
+				function = "cci_i2c";
+
+				bias-pull-up;
+				drive-strength = <2>; /* 2 mA */
+			};
+
+			cci2_sleep: cci2-sleep {
+				/* SDA, SCL */
+				pins = "gpio105","gpio106";
+				function = "cci_i2c";
+
+				bias-pull-down;
+				drive-strength = <2>; /* 2 mA */
+			};
+
+			cci3_default: cci3-default {
+				/* SDA, SCL */
+				pins = "gpio107","gpio108";
+				function = "cci_i2c";
+
+				bias-pull-up;
+				drive-strength = <2>; /* 2 mA */
+			};
+
+			cci3_sleep: cci3-sleep {
+				/* SDA, SCL */
+				pins = "gpio107","gpio108";
+				function = "cci_i2c";
+
+				bias-pull-down;
+				drive-strength = <2>; /* 2 mA */
+			};
+
 			pri_mi2s_active: pri-mi2s-active {
 				sclk {
 					pins = "gpio138";
-- 
2.35.1

[PATCH 4/4] arm64: dts: qcom: sm8250: camss: Add CCI definitions

[Bryan O'Donoghue]

sm8250 has two CCI busses with two I2C busses apiece.

Co-developed-by: Julian Grahsl <jgrahsl@snap.com>
Signed-off-by: Julian Grahsl <jgrahsl@snap.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 82 ++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 91ed079edbf7..98e96527702b 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3150,6 +3150,88 @@ videocc: clock-controller@abf0000 {
 			#power-domain-cells = <1>;
 		};
 
+		cci0: cci@ac4f000 {
+			compatible = "qcom,sm8250-cci";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			reg = <0 0x0ac4f000 0 0x1000>;
+			interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
+			power-domains = <&camcc TITAN_TOP_GDSC>;
+
+			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+				 <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
+				 <&camcc CAM_CC_CPAS_AHB_CLK>,
+				 <&camcc CAM_CC_CCI_0_CLK>,
+				 <&camcc CAM_CC_CCI_0_CLK_SRC>;
+			clock-names = "camnoc_axi",
+				      "slow_ahb_src",
+				      "cpas_ahb",
+				      "cci",
+				      "cci_src";
+
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&cci0_default &cci1_default>;
+			pinctrl-1 = <&cci0_sleep &cci1_sleep>;
+
+			status = "disabled";
+
+			cci_i2c0: i2c-bus@0 {
+				reg = <0>;
+				clock-frequency = <1000000>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+
+			cci_i2c1: i2c-bus@1 {
+				reg = <1>;
+				clock-frequency = <1000000>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+
+		cci1: cci@ac50000 {
+			compatible = "qcom,sm8250-cci";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			reg = <0 0x0ac50000 0 0x1000>;
+			interrupts = <GIC_SPI 271 IRQ_TYPE_EDGE_RISING>;
+			power-domains = <&camcc TITAN_TOP_GDSC>;
+
+			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+				 <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
+				 <&camcc CAM_CC_CPAS_AHB_CLK>,
+				 <&camcc CAM_CC_CCI_1_CLK>,
+				 <&camcc CAM_CC_CCI_1_CLK_SRC>;
+			clock-names = "camnoc_axi",
+				      "slow_ahb_src",
+				      "cpas_ahb",
+				      "cci",
+				      "cci_src";
+
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&cci2_default &cci3_default>;
+			pinctrl-1 = <&cci2_sleep &cci3_sleep>;
+
+			status = "disabled";
+
+			cci_i2c2: i2c-bus@0 {
+				reg = <0>;
+				clock-frequency = <1000000>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+
+			cci_i2c3: i2c-bus@1 {
+				reg = <1>;
+				clock-frequency = <1000000>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+
 		camss: camss@ac6a000 {
 			compatible = "qcom,sm8250-camss";
 			status = "disabled";
-- 
2.35.1

Re: [PATCH 3/4] arm64: dts: qcom: sm8250: camss: Add downstream camera pin definitions

[Bjorn Andersson]

On Sat 09 Apr 11:45 CDT 2022, Bryan O'Donoghue wrote:

> There are a number of pin mux descriptions for the CAMSS block which are
> defined downstream.
> 
> Bring in the definitions now. Subsequent changes will optionally activate
> those muxes for different sm8250 based boards.
> 

I'm not able to spot where these are referenced, are they not used or am
I just failing the search today?

I would prefer that we don't sprinkle pinctrl states that potentially
might be used at a later time...

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 471 +++++++++++++++++++++++++++
>  1 file changed, 471 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 3d6c617de045..91ed079edbf7 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -3687,6 +3687,477 @@ tlmm: pinctrl@f100000 {
>  			gpio-ranges = <&tlmm 0 0 181>;
>  			wakeup-parent = <&pdc>;
>  
> +			cam_sensor_mclk0_active: cam_sensor_mclk0_active {

Please don't use '_' in the node name.

> +				/* MCLK0 */

I would prefer that we don't inherit the mux/config split upstream.

You can either do:

cam-sensor-mclk0-active-state {
	pins = ;
	function = ;
	bias...
};

or:

cam-sensor-mclk0-active-state {
	mclk0 {
		pins = ;
		function = ;
		bias...
	};
};

And depending on how this state is actually used you could lump together
the configuration of several pins - with different properties - in the
same state node.

Regards,
Bjorn

Re: [PATCH 4/4] arm64: dts: qcom: sm8250: camss: Add CCI definitions

[Bjorn Andersson]

On Sat 09 Apr 11:45 CDT 2022, Bryan O'Donoghue wrote:

> sm8250 has two CCI busses with two I2C busses apiece.
> 
> Co-developed-by: Julian Grahsl <jgrahsl@snap.com>
> Signed-off-by: Julian Grahsl <jgrahsl@snap.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 82 ++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 91ed079edbf7..98e96527702b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -3150,6 +3150,88 @@ videocc: clock-controller@abf0000 {
>  			#power-domain-cells = <1>;
>  		};
>  
> +		cci0: cci@ac4f000 {
> +			compatible = "qcom,sm8250-cci";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			reg = <0 0x0ac4f000 0 0x1000>;
> +			interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
> +			power-domains = <&camcc TITAN_TOP_GDSC>;
> +
> +			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> +				 <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
> +				 <&camcc CAM_CC_CPAS_AHB_CLK>,
> +				 <&camcc CAM_CC_CCI_0_CLK>,
> +				 <&camcc CAM_CC_CCI_0_CLK_SRC>;
> +			clock-names = "camnoc_axi",
> +				      "slow_ahb_src",
> +				      "cpas_ahb",
> +				      "cci",
> +				      "cci_src";
> +
> +			pinctrl-names = "default", "sleep";
> +			pinctrl-0 = <&cci0_default &cci1_default>;
> +			pinctrl-1 = <&cci0_sleep &cci1_sleep>;

I would prefer that you include these in the same patch.

> +
> +			status = "disabled";
> +
> +			cci_i2c0: i2c-bus@0 {
> +				reg = <0>;
> +				clock-frequency = <1000000>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +
> +			cci_i2c1: i2c-bus@1 {
> +				reg = <1>;
> +				clock-frequency = <1000000>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +		};
> +
> +		cci1: cci@ac50000 {
> +			compatible = "qcom,sm8250-cci";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			reg = <0 0x0ac50000 0 0x1000>;
> +			interrupts = <GIC_SPI 271 IRQ_TYPE_EDGE_RISING>;
> +			power-domains = <&camcc TITAN_TOP_GDSC>;
> +
> +			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> +				 <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
> +				 <&camcc CAM_CC_CPAS_AHB_CLK>,
> +				 <&camcc CAM_CC_CCI_1_CLK>,
> +				 <&camcc CAM_CC_CCI_1_CLK_SRC>;
> +			clock-names = "camnoc_axi",
> +				      "slow_ahb_src",
> +				      "cpas_ahb",
> +				      "cci",
> +				      "cci_src";
> +
> +			pinctrl-names = "default", "sleep";
> +			pinctrl-0 = <&cci2_default &cci3_default>;
> +			pinctrl-1 = <&cci2_sleep &cci3_sleep>;
> +
> +			status = "disabled";
> +
> +			cci_i2c2: i2c-bus@0 {

Are these names (the label) used somewhere in the schematics? How about
cci0_i2c0 and cci1_i2c0 instead (unless these names are defined by some
documentation)?

Regards,
Bjorn

> +				reg = <0>;
> +				clock-frequency = <1000000>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +
> +			cci_i2c3: i2c-bus@1 {
> +				reg = <1>;
> +				clock-frequency = <1000000>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +		};
> +
>  		camss: camss@ac6a000 {
>  			compatible = "qcom,sm8250-camss";
>  			status = "disabled";
> -- 
> 2.35.1
> 

Re: [PATCH 2/4] arm64: dts: qcom: sm8250: camss: Add CAMSS block definition

[Bjorn Andersson]

On Sat 09 Apr 11:45 CDT 2022, Bryan O'Donoghue wrote:

> Adds a CAMSS definition block.
> 
> Co-developed-by: Julian Grahsl <jgrahsl@snap.com>
> Signed-off-by: Julian Grahsl <jgrahsl@snap.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 153 +++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 906bc8ed25b7..3d6c617de045 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -3150,6 +3150,159 @@ videocc: clock-controller@abf0000 {
>  			#power-domain-cells = <1>;
>  		};
>  
> +		camss: camss@ac6a000 {
> +			compatible = "qcom,sm8250-camss";
> +			status = "disabled";
> +
> +			reg = <0 0xac6a000 0 0x2000>,
> +			      <0 0xac6c000 0 0x2000>,
> +			      <0 0xac6e000 0 0x1000>,
> +			      <0 0xac70000 0 0x1000>,
> +			      <0 0xac72000 0 0x1000>,
> +			      <0 0xac74000 0 0x1000>,
> +			      <0 0xacb4000 0 0xd000>,
> +			      <0 0xacc3000 0 0xd000>,
> +			      <0 0xacd9000 0 0x2200>,
> +			      <0 0xacdb200 0 0x2200>;
> +			reg-names = "csiphy0",
> +				    "csiphy1",
> +				    "csiphy2",
> +				    "csiphy3",
> +				    "csiphy4",
> +				    "csiphy5",
> +				    "vfe0",
> +				    "vfe1",
> +				    "vfe_lite0",
> +				    "vfe_lite1";
> +
> +			interrupts = <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "csiphy0",
> +					  "csiphy1",
> +					  "csiphy2",
> +					  "csiphy3",
> +					  "csiphy4",
> +					  "csiphy5",
> +					  "csid0",
> +					  "csid1",
> +					  "csid2",
> +					  "csid3",
> +					  "vfe0",
> +					  "vfe1",
> +					  "vfe_lite0",
> +					  "vfe_lite1";
> +
> +			power-domains = <&camcc IFE_0_GDSC>,
> +					<&camcc IFE_1_GDSC>,
> +					<&camcc TITAN_TOP_GDSC>;
> +
> +			clocks = <&gcc GCC_CAMERA_AHB_CLK>,
> +				 <&gcc GCC_CAMERA_HF_AXI_CLK>,
> +				 <&gcc GCC_CAMERA_SF_AXI_CLK>,
> +				 <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> +				 <&camcc CAM_CC_CAMNOC_AXI_CLK_SRC>,
> +				 <&camcc CAM_CC_CORE_AHB_CLK>,
> +				 <&camcc CAM_CC_CPAS_AHB_CLK>,
> +				 <&camcc CAM_CC_CSIPHY0_CLK>,
> +				 <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY1_CLK>,
> +				 <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY2_CLK>,
> +				 <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY3_CLK>,
> +				 <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY4_CLK>,
> +				 <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY5_CLK>,
> +				 <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
> +				 <&camcc CAM_CC_IFE_0_AHB_CLK>,
> +				 <&camcc CAM_CC_IFE_0_AXI_CLK>,
> +				 <&camcc CAM_CC_IFE_0_CLK>,
> +				 <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
> +				 <&camcc CAM_CC_IFE_0_CSID_CLK>,
> +				 <&camcc CAM_CC_IFE_0_AREG_CLK>,
> +				 <&camcc CAM_CC_IFE_1_AHB_CLK>,
> +				 <&camcc CAM_CC_IFE_1_AXI_CLK>,
> +				 <&camcc CAM_CC_IFE_1_CLK>,
> +				 <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
> +				 <&camcc CAM_CC_IFE_1_CSID_CLK>,
> +				 <&camcc CAM_CC_IFE_1_AREG_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_AXI_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
> +
> +			clock-names = "cam_ahb_clk",
> +				      "cam_hf_axi",
> +				      "cam_sf_axi",
> +				      "camnoc_axi",
> +				      "camnoc_axi_src",
> +				      "core_ahb",
> +				      "cpas_ahb",
> +				      "csiphy0",
> +				      "csiphy0_timer",
> +				      "csiphy1",
> +				      "csiphy1_timer",
> +				      "csiphy2",
> +				      "csiphy2_timer",
> +				      "csiphy3",
> +				      "csiphy3_timer",
> +				      "csiphy4",
> +				      "csiphy4_timer",
> +				      "csiphy5",
> +				      "csiphy5_timer",
> +				      "slow_ahb_src",
> +				      "vfe0_ahb",
> +				      "vfe0_axi",
> +				      "vfe0",
> +				      "vfe0_cphy_rx",
> +				      "vfe0_csid",
> +				      "vfe0_areg",
> +				      "vfe1_ahb",
> +				      "vfe1_axi",
> +				      "vfe1",
> +				      "vfe1_cphy_rx",
> +				      "vfe1_csid",
> +				      "vfe1_areg",
> +				      "vfe_lite_ahb",
> +				      "vfe_lite_axi",
> +				      "vfe_lite",
> +				      "vfe_lite_cphy_rx",
> +				      "vfe_lite_csid";
> +
> +			iommus = <&apps_smmu 0x800 0x400>,
> +				 <&apps_smmu 0x801 0x400>,
> +				 <&apps_smmu 0x840 0x400>,
> +				 <&apps_smmu 0x841 0x400>,
> +				 <&apps_smmu 0xC00 0x400>,
> +				 <&apps_smmu 0xC01 0x400>,
> +				 <&apps_smmu 0xC40 0x400>,
> +				 <&apps_smmu 0xC41 0x400>;

Please use lower case hex digits in the iommus property as well.

Thanks,
Bjorn

> +
> +			interconnects = <&gem_noc MASTER_AMPSS_M0 &config_noc SLAVE_CAMERA_CFG>,
> +					<&mmss_noc MASTER_CAMNOC_HF &mc_virt SLAVE_EBI_CH0>,
> +					<&mmss_noc MASTER_CAMNOC_SF &mc_virt SLAVE_EBI_CH0>,
> +					<&mmss_noc MASTER_CAMNOC_ICP &mc_virt SLAVE_EBI_CH0>;
> +			interconnect-names = "cam_ahb",
> +					     "cam_hf_0_mnoc",
> +					     "cam_sf_0_mnoc",
> +					     "cam_sf_icp_mnoc";
> +		};
> +
>  		camcc: clock-controller@ad00000 {
>  			compatible = "qcom,sm8250-camcc";
>  			reg = <0 0x0ad00000 0 0x10000>;
> -- 
> 2.35.1
> 

Re: [PATCH 3/4] arm64: dts: qcom: sm8250: camss: Add downstream camera pin definitions

[Bryan O'Donoghue]

On 12/04/2022 03:04, Bjorn Andersson wrote:
> On Sat 09 Apr 11:45 CDT 2022, Bryan O'Donoghue wrote:
> 
>> There are a number of pin mux descriptions for the CAMSS block which are
>> defined downstream.
>>
>> Bring in the definitions now. Subsequent changes will optionally activate
>> those muxes for different sm8250 based boards.
>>
> 
> I'm not able to spot where these are referenced, are they not used or am
> I just failing the search today?

Used here:

https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=br-v5.19b%2brb5-dts%2bsensors&id=96d26b916efca18450ff67d385d846b50d8eb7e1

I'll send out the RB5 patch once I get regulator_bulk_*() added to the 
imx412 driver

https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=br-v5.19b%2brb5-dts%2bsensors&id=e11966d8101350d0e76eb3e1520165864bdbc296

> 
> I would prefer that we don't sprinkle pinctrl states that potentially
> might be used at a later time...
> 
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 471 +++++++++++++++++++++++++++
>>   1 file changed, 471 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> index 3d6c617de045..91ed079edbf7 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> @@ -3687,6 +3687,477 @@ tlmm: pinctrl@f100000 {
>>   			gpio-ranges = <&tlmm 0 0 181>;
>>   			wakeup-parent = <&pdc>;
>>   
>> +			cam_sensor_mclk0_active: cam_sensor_mclk0_active {
> 
> Please don't use '_' in the node name.
> 
>> +				/* MCLK0 */
> 
> I would prefer that we don't inherit the mux/config split upstream.
> 
> You can either do:
> 
> cam-sensor-mclk0-active-state {
> 	pins = ;
> 	function = ;
> 	bias...
> };
> 
> or:
> 
> cam-sensor-mclk0-active-state {
> 	mclk0 {
> 		pins = ;
> 		function = ;
> 		bias...
> 	};
> };
> 
> And depending on how this state is actually used you could lump together
> the configuration of several pins - with different properties - in the
> same state node.

Sure np

---
bod

Re: [PATCH 4/4] arm64: dts: qcom: sm8250: camss: Add CCI definitions

[Bryan O'Donoghue]

On 12/04/2022 03:06, Bjorn Andersson wrote:
> On Sat 09 Apr 11:45 CDT 2022, Bryan O'Donoghue wrote:
> 
>> sm8250 has two CCI busses with two I2C busses apiece.
>>
>> Co-developed-by: Julian Grahsl <jgrahsl@snap.com>
>> Signed-off-by: Julian Grahsl <jgrahsl@snap.com>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 82 ++++++++++++++++++++++++++++
>>   1 file changed, 82 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> index 91ed079edbf7..98e96527702b 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> @@ -3150,6 +3150,88 @@ videocc: clock-controller@abf0000 {
>>   			#power-domain-cells = <1>;
>>   		};
>>   
>> +		cci0: cci@ac4f000 {
>> +			compatible = "qcom,sm8250-cci";
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			reg = <0 0x0ac4f000 0 0x1000>;
>> +			interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
>> +			power-domains = <&camcc TITAN_TOP_GDSC>;
>> +
>> +			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
>> +				 <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
>> +				 <&camcc CAM_CC_CPAS_AHB_CLK>,
>> +				 <&camcc CAM_CC_CCI_0_CLK>,
>> +				 <&camcc CAM_CC_CCI_0_CLK_SRC>;
>> +			clock-names = "camnoc_axi",
>> +				      "slow_ahb_src",
>> +				      "cpas_ahb",
>> +				      "cci",
>> +				      "cci_src";
>> +
>> +			pinctrl-names = "default", "sleep";
>> +			pinctrl-0 = <&cci0_default &cci1_default>;
>> +			pinctrl-1 = <&cci0_sleep &cci1_sleep>;
> 
> I would prefer that you include these in the same patch.

You mean CAMSS and CCI in the one patch ?
Sure.

> 
>> +
>> +			status = "disabled";
>> +
>> +			cci_i2c0: i2c-bus@0 {
>> +				reg = <0>;
>> +				clock-frequency = <1000000>;
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +			};
>> +
>> +			cci_i2c1: i2c-bus@1 {
>> +				reg = <1>;
>> +				clock-frequency = <1000000>;
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +			};
>> +		};
>> +
>> +		cci1: cci@ac50000 {
>> +			compatible = "qcom,sm8250-cci";
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			reg = <0 0x0ac50000 0 0x1000>;
>> +			interrupts = <GIC_SPI 271 IRQ_TYPE_EDGE_RISING>;
>> +			power-domains = <&camcc TITAN_TOP_GDSC>;
>> +
>> +			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
>> +				 <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
>> +				 <&camcc CAM_CC_CPAS_AHB_CLK>,
>> +				 <&camcc CAM_CC_CCI_1_CLK>,
>> +				 <&camcc CAM_CC_CCI_1_CLK_SRC>;
>> +			clock-names = "camnoc_axi",
>> +				      "slow_ahb_src",
>> +				      "cpas_ahb",
>> +				      "cci",
>> +				      "cci_src";
>> +
>> +			pinctrl-names = "default", "sleep";
>> +			pinctrl-0 = <&cci2_default &cci3_default>;
>> +			pinctrl-1 = <&cci2_sleep &cci3_sleep>;
>> +
>> +			status = "disabled";
>> +
>> +			cci_i2c2: i2c-bus@0 {
> 
> Are these names (the label) used somewhere in the schematics? How about
> cci0_i2c0 and cci1_i2c0 instead (unless these names are defined by some
> documentation)?

Schematic just says cci_i2c_sda0, cci_i2c_sda3 and so on.

I'll rename.

---
bod

Re: [PATCH 4/4] arm64: dts: qcom: sm8250: camss: Add CCI definitions

[Bryan O'Donoghue]

On 12/04/2022 03:06, Bjorn Andersson wrote:
>> +			pinctrl-names = "default", "sleep";
>> +			pinctrl-0 = <&cci0_default &cci1_default>;
>> +			pinctrl-1 = <&cci0_sleep &cci1_sleep>;
> I would prefer that you include these in the same patch.
> 

You mean the CCI definition and the pinmux for it

OK, yes