* [PATCH 1/6] dt-bindings: i2c: qcom-cci: Document x1e80100 compatible
2024-11-19 13:10 [PATCH 0/6] Add dt-bindings and dtsi changes for CAMSS on x1e80100 silicon Bryan O'Donoghue
@ 2024-11-19 13:10 ` Bryan O'Donoghue
2024-11-20 8:49 ` Krzysztof Kozlowski
2024-11-19 13:10 ` [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding Bryan O'Donoghue
` (4 subsequent siblings)
5 siblings, 1 reply; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 13:10 UTC (permalink / raw)
To: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk, Bryan O'Donoghue
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml
index ef26ba6eda28e95875853fe5043fe11deb5af088..89f852ca0d6b8a6b57b596eca0a3765efd058f39 100644
--- a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml
@@ -34,6 +34,7 @@ properties:
- qcom,sm8450-cci
- qcom,sm8550-cci
- qcom,sm8650-cci
+ - qcom,x1e80100-cci
- const: qcom,msm8996-cci # CCI v2
"#address-cells":
@@ -205,6 +206,7 @@ allOf:
contains:
enum:
- qcom,sc8280xp-cci
+ - qcom,x1e80100-cci
then:
properties:
clocks:
--
2.45.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 1/6] dt-bindings: i2c: qcom-cci: Document x1e80100 compatible
2024-11-19 13:10 ` [PATCH 1/6] dt-bindings: i2c: qcom-cci: Document x1e80100 compatible Bryan O'Donoghue
@ 2024-11-20 8:49 ` Krzysztof Kozlowski
2024-11-20 14:35 ` Bryan O'Donoghue
0 siblings, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-20 8:49 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On Tue, Nov 19, 2024 at 01:10:30PM +0000, Bryan O'Donoghue wrote:
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
Missing commit msg. Checkpatch :)
> Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml | 2 ++
> 1 file changed, 2 insertions(+)
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6] dt-bindings: i2c: qcom-cci: Document x1e80100 compatible
2024-11-20 8:49 ` Krzysztof Kozlowski
@ 2024-11-20 14:35 ` Bryan O'Donoghue
0 siblings, 0 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-20 14:35 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 20/11/2024 08:49, Krzysztof Kozlowski wrote:
> On Tue, Nov 19, 2024 at 01:10:30PM +0000, Bryan O'Donoghue wrote:
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>
> Missing commit msg. Checkpatch :)
>
>> Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml | 2 ++
>> 1 file changed, 2 insertions(+)
>
> Best regards,
> Krzysztof
>
How did I miss that...
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding
2024-11-19 13:10 [PATCH 0/6] Add dt-bindings and dtsi changes for CAMSS on x1e80100 silicon Bryan O'Donoghue
2024-11-19 13:10 ` [PATCH 1/6] dt-bindings: i2c: qcom-cci: Document x1e80100 compatible Bryan O'Donoghue
@ 2024-11-19 13:10 ` Bryan O'Donoghue
2024-11-19 14:34 ` Vladimir Zapolskiy
2024-11-20 8:55 ` Krzysztof Kozlowski
2024-11-19 13:10 ` [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC Bryan O'Donoghue
` (3 subsequent siblings)
5 siblings, 2 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 13:10 UTC (permalink / raw)
To: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk, Bryan O'Donoghue
Add bindings for qcom,x1e80100-camss in order to support the camera
subsystem for x1e80100 as found in various Co-Pilot laptops.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
.../bindings/media/qcom,x1e80100-camss.yaml | 354 +++++++++++++++++++++
1 file changed, 354 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..ca2499cd52a51e14bad3cf8a8ca94c9d23ed5030
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
@@ -0,0 +1,354 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,x1e80100-camss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm X1E80100 Camera Subsystem (CAMSS)
+
+maintainers:
+ - Bryan O'Donoghue <bryan.odonoghue@linaro.org>
+
+description: |
+ The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
+
+properties:
+ compatible:
+ const: qcom,x1e80100-camss
+
+ clocks:
+ maxItems: 29
+
+ clock-names:
+ items:
+ - const: camnoc_rt_axi
+ - const: camnoc_nrt_axi
+ - const: core_ahb
+ - const: cpas_ahb
+ - const: cpas_fast_ahb
+ - const: cpas_vfe0
+ - const: cpas_vfe1
+ - const: cpas_vfe_lite
+ - const: cphy_rx_clk_src
+ - const: csid
+ - const: csid_csiphy_rx
+ - const: csiphy0
+ - const: csiphy0_timer
+ - const: csiphy1
+ - const: csiphy1_timer
+ - const: csiphy2
+ - const: csiphy2_timer
+ - const: csiphy4
+ - const: csiphy4_timer
+ - const: gcc_axi_hf
+ - const: gcc_axi_sf
+ - const: vfe0
+ - const: vfe0_fast_ahb
+ - const: vfe1
+ - const: vfe1_fast_ahb
+ - const: vfe_lite
+ - const: vfe_lite_ahb
+ - const: vfe_lite_cphy_rx
+ - const: vfe_lite_csid
+
+ interrupts:
+ maxItems: 13
+
+ interrupt-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csid_lite0
+ - const: csid_lite1
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: csiphy4
+ - const: vfe0
+ - const: vfe1
+ - const: vfe_lite0
+ - const: vfe_lite1
+
+ iommus:
+ maxItems: 13
+
+ interconnects:
+ maxItems: 4
+
+ interconnect-names:
+ items:
+ - const: cam_ahb
+ - const: cam_hf_mnoc
+ - const: cam_sf_mnoc
+ - const: cam_sf_icp_mnoc
+
+ power-domains:
+ items:
+ - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
+ - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
+ - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
+
+ power-domain-names:
+ items:
+ - const: ife0
+ - const: ife1
+ - const: top
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ description:
+ CSI input ports.
+
+ patternProperties:
+ "^port@[03]+$":
+ $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:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+ reg:
+ maxItems: 12
+
+ reg-names:
+ items:
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csid_wrapper
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: csiphy4
+ - const: vfe_lite0
+ - const: vfe_lite1
+ - const: vfe0
+ - const: vfe1
+
+ vdda-phy-supply:
+ description:
+ Phandle to a 0.9V regulator supply to PHY core block.
+
+ vdda-pll-supply:
+ description:
+ Phandle to 1.2V regulator supply to PHY refclk pll block.
+
+required:
+ - clock-names
+ - clocks
+ - compatible
+ - interconnects
+ - interconnect-names
+ - interrupts
+ - interrupt-names
+ - iommus
+ - ports
+ - power-domains
+ - power-domain-names
+ - reg
+ - reg-names
+ - vdda-phy-supply
+ - vdda-pll-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
+ #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
+ #include <dt-bindings/interconnect/qcom,x1e80100-rpmh.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ camss: camss@ac62000 {
+ compatible = "qcom,x1e80100-camss";
+
+ reg = <0 0x0acb7000 0 0x2000>,
+ <0 0x0acb9000 0 0x2000>,
+ <0 0x0acbb000 0 0x2000>,
+ <0 0x0acb6000 0 0x1000>,
+ <0 0x0ace4000 0 0x1000>,
+ <0 0x0ace6000 0 0x1000>,
+ <0 0x0ace8000 0 0x1000>,
+ <0 0x0acec000 0 0x4000>,
+ <0 0x0acc7000 0 0x2000>,
+ <0 0x0accb000 0 0x2000>,
+ <0 0x0ac62000 0 0x2a00>,
+ <0 0x0ac71000 0 0x2a00>;
+
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_wrapper",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy4",
+ "vfe_lite0",
+ "vfe_lite1",
+ "vfe0",
+ "vfe1";
+
+ vdda-phy-supply = <&csiphy0_vdda_phy_supply>;
+ vdda-pll-supply = <&csiphy0_vdda_pll_supply>;
+
+ interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 359 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 122 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 = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy4",
+ "vfe0",
+ "vfe1",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ power-domains = <&camcc CAM_CC_IFE_0_GDSC>,
+ <&camcc CAM_CC_IFE_1_GDSC>,
+ <&camcc CAM_CC_TITAN_TOP_GDSC>;
+
+ power-domain-names = "ife0",
+ "ife1",
+ "top";
+
+ clocks = <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
+ <&camcc CAM_CC_CAMNOC_AXI_NRT_CLK>,
+ <&camcc CAM_CC_CORE_AHB_CLK>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_CPAS_FAST_AHB_CLK>,
+ <&camcc CAM_CC_CPAS_IFE_0_CLK>,
+ <&camcc CAM_CC_CPAS_IFE_1_CLK>,
+ <&camcc CAM_CC_CPAS_IFE_LITE_CLK>,
+ <&camcc CAM_CC_CPHY_RX_CLK_SRC>,
+ <&camcc CAM_CC_CSID_CLK>,
+ <&camcc CAM_CC_CSID_CSIPHY_RX_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_CSIPHY4_CLK>,
+ <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
+ <&gcc GCC_CAMERA_HF_AXI_CLK>,
+ <&gcc GCC_CAMERA_SF_AXI_CLK>,
+ <&camcc CAM_CC_IFE_0_CLK>,
+ <&camcc CAM_CC_IFE_0_FAST_AHB_CLK>,
+ <&camcc CAM_CC_IFE_1_CLK>,
+ <&camcc CAM_CC_IFE_1_FAST_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
+
+ clock-names = "camnoc_rt_axi",
+ "camnoc_nrt_axi",
+ "core_ahb",
+ "cpas_ahb",
+ "cpas_fast_ahb",
+ "cpas_vfe0",
+ "cpas_vfe1",
+ "cpas_vfe_lite",
+ "cphy_rx_clk_src",
+ "csid",
+ "csid_csiphy_rx",
+ "csiphy0",
+ "csiphy0_timer",
+ "csiphy1",
+ "csiphy1_timer",
+ "csiphy2",
+ "csiphy2_timer",
+ "csiphy4",
+ "csiphy4_timer",
+ "gcc_axi_hf",
+ "gcc_axi_sf",
+ "vfe0",
+ "vfe0_fast_ahb",
+ "vfe1",
+ "vfe1_fast_ahb",
+ "vfe_lite",
+ "vfe_lite_ahb",
+ "vfe_lite_cphy_rx",
+ "vfe_lite_csid";
+
+ iommus = <&apps_smmu 0x800 0x60>,
+ <&apps_smmu 0x820 0x60>,
+ <&apps_smmu 0x840 0x60>,
+ <&apps_smmu 0x860 0x60>,
+ <&apps_smmu 0x1800 0x60>,
+ <&apps_smmu 0x1820 0x60>,
+ <&apps_smmu 0x1840 0x60>,
+ <&apps_smmu 0x1860 0x60>,
+ <&apps_smmu 0x18a0 0x00>,
+ <&apps_smmu 0x18e0 0x00>,
+ <&apps_smmu 0x1980 0x20>,
+ <&apps_smmu 0x1900 0x00>,
+ <&apps_smmu 0x19a0 0x20>;
+
+ interconnects = <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_CAMERA_CFG 0>,
+ <&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt SLAVE_EBI1 0>,
+ <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI1 0>,
+ <&mmss_noc MASTER_CAMNOC_ICP 0 &mc_virt SLAVE_EBI1 0>;
+ interconnect-names = "cam_ahb",
+ "cam_hf_mnoc",
+ "cam_sf_mnoc",
+ "cam_sf_icp_mnoc";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ csiphy_ep0: endpoint {
+ clock-lanes = <7>;
+ data-lanes = <0 1>;
+ remote-endpoint = <&sensor_ep>;
+ };
+ };
+ };
+ };
+ };
--
2.45.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding
2024-11-19 13:10 ` [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding Bryan O'Donoghue
@ 2024-11-19 14:34 ` Vladimir Zapolskiy
2024-11-19 15:11 ` Bryan O'Donoghue
2024-11-20 8:55 ` Krzysztof Kozlowski
1 sibling, 1 reply; 31+ messages in thread
From: Vladimir Zapolskiy @ 2024-11-19 14:34 UTC (permalink / raw)
To: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
Hi Bryan,
please find a few review comments below.
On 11/19/24 15:10, Bryan O'Donoghue wrote:
> Add bindings for qcom,x1e80100-camss in order to support the camera
> subsystem for x1e80100 as found in various Co-Pilot laptops.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> .../bindings/media/qcom,x1e80100-camss.yaml | 354 +++++++++++++++++++++
> 1 file changed, 354 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..ca2499cd52a51e14bad3cf8a8ca94c9d23ed5030
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
> @@ -0,0 +1,354 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,x1e80100-camss.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm X1E80100 Camera Subsystem (CAMSS)
> +
> +maintainers:
> + - Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> +
> +description: |
> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
> +
> +properties:
> + compatible:
> + const: qcom,x1e80100-camss
> +
> + clocks:
> + maxItems: 29
> +
> + clock-names:
> + items:
> + - const: camnoc_rt_axi
> + - const: camnoc_nrt_axi
> + - const: core_ahb
> + - const: cpas_ahb
> + - const: cpas_fast_ahb
> + - const: cpas_vfe0
> + - const: cpas_vfe1
> + - const: cpas_vfe_lite
> + - const: cphy_rx_clk_src
> + - const: csid
> + - const: csid_csiphy_rx
> + - const: csiphy0
> + - const: csiphy0_timer
> + - const: csiphy1
> + - const: csiphy1_timer
> + - const: csiphy2
> + - const: csiphy2_timer
> + - const: csiphy4
> + - const: csiphy4_timer
What does happen to csiphy3? Could it fall through the cracks?
> + - const: gcc_axi_hf
> + - const: gcc_axi_sf
> + - const: vfe0
> + - const: vfe0_fast_ahb
> + - const: vfe1
> + - const: vfe1_fast_ahb
> + - const: vfe_lite
> + - const: vfe_lite_ahb
> + - const: vfe_lite_cphy_rx
> + - const: vfe_lite_csid
> +
> + interrupts:
> + maxItems: 13
> +
> + interrupt-names:
> + items:
> + - const: csid0
> + - const: csid1
> + - const: csid2
> + - const: csid_lite0
> + - const: csid_lite1
> + - const: csiphy0
> + - const: csiphy1
> + - const: csiphy2
> + - const: csiphy4
> + - const: vfe0
> + - const: vfe1
> + - const: vfe_lite0
> + - const: vfe_lite1
> +
> + iommus:
> + maxItems: 13
> +
> + interconnects:
> + maxItems: 4
> +
> + interconnect-names:
> + items:
> + - const: cam_ahb
> + - const: cam_hf_mnoc
> + - const: cam_sf_mnoc
> + - const: cam_sf_icp_mnoc
> +
> + power-domains:
> + items:
> + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
> + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
> + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
> +
> + power-domain-names:
> + items:
> + - const: ife0
> + - const: ife1
> + - const: top
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + description:
> + CSI input ports.
> +
> + patternProperties:
> + "^port@[03]+$":
> + $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:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + required:
> + - clock-lanes
> + - data-lanes
> +
> + reg:
> + maxItems: 12
> +
> + reg-names:
> + items:
> + - const: csid0
> + - const: csid1
> + - const: csid2
> + - const: csid_wrapper
> + - const: csiphy0
> + - const: csiphy1
> + - const: csiphy2
> + - const: csiphy4
> + - const: vfe_lite0
> + - const: vfe_lite1
> + - const: vfe0
> + - const: vfe1
> +
> + vdda-phy-supply:
> + description:
> + Phandle to a 0.9V regulator supply to PHY core block.
> +
> + vdda-pll-supply:
> + description:
> + Phandle to 1.2V regulator supply to PHY refclk pll block.
I believe it's very unlikely that the SoC pads are called like this,
as we discussed it in the recent past.
Please rename the properties to reflect the names inherited from
the actual hardware.
> +
> +required:
> + - clock-names
> + - clocks
> + - compatible
> + - interconnects
> + - interconnect-names
> + - interrupts
> + - interrupt-names
> + - iommus
> + - ports
> + - power-domains
> + - power-domain-names
> + - reg
> + - reg-names
> + - vdda-phy-supply
> + - vdda-pll-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
> + #include <dt-bindings/interconnect/qcom,x1e80100-rpmh.h>
> + #include <dt-bindings/power/qcom-rpmpd.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + camss: camss@ac62000 {
> + compatible = "qcom,x1e80100-camss";
> +
> + reg = <0 0x0acb7000 0 0x2000>,
As usual, and at no surprise, there is an immediate problem with
the incorrespondent unit address.
> + <0 0x0acb9000 0 0x2000>,
> + <0 0x0acbb000 0 0x2000>,
> + <0 0x0acb6000 0 0x1000>,
> + <0 0x0ace4000 0 0x1000>,
> + <0 0x0ace6000 0 0x1000>,
> + <0 0x0ace8000 0 0x1000>,
> + <0 0x0acec000 0 0x4000>,
> + <0 0x0acc7000 0 0x2000>,
> + <0 0x0accb000 0 0x2000>,
> + <0 0x0ac62000 0 0x2a00>,
> + <0 0x0ac71000 0 0x2a00>;
> +
> + reg-names = "csid0",
> + "csid1",
> + "csid2",
> + "csid_wrapper",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "csiphy4",
> + "vfe_lite0",
> + "vfe_lite1",
> + "vfe0",
> + "vfe1";
> +
> + vdda-phy-supply = <&csiphy0_vdda_phy_supply>;
> + vdda-pll-supply = <&csiphy0_vdda_pll_supply>;
> +
> + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 359 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 122 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 = "csid0",
> + "csid1",
> + "csid2",
> + "csid_lite0",
> + "csid_lite1",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "csiphy4",
> + "vfe0",
> + "vfe1",
> + "vfe_lite0",
> + "vfe_lite1";
> +
> + power-domains = <&camcc CAM_CC_IFE_0_GDSC>,
> + <&camcc CAM_CC_IFE_1_GDSC>,
> + <&camcc CAM_CC_TITAN_TOP_GDSC>;
> +
> + power-domain-names = "ife0",
> + "ife1",
> + "top";
> +
> + clocks = <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
> + <&camcc CAM_CC_CAMNOC_AXI_NRT_CLK>,
> + <&camcc CAM_CC_CORE_AHB_CLK>,
> + <&camcc CAM_CC_CPAS_AHB_CLK>,
> + <&camcc CAM_CC_CPAS_FAST_AHB_CLK>,
> + <&camcc CAM_CC_CPAS_IFE_0_CLK>,
> + <&camcc CAM_CC_CPAS_IFE_1_CLK>,
> + <&camcc CAM_CC_CPAS_IFE_LITE_CLK>,
> + <&camcc CAM_CC_CPHY_RX_CLK_SRC>,
> + <&camcc CAM_CC_CSID_CLK>,
> + <&camcc CAM_CC_CSID_CSIPHY_RX_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_CSIPHY4_CLK>,
> + <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
> + <&gcc GCC_CAMERA_HF_AXI_CLK>,
> + <&gcc GCC_CAMERA_SF_AXI_CLK>,
> + <&camcc CAM_CC_IFE_0_CLK>,
> + <&camcc CAM_CC_IFE_0_FAST_AHB_CLK>,
> + <&camcc CAM_CC_IFE_1_CLK>,
> + <&camcc CAM_CC_IFE_1_FAST_AHB_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CLK>,
> + <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
> + <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
> +
> + clock-names = "camnoc_rt_axi",
> + "camnoc_nrt_axi",
> + "core_ahb",
> + "cpas_ahb",
> + "cpas_fast_ahb",
> + "cpas_vfe0",
> + "cpas_vfe1",
> + "cpas_vfe_lite",
> + "cphy_rx_clk_src",
> + "csid",
> + "csid_csiphy_rx",
> + "csiphy0",
> + "csiphy0_timer",
> + "csiphy1",
> + "csiphy1_timer",
> + "csiphy2",
> + "csiphy2_timer",
> + "csiphy4",
> + "csiphy4_timer",
> + "gcc_axi_hf",
> + "gcc_axi_sf",
> + "vfe0",
> + "vfe0_fast_ahb",
> + "vfe1",
> + "vfe1_fast_ahb",
> + "vfe_lite",
> + "vfe_lite_ahb",
> + "vfe_lite_cphy_rx",
> + "vfe_lite_csid";
> +
> + iommus = <&apps_smmu 0x800 0x60>,
> + <&apps_smmu 0x820 0x60>,
> + <&apps_smmu 0x840 0x60>,
> + <&apps_smmu 0x860 0x60>,
> + <&apps_smmu 0x1800 0x60>,
> + <&apps_smmu 0x1820 0x60>,
> + <&apps_smmu 0x1840 0x60>,
> + <&apps_smmu 0x1860 0x60>,
> + <&apps_smmu 0x18a0 0x00>,
> + <&apps_smmu 0x18e0 0x00>,
> + <&apps_smmu 0x1980 0x20>,
> + <&apps_smmu 0x1900 0x00>,
> + <&apps_smmu 0x19a0 0x20>;
> +
> + interconnects = <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_CAMERA_CFG 0>,
> + <&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt SLAVE_EBI1 0>,
> + <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI1 0>,
> + <&mmss_noc MASTER_CAMNOC_ICP 0 &mc_virt SLAVE_EBI1 0>;
> + interconnect-names = "cam_ahb",
> + "cam_hf_mnoc",
> + "cam_sf_mnoc",
> + "cam_sf_icp_mnoc";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
It's unclear why #address-cells/#size-cells are needed here.
> +
> + csiphy_ep0: endpoint {
> + clock-lanes = <7>;
As it's known, there is no lane 7.
> + data-lanes = <0 1>;
> + remote-endpoint = <&sensor_ep>;
> + };
> + };
> + };
> + };
> + };
>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding
2024-11-19 14:34 ` Vladimir Zapolskiy
@ 2024-11-19 15:11 ` Bryan O'Donoghue
2024-11-20 23:02 ` Vladimir Zapolskiy
0 siblings, 1 reply; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 15:11 UTC (permalink / raw)
To: Vladimir Zapolskiy, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 19/11/2024 14:34, Vladimir Zapolskiy wrote:
> Hi Bryan,
>
> please find a few review comments below.
>
> On 11/19/24 15:10, Bryan O'Donoghue wrote:
>> Add bindings for qcom,x1e80100-camss in order to support the camera
>> subsystem for x1e80100 as found in various Co-Pilot laptops.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>> .../bindings/media/qcom,x1e80100-camss.yaml | 354 +++++++++++
>> ++++++++++
>> 1 file changed, 354 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-
>> camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-
>> camss.yaml
>> new file mode 100644
>> index
>> 0000000000000000000000000000000000000000..ca2499cd52a51e14bad3cf8a8ca94c9d23ed5030
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
>> @@ -0,0 +1,354 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/qcom,x1e80100-camss.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm X1E80100 Camera Subsystem (CAMSS)
>> +
>> +maintainers:
>> + - Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> +
>> +description: |
>> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
>> +
>> +properties:
>> + compatible:
>> + const: qcom,x1e80100-camss
>> +
>> + clocks:
>> + maxItems: 29
>> +
>> + clock-names:
>> + items:
>> + - const: camnoc_rt_axi
>> + - const: camnoc_nrt_axi
>> + - const: core_ahb
>> + - const: cpas_ahb
>> + - const: cpas_fast_ahb
>> + - const: cpas_vfe0
>> + - const: cpas_vfe1
>> + - const: cpas_vfe_lite
>> + - const: cphy_rx_clk_src
>> + - const: csid
>> + - const: csid_csiphy_rx
>> + - const: csiphy0
>> + - const: csiphy0_timer
>> + - const: csiphy1
>> + - const: csiphy1_timer
>> + - const: csiphy2
>> + - const: csiphy2_timer
>> + - const: csiphy4
>> + - const: csiphy4_timer
>
> What does happen to csiphy3? Could it fall through the cracks?
>
Nope.
For whatever reason csiphy4 is the name here. I guess different SKUs
have been fused out this way. I'd assume there's some version that does
csiphy0-csiphy4 inclusive.
Not here though.
>> + - const: gcc_axi_hf
>> + - const: gcc_axi_sf
>> + - const: vfe0
>> + - const: vfe0_fast_ahb
>> + - const: vfe1
>> + - const: vfe1_fast_ahb
>> + - const: vfe_lite
>> + - const: vfe_lite_ahb
>> + - const: vfe_lite_cphy_rx
>> + - const: vfe_lite_csid
>> +
>> + interrupts:
>> + maxItems: 13
>> +
>> + interrupt-names:
>> + items:
>> + - const: csid0
>> + - const: csid1
>> + - const: csid2
>> + - const: csid_lite0
>> + - const: csid_lite1
>> + - const: csiphy0
>> + - const: csiphy1
>> + - const: csiphy2
>> + - const: csiphy4
>> + - const: vfe0
>> + - const: vfe1
>> + - const: vfe_lite0
>> + - const: vfe_lite1
>> +
>> + iommus:
>> + maxItems: 13
>> +
>> + interconnects:
>> + maxItems: 4
>> +
>> + interconnect-names:
>> + items:
>> + - const: cam_ahb
>> + - const: cam_hf_mnoc
>> + - const: cam_sf_mnoc
>> + - const: cam_sf_icp_mnoc
>> +
>> + power-domains:
>> + items:
>> + - description: IFE0 GDSC - Image Front End, Global Distributed
>> Switch Controller.
>> + - description: IFE1 GDSC - Image Front End, Global Distributed
>> Switch Controller.
>> + - description: Titan Top GDSC - Titan ISP Block, Global
>> Distributed Switch Controller.
>> +
>> + power-domain-names:
>> + items:
>> + - const: ife0
>> + - const: ife1
>> + - const: top
>> +
>> + ports:
>> + $ref: /schemas/graph.yaml#/properties/ports
>> +
>> + description:
>> + CSI input ports.
>> +
>> + patternProperties:
>> + "^port@[03]+$":
>> + $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:
>> + clock-lanes:
>> + maxItems: 1
>> +
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>> +
>> + required:
>> + - clock-lanes
>> + - data-lanes
>> +
>> + reg:
>> + maxItems: 12
>> +
>> + reg-names:
>> + items:
>> + - const: csid0
>> + - const: csid1
>> + - const: csid2
>> + - const: csid_wrapper
>> + - const: csiphy0
>> + - const: csiphy1
>> + - const: csiphy2
>> + - const: csiphy4
>> + - const: vfe_lite0
>> + - const: vfe_lite1
>> + - const: vfe0
>> + - const: vfe1
>> +
>> + vdda-phy-supply:
>> + description:
>> + Phandle to a 0.9V regulator supply to PHY core block.
>> +
>> + vdda-pll-supply:
>> + description:
>> + Phandle to 1.2V regulator supply to PHY refclk pll block.
>
> I believe it's very unlikely that the SoC pads are called like this,
> as we discussed it in the recent past.
>
> Please rename the properties to reflect the names inherited from
> the actual hardware.
I believe we agreed to convert to the PHY infrastructure after 8550,
7280 and x1e80100.
So these names should rename as is.
>
>> +
>> +required:
>> + - clock-names
>> + - clocks
>> + - compatible
>> + - interconnects
>> + - interconnect-names
>> + - interrupts
>> + - interrupt-names
>> + - iommus
>> + - ports
>> + - power-domains
>> + - power-domain-names
>> + - reg
>> + - reg-names
>> + - vdda-phy-supply
>> + - vdda-pll-supply
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>> + #include <dt-bindings/interconnect/qcom,x1e80100-rpmh.h>
>> + #include <dt-bindings/power/qcom-rpmpd.h>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + camss: camss@ac62000 {
>> + compatible = "qcom,x1e80100-camss";
>> +
>> + reg = <0 0x0acb7000 0 0x2000>,
>
> As usual, and at no surprise, there is an immediate problem with
> the incorrespondent unit address.
>
Actually you're right
git show c830aff08d51f8391e59fc6744757c58e320b41b
this shoiuld be "vfe0" first and then sorted alphabetically by IP.
:(
>> + <0 0x0acb9000 0 0x2000>,
>> + <0 0x0acbb000 0 0x2000>,
>> + <0 0x0acb6000 0 0x1000>,
>> + <0 0x0ace4000 0 0x1000>,
>> + <0 0x0ace6000 0 0x1000>,
>> + <0 0x0ace8000 0 0x1000>,
>> + <0 0x0acec000 0 0x4000>,
>> + <0 0x0acc7000 0 0x2000>,
>> + <0 0x0accb000 0 0x2000>,
>> + <0 0x0ac62000 0 0x2a00>,
>> + <0 0x0ac71000 0 0x2a00>;
>> +
>> + reg-names = "csid0",
>> + "csid1",
>> + "csid2",
>> + "csid_wrapper",
>> + "csiphy0",
>> + "csiphy1",
>> + "csiphy2",
>> + "csiphy4",
>> + "vfe_lite0",
>> + "vfe_lite1",
>> + "vfe0",
>> + "vfe1";
>> +
>> + vdda-phy-supply = <&csiphy0_vdda_phy_supply>;
>> + vdda-pll-supply = <&csiphy0_vdda_pll_supply>;
>> +
>> + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 359 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 122 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 = "csid0",
>> + "csid1",
>> + "csid2",
>> + "csid_lite0",
>> + "csid_lite1",
>> + "csiphy0",
>> + "csiphy1",
>> + "csiphy2",
>> + "csiphy4",
>> + "vfe0",
>> + "vfe1",
>> + "vfe_lite0",
>> + "vfe_lite1";
>> +
>> + power-domains = <&camcc CAM_CC_IFE_0_GDSC>,
>> + <&camcc CAM_CC_IFE_1_GDSC>,
>> + <&camcc CAM_CC_TITAN_TOP_GDSC>;
>> +
>> + power-domain-names = "ife0",
>> + "ife1",
>> + "top";
>> +
>> + clocks = <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
>> + <&camcc CAM_CC_CAMNOC_AXI_NRT_CLK>,
>> + <&camcc CAM_CC_CORE_AHB_CLK>,
>> + <&camcc CAM_CC_CPAS_AHB_CLK>,
>> + <&camcc CAM_CC_CPAS_FAST_AHB_CLK>,
>> + <&camcc CAM_CC_CPAS_IFE_0_CLK>,
>> + <&camcc CAM_CC_CPAS_IFE_1_CLK>,
>> + <&camcc CAM_CC_CPAS_IFE_LITE_CLK>,
>> + <&camcc CAM_CC_CPHY_RX_CLK_SRC>,
>> + <&camcc CAM_CC_CSID_CLK>,
>> + <&camcc CAM_CC_CSID_CSIPHY_RX_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_CSIPHY4_CLK>,
>> + <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
>> + <&gcc GCC_CAMERA_HF_AXI_CLK>,
>> + <&gcc GCC_CAMERA_SF_AXI_CLK>,
>> + <&camcc CAM_CC_IFE_0_CLK>,
>> + <&camcc CAM_CC_IFE_0_FAST_AHB_CLK>,
>> + <&camcc CAM_CC_IFE_1_CLK>,
>> + <&camcc CAM_CC_IFE_1_FAST_AHB_CLK>,
>> + <&camcc CAM_CC_IFE_LITE_CLK>,
>> + <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
>> + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
>> + <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
>> +
>> + clock-names = "camnoc_rt_axi",
>> + "camnoc_nrt_axi",
>> + "core_ahb",
>> + "cpas_ahb",
>> + "cpas_fast_ahb",
>> + "cpas_vfe0",
>> + "cpas_vfe1",
>> + "cpas_vfe_lite",
>> + "cphy_rx_clk_src",
>> + "csid",
>> + "csid_csiphy_rx",
>> + "csiphy0",
>> + "csiphy0_timer",
>> + "csiphy1",
>> + "csiphy1_timer",
>> + "csiphy2",
>> + "csiphy2_timer",
>> + "csiphy4",
>> + "csiphy4_timer",
>> + "gcc_axi_hf",
>> + "gcc_axi_sf",
>> + "vfe0",
>> + "vfe0_fast_ahb",
>> + "vfe1",
>> + "vfe1_fast_ahb",
>> + "vfe_lite",
>> + "vfe_lite_ahb",
>> + "vfe_lite_cphy_rx",
>> + "vfe_lite_csid";
>> +
>> + iommus = <&apps_smmu 0x800 0x60>,
>> + <&apps_smmu 0x820 0x60>,
>> + <&apps_smmu 0x840 0x60>,
>> + <&apps_smmu 0x860 0x60>,
>> + <&apps_smmu 0x1800 0x60>,
>> + <&apps_smmu 0x1820 0x60>,
>> + <&apps_smmu 0x1840 0x60>,
>> + <&apps_smmu 0x1860 0x60>,
>> + <&apps_smmu 0x18a0 0x00>,
>> + <&apps_smmu 0x18e0 0x00>,
>> + <&apps_smmu 0x1980 0x20>,
>> + <&apps_smmu 0x1900 0x00>,
>> + <&apps_smmu 0x19a0 0x20>;
>> +
>> + interconnects = <&gem_noc MASTER_APPSS_PROC 0 &config_noc
>> SLAVE_CAMERA_CFG 0>,
>> + <&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt
>> SLAVE_EBI1 0>,
>> + <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt
>> SLAVE_EBI1 0>,
>> + <&mmss_noc MASTER_CAMNOC_ICP 0 &mc_virt
>> SLAVE_EBI1 0>;
>> + interconnect-names = "cam_ahb",
>> + "cam_hf_mnoc",
>> + "cam_sf_mnoc",
>> + "cam_sf_icp_mnoc";
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>
> It's unclear why #address-cells/#size-cells are needed here.
Because the checker wants it. I'll check again to be sure.
>
>> +
>> + csiphy_ep0: endpoint {
>> + clock-lanes = <7>;
>
> As it's known, there is no lane 7.
Yes true this should be five. It works because this value is ignored by
the driver.
---
bod
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding
2024-11-19 15:11 ` Bryan O'Donoghue
@ 2024-11-20 23:02 ` Vladimir Zapolskiy
2024-11-20 23:27 ` Bryan O'Donoghue
0 siblings, 1 reply; 31+ messages in thread
From: Vladimir Zapolskiy @ 2024-11-20 23:02 UTC (permalink / raw)
To: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
Hi Bryan,
On 11/19/24 17:11, Bryan O'Donoghue wrote:
> On 19/11/2024 14:34, Vladimir Zapolskiy wrote:
>> Hi Bryan,
>>
>> please find a few review comments below.
>>
>> On 11/19/24 15:10, Bryan O'Donoghue wrote:
>>> Add bindings for qcom,x1e80100-camss in order to support the camera
>>> subsystem for x1e80100 as found in various Co-Pilot laptops.
>>>
>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> ---
>>> .../bindings/media/qcom,x1e80100-camss.yaml | 354 +++++++++++
>>> ++++++++++
>>> 1 file changed, 354 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-
>>> camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-
>>> camss.yaml
>>> new file mode 100644
>>> index
>>> 0000000000000000000000000000000000000000..ca2499cd52a51e14bad3cf8a8ca94c9d23ed5030
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
>>> @@ -0,0 +1,354 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/media/qcom,x1e80100-camss.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm X1E80100 Camera Subsystem (CAMSS)
>>> +
>>> +maintainers:
>>> + - Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> +
>>> +description: |
>>> + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: qcom,x1e80100-camss
>>> +
>>> + clocks:
>>> + maxItems: 29
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: camnoc_rt_axi
>>> + - const: camnoc_nrt_axi
>>> + - const: core_ahb
>>> + - const: cpas_ahb
>>> + - const: cpas_fast_ahb
>>> + - const: cpas_vfe0
>>> + - const: cpas_vfe1
>>> + - const: cpas_vfe_lite
>>> + - const: cphy_rx_clk_src
>>> + - const: csid
>>> + - const: csid_csiphy_rx
>>> + - const: csiphy0
>>> + - const: csiphy0_timer
>>> + - const: csiphy1
>>> + - const: csiphy1_timer
>>> + - const: csiphy2
>>> + - const: csiphy2_timer
>>> + - const: csiphy4
>>> + - const: csiphy4_timer
>>
>> What does happen to csiphy3? Could it fall through the cracks?
>>
>
> Nope.
>
> For whatever reason csiphy4 is the name here. I guess different SKUs
> have been fused out this way. I'd assume there's some version that does
> csiphy0-csiphy4 inclusive.
>
> Not here though.
>
>>> + - const: gcc_axi_hf
>>> + - const: gcc_axi_sf
>>> + - const: vfe0
>>> + - const: vfe0_fast_ahb
>>> + - const: vfe1
>>> + - const: vfe1_fast_ahb
>>> + - const: vfe_lite
>>> + - const: vfe_lite_ahb
>>> + - const: vfe_lite_cphy_rx
>>> + - const: vfe_lite_csid
>>> +
>>> + interrupts:
>>> + maxItems: 13
>>> +
>>> + interrupt-names:
>>> + items:
>>> + - const: csid0
>>> + - const: csid1
>>> + - const: csid2
>>> + - const: csid_lite0
>>> + - const: csid_lite1
>>> + - const: csiphy0
>>> + - const: csiphy1
>>> + - const: csiphy2
>>> + - const: csiphy4
>>> + - const: vfe0
>>> + - const: vfe1
>>> + - const: vfe_lite0
>>> + - const: vfe_lite1
>>> +
>>> + iommus:
>>> + maxItems: 13
>>> +
>>> + interconnects:
>>> + maxItems: 4
>>> +
>>> + interconnect-names:
>>> + items:
>>> + - const: cam_ahb
>>> + - const: cam_hf_mnoc
>>> + - const: cam_sf_mnoc
>>> + - const: cam_sf_icp_mnoc
>>> +
>>> + power-domains:
>>> + items:
>>> + - description: IFE0 GDSC - Image Front End, Global Distributed
>>> Switch Controller.
>>> + - description: IFE1 GDSC - Image Front End, Global Distributed
>>> Switch Controller.
>>> + - description: Titan Top GDSC - Titan ISP Block, Global
>>> Distributed Switch Controller.
>>> +
>>> + power-domain-names:
>>> + items:
>>> + - const: ife0
>>> + - const: ife1
>>> + - const: top
>>> +
>>> + ports:
>>> + $ref: /schemas/graph.yaml#/properties/ports
>>> +
>>> + description:
>>> + CSI input ports.
>>> +
>>> + patternProperties:
>>> + "^port@[03]+$":
>>> + $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:
>>> + clock-lanes:
>>> + maxItems: 1
>>> +
>>> + data-lanes:
>>> + minItems: 1
>>> + maxItems: 4
>>> +
>>> + required:
>>> + - clock-lanes
>>> + - data-lanes
>>> +
>>> + reg:
>>> + maxItems: 12
>>> +
>>> + reg-names:
>>> + items:
>>> + - const: csid0
>>> + - const: csid1
>>> + - const: csid2
>>> + - const: csid_wrapper
>>> + - const: csiphy0
>>> + - const: csiphy1
>>> + - const: csiphy2
>>> + - const: csiphy4
>>> + - const: vfe_lite0
>>> + - const: vfe_lite1
>>> + - const: vfe0
>>> + - const: vfe1
>>> +
>>> + vdda-phy-supply:
>>> + description:
>>> + Phandle to a 0.9V regulator supply to PHY core block.
>>> +
>>> + vdda-pll-supply:
>>> + description:
>>> + Phandle to 1.2V regulator supply to PHY refclk pll block.
>>
>> I believe it's very unlikely that the SoC pads are called like this,
>> as we discussed it in the recent past.
>>
>> Please rename the properties to reflect the names inherited from
>> the actual hardware.
>
> I believe we agreed to convert to the PHY infrastructure after 8550,
> 7280 and x1e80100.
>
> So these names should rename as is.
my ask is not related to the planned PHY conversion, it's much simpler and
easily doable, just reflect the proper pad names in the property names.
There is no such hardware objects on the SoC, which names can be associated
to "vdda-phy" or "vdda-pll" property names. Okay, split of CSIPHY specific
supplies can be done separately, but can you introduce here property names
like "vdd-csiphy-0p9-supply" and "vdd-csiphy-1p2-supply"?
Also you put a description like "supply to PHY refclk pll block", but if I
remember correctly once you've said that the datasheet (of another SoC)
does not give any clues about the usage of the supply, thus it invalidates
the given description.
I'm unhappy that people tend to copy defects, which are trivial to fix or
avoid at least.
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding
2024-11-20 23:02 ` Vladimir Zapolskiy
@ 2024-11-20 23:27 ` Bryan O'Donoghue
2024-11-21 0:37 ` Vladimir Zapolskiy
0 siblings, 1 reply; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-20 23:27 UTC (permalink / raw)
To: Vladimir Zapolskiy, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 20/11/2024 23:02, Vladimir Zapolskiy wrote:
> like "vdd-csiphy-0p9-supply" and "vdd-csiphy-1p2-supply"?
In theory, however I'd like to avoid adding endless strings of new names
into the driver code for each different power input.
We can add this additional string name though in the interim between now
and refactor for the PHY API.
> Also you put a description like "supply to PHY refclk pll block", but if I
> remember correctly once you've said that the datasheet (of another SoC)
> does not give any clues about the usage of the supply, thus it invalidates
> the given description.
I'm surmising by extrapolation - that's "probably" what those are just
at different voltage levels based on previous iterations of this PHY.
I'm just as happy not to describe this or to describe it as no mor that
the 1.2v supply etc.
---
bod
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding
2024-11-20 23:27 ` Bryan O'Donoghue
@ 2024-11-21 0:37 ` Vladimir Zapolskiy
2024-11-21 9:36 ` Bryan O'Donoghue
0 siblings, 1 reply; 31+ messages in thread
From: Vladimir Zapolskiy @ 2024-11-21 0:37 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
Robert Foss, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Todor Tomov, Mauro Carvalho Chehab, Bjorn Andersson,
Michael Turquette, Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 11/21/24 01:27, Bryan O'Donoghue wrote:
> On 20/11/2024 23:02, Vladimir Zapolskiy wrote:
>> like "vdd-csiphy-0p9-supply" and "vdd-csiphy-1p2-supply"?
>
> In theory, however I'd like to avoid adding endless strings of new names
> into the driver code for each different power input.
I don't understand this argument, it's the same degree of endlessness as
the endlessness of new designed SoCs. Should it be stopped now or what's
the point here?
My argument is to represent the actual hardware instead of copying errors.
> We can add this additional string name though in the interim between now
> and refactor for the PHY API.
I don't see it as a good reason to copy an easy to correct mistake.
>> Also you put a description like "supply to PHY refclk pll block", but if I
>> remember correctly once you've said that the datasheet (of another SoC)
>> does not give any clues about the usage of the supply, thus it invalidates
>> the given description.
>
> I'm surmising by extrapolation - that's "probably" what those are just
> at different voltage levels based on previous iterations of this PHY.
But this is proven to be wrong, let me kindly ask you to align with the SoC
documentation here.
> I'm just as happy not to describe this or to describe it as no mor that
> the 1.2v supply etc.
>
Thank you for understanding.
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding
2024-11-21 0:37 ` Vladimir Zapolskiy
@ 2024-11-21 9:36 ` Bryan O'Donoghue
0 siblings, 0 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-21 9:36 UTC (permalink / raw)
To: Vladimir Zapolskiy, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 21/11/2024 00:37, Vladimir Zapolskiy wrote:
> On 11/21/24 01:27, Bryan O'Donoghue wrote:
>> On 20/11/2024 23:02, Vladimir Zapolskiy wrote:
>>> like "vdd-csiphy-0p9-supply" and "vdd-csiphy-1p2-supply"?
>>
>> In theory, however I'd like to avoid adding endless strings of new names
>> into the driver code for each different power input.
>
> I don't understand this argument, it's the same degree of endlessness as
> the endlessness of new designed SoCs. Should it be stopped now or what's
> the point here?
I mean to say, I don't want to backdoor a bunch of optional regulators
with odd names via this method.
Per our previous disucssions/agreements we will do this in a real PHY
instead of inline in CAMSS.
---
bod
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding
2024-11-19 13:10 ` [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding Bryan O'Donoghue
2024-11-19 14:34 ` Vladimir Zapolskiy
@ 2024-11-20 8:55 ` Krzysztof Kozlowski
1 sibling, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-20 8:55 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On Tue, Nov 19, 2024 at 01:10:31PM +0000, Bryan O'Donoghue wrote:
A nit, subject: drop second/last, redundant "binding". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> + power-domains:
> + items:
> + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
> + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
> + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
> +
> + power-domain-names:
> + items:
> + - const: ife0
> + - const: ife1
> + - const: top
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + description:
> + CSI input ports.
> +
> + patternProperties:
> + "^port@[03]+$":
[0-3], no?
> + $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:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + required:
> + - clock-lanes
> + - data-lanes
> +
...
> + interconnects = <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_CAMERA_CFG 0>,
> + <&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt SLAVE_EBI1 0>,
> + <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI1 0>,
> + <&mmss_noc MASTER_CAMNOC_ICP 0 &mc_virt SLAVE_EBI1 0>;
> + interconnect-names = "cam_ahb",
> + "cam_hf_mnoc",
> + "cam_sf_mnoc",
> + "cam_sf_icp_mnoc";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
These are not needed, I think. Not sure if even correct...
> +
> + csiphy_ep0: endpoint {
> + clock-lanes = <7>;
> + data-lanes = <0 1>;
> + remote-endpoint = <&sensor_ep>;
> + };
> + };
> + };
> + };
> + };
>
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC
2024-11-19 13:10 [PATCH 0/6] Add dt-bindings and dtsi changes for CAMSS on x1e80100 silicon Bryan O'Donoghue
2024-11-19 13:10 ` [PATCH 1/6] dt-bindings: i2c: qcom-cci: Document x1e80100 compatible Bryan O'Donoghue
2024-11-19 13:10 ` [PATCH 2/6] dt-bindings: media: Add qcom,x1e80100-camss binding Bryan O'Donoghue
@ 2024-11-19 13:10 ` Bryan O'Donoghue
2024-11-19 14:47 ` Vladimir Zapolskiy
` (2 more replies)
2024-11-19 13:10 ` [PATCH 4/6] arm64: dts: qcom: x1e80100: Add CAMCC block definition Bryan O'Donoghue
` (2 subsequent siblings)
5 siblings, 3 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 13:10 UTC (permalink / raw)
To: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk, Bryan O'Donoghue
The x1e80100 has two power-domains for the CAMCC not one.
Capture this as:
minItems:1
maxItems:2
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
index 0766f66c7dc4f6b81afa01f156c490f4f742fcee..afb7e37118b691658fc5cc71e97b110dcee7f22a 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
@@ -39,9 +39,10 @@ properties:
- description: Sleep clock source
power-domains:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
description:
- A phandle and PM domain specifier for the MMCX power domain.
+ A phandle and PM domain specifier for the MMCX or MCX power domains.
required-opps:
maxItems: 1
--
2.45.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC
2024-11-19 13:10 ` [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC Bryan O'Donoghue
@ 2024-11-19 14:47 ` Vladimir Zapolskiy
2024-11-19 15:12 ` Bryan O'Donoghue
2024-11-20 8:51 ` Krzysztof Kozlowski
2024-11-20 11:53 ` Dmitry Baryshkov
2 siblings, 1 reply; 31+ messages in thread
From: Vladimir Zapolskiy @ 2024-11-19 14:47 UTC (permalink / raw)
To: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 11/19/24 15:10, Bryan O'Donoghue wrote:
> The x1e80100 has two power-domains for the CAMCC not one.
>
> Capture this as:
> minItems:1
> maxItems:2
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> index 0766f66c7dc4f6b81afa01f156c490f4f742fcee..afb7e37118b691658fc5cc71e97b110dcee7f22a 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> @@ -39,9 +39,10 @@ properties:
> - description: Sleep clock source
>
> power-domains:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> description:
> - A phandle and PM domain specifier for the MMCX power domain.
> + A phandle and PM domain specifier for the MMCX or MCX power domains.
It's a list of two phandles, not a one or another one. Can you please
>
> required-opps:
> maxItems: 1
>
Apart of the review comment above to be resolved,
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC
2024-11-19 14:47 ` Vladimir Zapolskiy
@ 2024-11-19 15:12 ` Bryan O'Donoghue
0 siblings, 0 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 15:12 UTC (permalink / raw)
To: Vladimir Zapolskiy, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 19/11/2024 14:47, Vladimir Zapolskiy wrote:
>> + maxItems: 2
>> description:
>> - A phandle and PM domain specifier for the MMCX power domain.
>> + A phandle and PM domain specifier for the MMCX or MCX power
>> domains.
>
> It's a list of two phandles, not a one or another one. Can you please
Its two for x1e80100 or one for sm8450.
I'll find a way to express this more clearly.
---
bod
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC
2024-11-19 13:10 ` [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC Bryan O'Donoghue
2024-11-19 14:47 ` Vladimir Zapolskiy
@ 2024-11-20 8:51 ` Krzysztof Kozlowski
2024-11-20 11:53 ` Dmitry Baryshkov
2 siblings, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-20 8:51 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On Tue, Nov 19, 2024 at 01:10:32PM +0000, Bryan O'Donoghue wrote:
> The x1e80100 has two power-domains for the CAMCC not one.
>
> Capture this as:
> minItems:1
> maxItems:2
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> index 0766f66c7dc4f6b81afa01f156c490f4f742fcee..afb7e37118b691658fc5cc71e97b110dcee7f22a 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> @@ -39,9 +39,10 @@ properties:
> - description: Sleep clock source
>
> power-domains:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> description:
> - A phandle and PM domain specifier for the MMCX power domain.
> + A phandle and PM domain specifier for the MMCX or MCX power domains.
>
Instead list the items with description and minItems
minItems: 1
items:
- description:
- description:
also add in allOf section if:then: constraining it for all variants
(maxItems: 1 and minItems: 2).
Optionally X1E could be moved to a new binding. I think this would be
better, but I do not insist.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC
2024-11-19 13:10 ` [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC Bryan O'Donoghue
2024-11-19 14:47 ` Vladimir Zapolskiy
2024-11-20 8:51 ` Krzysztof Kozlowski
@ 2024-11-20 11:53 ` Dmitry Baryshkov
2 siblings, 0 replies; 31+ messages in thread
From: Dmitry Baryshkov @ 2024-11-20 11:53 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On Tue, Nov 19, 2024 at 01:10:32PM +0000, Bryan O'Donoghue wrote:
> The x1e80100 has two power-domains for the CAMCC not one.
>
> Capture this as:
> minItems:1
> maxItems:2
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> index 0766f66c7dc4f6b81afa01f156c490f4f742fcee..afb7e37118b691658fc5cc71e97b110dcee7f22a 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> @@ -39,9 +39,10 @@ properties:
> - description: Sleep clock source
>
> power-domains:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> description:
> - A phandle and PM domain specifier for the MMCX power domain.
> + A phandle and PM domain specifier for the MMCX or MCX power domains.
Should there be an if, selecting which platforms need 1 domain and which
need both?
>
> required-opps:
> maxItems: 1
>
> --
> 2.45.2
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 4/6] arm64: dts: qcom: x1e80100: Add CAMCC block definition
2024-11-19 13:10 [PATCH 0/6] Add dt-bindings and dtsi changes for CAMSS on x1e80100 silicon Bryan O'Donoghue
` (2 preceding siblings ...)
2024-11-19 13:10 ` [PATCH 3/6] dt-bindings: clock: qcom: Add second power-domain to CAMCC Bryan O'Donoghue
@ 2024-11-19 13:10 ` Bryan O'Donoghue
2024-12-02 14:27 ` Konrad Dybcio
2024-11-19 13:10 ` [PATCH 5/6] arm64: dts: qcom: x1e80100: Add CCI definitions Bryan O'Donoghue
2024-11-19 13:10 ` [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition Bryan O'Donoghue
5 siblings, 1 reply; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 13:10 UTC (permalink / raw)
To: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk, Bryan O'Donoghue
Add the CAMCC block for x1e80100. The x1e80100 CAMCC block is an iteration
of previous CAMCC blocks with the exception of having two required
power-domains not just one.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index c18b99765c25c901b3d0a3fbaddc320c0a8c1716..5119cf64b461eb517e9306869ad0ec1b2cae629e 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -3,6 +3,7 @@
* Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
*/
+#include <dt-bindings/clock/qcom,x1e80100-camcc.h>
#include <dt-bindings/clock/qcom,rpmh.h>
#include <dt-bindings/clock/qcom,sc8280xp-lpasscc.h>
#include <dt-bindings/clock/qcom,x1e80100-dispcc.h>
@@ -4647,6 +4648,22 @@ usb_1_ss1_dwc3_ss: endpoint {
};
};
+ camcc: clock-controller@ade0000 {
+ compatible = "qcom,x1e80100-camcc";
+ reg = <0 0x0ade0000 0 0x20000>;
+ clocks = <&gcc GCC_CAMERA_AHB_CLK>,
+ <&bi_tcxo_div2>,
+ <&bi_tcxo_ao_div2>,
+ <&sleep_clk>;
+ power-domains = <&rpmhpd RPMHPD_MXC>,
+ <&rpmhpd RPMHPD_MMCX>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ #power-domain-cells = <1>;
+ status = "disabled";
+ };
+
mdss: display-subsystem@ae00000 {
compatible = "qcom,x1e80100-mdss";
reg = <0 0x0ae00000 0 0x1000>;
--
2.45.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 4/6] arm64: dts: qcom: x1e80100: Add CAMCC block definition
2024-11-19 13:10 ` [PATCH 4/6] arm64: dts: qcom: x1e80100: Add CAMCC block definition Bryan O'Donoghue
@ 2024-12-02 14:27 ` Konrad Dybcio
2024-12-02 15:02 ` Dmitry Baryshkov
0 siblings, 1 reply; 31+ messages in thread
From: Konrad Dybcio @ 2024-12-02 14:27 UTC (permalink / raw)
To: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 19.11.2024 2:10 PM, Bryan O'Donoghue wrote:
> Add the CAMCC block for x1e80100. The x1e80100 CAMCC block is an iteration
> of previous CAMCC blocks with the exception of having two required
> power-domains not just one.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index c18b99765c25c901b3d0a3fbaddc320c0a8c1716..5119cf64b461eb517e9306869ad0ec1b2cae629e 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -3,6 +3,7 @@
> * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> +#include <dt-bindings/clock/qcom,x1e80100-camcc.h>
> #include <dt-bindings/clock/qcom,rpmh.h>
> #include <dt-bindings/clock/qcom,sc8280xp-lpasscc.h>
> #include <dt-bindings/clock/qcom,x1e80100-dispcc.h>
> @@ -4647,6 +4648,22 @@ usb_1_ss1_dwc3_ss: endpoint {
> };
> };
>
> + camcc: clock-controller@ade0000 {
> + compatible = "qcom,x1e80100-camcc";
> + reg = <0 0x0ade0000 0 0x20000>;
> + clocks = <&gcc GCC_CAMERA_AHB_CLK>,
This clock is not registered with the CCF
Konrad
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 4/6] arm64: dts: qcom: x1e80100: Add CAMCC block definition
2024-12-02 14:27 ` Konrad Dybcio
@ 2024-12-02 15:02 ` Dmitry Baryshkov
2024-12-02 15:30 ` Bryan O'Donoghue
0 siblings, 1 reply; 31+ messages in thread
From: Dmitry Baryshkov @ 2024-12-02 15:02 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On Mon, Dec 02, 2024 at 03:27:11PM +0100, Konrad Dybcio wrote:
> On 19.11.2024 2:10 PM, Bryan O'Donoghue wrote:
> > Add the CAMCC block for x1e80100. The x1e80100 CAMCC block is an iteration
> > of previous CAMCC blocks with the exception of having two required
> > power-domains not just one.
> >
> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > ---
> > arch/arm64/boot/dts/qcom/x1e80100.dtsi | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > index c18b99765c25c901b3d0a3fbaddc320c0a8c1716..5119cf64b461eb517e9306869ad0ec1b2cae629e 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > @@ -3,6 +3,7 @@
> > * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> > */
> >
> > +#include <dt-bindings/clock/qcom,x1e80100-camcc.h>
> > #include <dt-bindings/clock/qcom,rpmh.h>
> > #include <dt-bindings/clock/qcom,sc8280xp-lpasscc.h>
> > #include <dt-bindings/clock/qcom,x1e80100-dispcc.h>
> > @@ -4647,6 +4648,22 @@ usb_1_ss1_dwc3_ss: endpoint {
> > };
> > };
> >
> > + camcc: clock-controller@ade0000 {
> > + compatible = "qcom,x1e80100-camcc";
> > + reg = <0 0x0ade0000 0 0x20000>;
> > + clocks = <&gcc GCC_CAMERA_AHB_CLK>,
>
> This clock is not registered with the CCF
Isn't that be going to be handled by the CCF on its own (like orphans,
etc)?
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 4/6] arm64: dts: qcom: x1e80100: Add CAMCC block definition
2024-12-02 15:02 ` Dmitry Baryshkov
@ 2024-12-02 15:30 ` Bryan O'Donoghue
2024-12-05 17:27 ` Konrad Dybcio
0 siblings, 1 reply; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-12-02 15:30 UTC (permalink / raw)
To: Dmitry Baryshkov, Konrad Dybcio
Cc: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 02/12/2024 15:02, Dmitry Baryshkov wrote:
>>> + clocks = <&gcc GCC_CAMERA_AHB_CLK>,
>> This clock is not registered with the CCF
> Isn't that be going to be handled by the CCF on its own (like orphans,
> etc)?
For refence this is always-on ATM.
drivers/clk/qcom/gcc-x1e80100.c: qcom_branch_set_clk_en(regmap,
0x26004); /* GCC_CAMERA_AHB_CLK */
---
bod
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 4/6] arm64: dts: qcom: x1e80100: Add CAMCC block definition
2024-12-02 15:30 ` Bryan O'Donoghue
@ 2024-12-05 17:27 ` Konrad Dybcio
0 siblings, 0 replies; 31+ messages in thread
From: Konrad Dybcio @ 2024-12-05 17:27 UTC (permalink / raw)
To: Bryan O'Donoghue, Dmitry Baryshkov, Konrad Dybcio
Cc: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 2.12.2024 4:30 PM, Bryan O'Donoghue wrote:
> On 02/12/2024 15:02, Dmitry Baryshkov wrote:
>>>> + clocks = <&gcc GCC_CAMERA_AHB_CLK>,
>>> This clock is not registered with the CCF
>> Isn't that be going to be handled by the CCF on its own (like orphans,
>> etc)?
>
> For refence this is always-on ATM.
>
> drivers/clk/qcom/gcc-x1e80100.c: qcom_branch_set_clk_en(regmap, 0x26004); /* GCC_CAMERA_AHB_CLK */
Okay let's keep it asis and call this a Linux impl detail
Konrad
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 5/6] arm64: dts: qcom: x1e80100: Add CCI definitions
2024-11-19 13:10 [PATCH 0/6] Add dt-bindings and dtsi changes for CAMSS on x1e80100 silicon Bryan O'Donoghue
` (3 preceding siblings ...)
2024-11-19 13:10 ` [PATCH 4/6] arm64: dts: qcom: x1e80100: Add CAMCC block definition Bryan O'Donoghue
@ 2024-11-19 13:10 ` Bryan O'Donoghue
2024-11-19 14:42 ` Vladimir Zapolskiy
2024-12-07 11:59 ` Konrad Dybcio
2024-11-19 13:10 ` [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition Bryan O'Donoghue
5 siblings, 2 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 13:10 UTC (permalink / raw)
To: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk, Bryan O'Donoghue
Add in 2 CCI busses. One bus has two CCI bus master pinouts:
cci_i2c_scl0 = gpio101
cci_i2c_sda0 = gpio102
cci_i2c_scl1 = gpio103
cci_i2c_sda1 = gpio104
A second bus has a single CCI bus master pinout:
cci_i2c_scl2 = gpio105
cci_i2c_sda2 = gpio106
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 162 +++++++++++++++++++++++++++++++++
1 file changed, 162 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 5119cf64b461eb517e9306869ad0ec1b2cae629e..c19754fdc7e0fa4f674ce19f813db77fe2615cf3 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -4648,6 +4648,88 @@ usb_1_ss1_dwc3_ss: endpoint {
};
};
+ cci0: cci@ac15000 {
+ compatible = "qcom,x1e80100-cci", "qcom,msm8996-cci";
+ reg = <0 0x0ac15000 0 0x1000>;
+
+ interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
+
+ clocks = <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
+ <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_CCI_0_CLK>;
+ clock-names = "camnoc_axi",
+ "slow_ahb_src",
+ "cpas_ahb",
+ "cci";
+
+ power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>;
+
+ pinctrl-0 = <&cci0_default>;
+ pinctrl-1 = <&cci0_sleep>;
+ pinctrl-names = "default", "sleep";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+
+ cci0_i2c0: i2c-bus@0 {
+ reg = <0>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ cci0_i2c1: i2c-bus@1 {
+ reg = <1>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ cci1: cci@ac16000 {
+ compatible = "qcom,x1e80100-cci", "qcom,msm8996-cci";
+ reg = <0 0x0ac16000 0 0x1000>;
+
+ interrupts = <GIC_SPI 271 IRQ_TYPE_EDGE_RISING>;
+
+ clocks = <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
+ <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_CCI_1_CLK>;
+ clock-names = "camnoc_axi",
+ "slow_ahb_src",
+ "cpas_ahb",
+ "cci";
+
+ power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>;
+
+ pinctrl-0 = <&cci1_default>;
+ pinctrl-1 = <&cci1_sleep>;
+ pinctrl-names = "default", "sleep";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+
+ cci1_i2c0: i2c-bus@0 {
+ reg = <0>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ cci1_i2c1: i2c-bus@1 {
+ reg = <1>;
+ clock-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
camcc: clock-controller@ade0000 {
compatible = "qcom,x1e80100-camcc";
reg = <0 0x0ade0000 0 0x20000>;
@@ -5272,6 +5354,86 @@ tlmm: pinctrl@f100000 {
gpio-ranges = <&tlmm 0 0 239>;
wakeup-parent = <&pdc>;
+ cci0_default: cci0-default-state {
+ cci0_i2c0_default: cci0-i2c0-default-pins {
+ /* cci_i2c_sda0, cci_i2c_scl0 */
+ pins = "gpio101", "gpio102";
+ function = "cci_i2c";
+
+ bias-pull-up;
+ drive-strength = <2>; /* 2 mA */
+ };
+
+ cci0_i2c1_default: cci0-i2c1-default-pins {
+ /* cci_i2c_sda1, cci_i2c_scl1 */
+ pins = "gpio103", "gpio104";
+ function = "cci_i2c";
+
+ bias-pull-up;
+ drive-strength = <2>; /* 2 mA */
+ };
+ };
+
+ cci0_sleep: cci0-sleep-state {
+ cci0_i2c0_sleep: cci0-i2c0-sleep-pins {
+ /* cci_i2c_sda0, cci_i2c_scl0 */
+ pins = "gpio101", "gpio102";
+ function = "cci_i2c";
+
+ drive-strength = <2>; /* 2 mA */
+ bias-pull-down;
+ };
+
+ cci0_i2c1_sleep: cci0-i2c1-sleep-pins {
+ /* cci_i2c_sda1, cci_i2c_scl1 */
+ pins = "gpio103", "gpio104";
+ function = "cci_i2c";
+
+ drive-strength = <2>; /* 2 mA */
+ bias-pull-down;
+ };
+ };
+
+ cci1_default: cci1-default-state {
+ cci1_i2c0_default: cci1-i2c0-default-pins {
+ /* cci_i2c_sda2, cci_i2c_scl2 */
+ pins = "gpio105","gpio106";
+ function = "cci_i2c";
+
+ bias-pull-up;
+ drive-strength = <2>; /* 2 mA */
+ };
+
+ cci1_i2c1_default: cci1-i2c1-default-pins {
+ /* aon_cci_i2c_sda3, aon_cci_i2c_scl3 */
+ pins = "gpio235","gpio236";
+ function = "aon_cci";
+
+ bias-pull-up;
+ drive-strength = <2>; /* 2 mA */
+ };
+ };
+
+ cci1_sleep: cci1-sleep-state {
+ cci1_i2c0_sleep: cci1-i2c0-sleep-pins {
+ /* cci_i2c_sda2, cci_i2c_scl2 */
+ pins = "gpio105","gpio106";
+ function = "cci_i2c";
+
+ bias-pull-down;
+ drive-strength = <2>; /* 2 mA */
+ };
+
+ cci1_i2c1_sleep: cci1-i2c1-sleep-pins {
+ /* aon_cci_i2c_sda3, aon_cci_i2c_scl3 */
+ pins = "gpio235","gpio236";
+ function = "aon_cci";
+
+ bias-pull-down;
+ drive-strength = <2>; /* 2 mA */
+ };
+ };
+
qup_i2c0_data_clk: qup-i2c0-data-clk-state {
/* SDA, SCL */
pins = "gpio0", "gpio1";
--
2.45.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 5/6] arm64: dts: qcom: x1e80100: Add CCI definitions
2024-11-19 13:10 ` [PATCH 5/6] arm64: dts: qcom: x1e80100: Add CCI definitions Bryan O'Donoghue
@ 2024-11-19 14:42 ` Vladimir Zapolskiy
2024-12-07 11:59 ` Konrad Dybcio
1 sibling, 0 replies; 31+ messages in thread
From: Vladimir Zapolskiy @ 2024-11-19 14:42 UTC (permalink / raw)
To: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
Hi Bryan,
On 11/19/24 15:10, Bryan O'Donoghue wrote:
> Add in 2 CCI busses. One bus has two CCI bus master pinouts:
> cci_i2c_scl0 = gpio101
> cci_i2c_sda0 = gpio102
> cci_i2c_scl1 = gpio103
> cci_i2c_sda1 = gpio104
>
> A second bus has a single CCI bus master pinout:
> cci_i2c_scl2 = gpio105
> cci_i2c_sda2 = gpio106
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 162 +++++++++++++++++++++++++++++++++
> 1 file changed, 162 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 5119cf64b461eb517e9306869ad0ec1b2cae629e..c19754fdc7e0fa4f674ce19f813db77fe2615cf3 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -4648,6 +4648,88 @@ usb_1_ss1_dwc3_ss: endpoint {
> };
> };
>
> + cci0: cci@ac15000 {
> + compatible = "qcom,x1e80100-cci", "qcom,msm8996-cci";
> + reg = <0 0x0ac15000 0 0x1000>;
> +
> + interrupts = <GIC_SPI 460 IRQ_TYPE_EDGE_RISING>;
> +
> + clocks = <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
> + <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
> + <&camcc CAM_CC_CPAS_AHB_CLK>,
> + <&camcc CAM_CC_CCI_0_CLK>;
> + clock-names = "camnoc_axi",
> + "slow_ahb_src",
> + "cpas_ahb",
> + "cci";
cpas_ahb clock is a child of slow_ahb_src clock, please follow the
newly introduced scheme, and exclude slow_ahb_src clock from the list.
> +
> + power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>;
> +
> + pinctrl-0 = <&cci0_default>;
> + pinctrl-1 = <&cci0_sleep>;
> + pinctrl-names = "default", "sleep";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + status = "disabled";
> +
> + cci0_i2c0: i2c-bus@0 {
> + reg = <0>;
> + clock-frequency = <1000000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + cci0_i2c1: i2c-bus@1 {
> + reg = <1>;
> + clock-frequency = <1000000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 5/6] arm64: dts: qcom: x1e80100: Add CCI definitions
2024-11-19 13:10 ` [PATCH 5/6] arm64: dts: qcom: x1e80100: Add CCI definitions Bryan O'Donoghue
2024-11-19 14:42 ` Vladimir Zapolskiy
@ 2024-12-07 11:59 ` Konrad Dybcio
2024-12-07 12:54 ` Bryan O'Donoghue
1 sibling, 1 reply; 31+ messages in thread
From: Konrad Dybcio @ 2024-12-07 11:59 UTC (permalink / raw)
To: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 19.11.2024 2:10 PM, Bryan O'Donoghue wrote:
> Add in 2 CCI busses. One bus has two CCI bus master pinouts:
> cci_i2c_scl0 = gpio101
> cci_i2c_sda0 = gpio102
> cci_i2c_scl1 = gpio103
> cci_i2c_sda1 = gpio104
>
> A second bus has a single CCI bus master pinout:
> cci_i2c_scl2 = gpio105
> cci_i2c_sda2 = gpio106
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 162 +++++++++++++++++++++++++++++++++
> 1 file changed, 162 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 5119cf64b461eb517e9306869ad0ec1b2cae629e..c19754fdc7e0fa4f674ce19f813db77fe2615cf3 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -4648,6 +4648,88 @@ usb_1_ss1_dwc3_ss: endpoint {
> };
> };
>
> + cci0: cci@ac15000 {
[...]
> + cci0_default: cci0-default-state {
> + cci0_i2c0_default: cci0-i2c0-default-pins {
> + /* cci_i2c_sda0, cci_i2c_scl0 */
> + pins = "gpio101", "gpio102";
> + function = "cci_i2c";
> +
> + bias-pull-up;
> + drive-strength = <2>; /* 2 mA */
> + };
Please match the style of other nodes (flip drive-strength and bias, remove
the newline and remove the mA comment)
Otherwise looks good and I can attest to this working, as the sensor on the
SL7 happily talks back
Konrad
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 5/6] arm64: dts: qcom: x1e80100: Add CCI definitions
2024-12-07 11:59 ` Konrad Dybcio
@ 2024-12-07 12:54 ` Bryan O'Donoghue
0 siblings, 0 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-12-07 12:54 UTC (permalink / raw)
To: Konrad Dybcio, Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 07/12/2024 11:59, Konrad Dybcio wrote:
> Otherwise looks good and I can attest to this working, as the sensor on the
> SL7 happily talks back
:x
---
bod
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition
2024-11-19 13:10 [PATCH 0/6] Add dt-bindings and dtsi changes for CAMSS on x1e80100 silicon Bryan O'Donoghue
` (4 preceding siblings ...)
2024-11-19 13:10 ` [PATCH 5/6] arm64: dts: qcom: x1e80100: Add CCI definitions Bryan O'Donoghue
@ 2024-11-19 13:10 ` Bryan O'Donoghue
2024-11-19 14:44 ` Vladimir Zapolskiy
` (2 more replies)
5 siblings, 3 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 13:10 UTC (permalink / raw)
To: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk, Bryan O'Donoghue
Add dtsi to describe the xe180100 CAMSS block
4 x CSIPHY
2 x CSID
2 x CSID Lite
2 x IFE
2 x IFE Lite
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 180 +++++++++++++++++++++++++++++++++
1 file changed, 180 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index c19754fdc7e0fa4f674ce19f813db77fe2615cf3..f23352493cb270c0fdc3c42add032286601db1e9 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -4730,6 +4730,186 @@ cci1_i2c1: i2c-bus@1 {
};
};
+ camss: camss@ac62000 {
+ compatible = "qcom,x1e80100-camss";
+
+ reg = <0 0x0acb7000 0 0x2000>,
+ <0 0x0acb9000 0 0x2000>,
+ <0 0x0acbb000 0 0x2000>,
+ <0 0x0acb6000 0 0x1000>,
+ <0 0x0ace4000 0 0x1000>,
+ <0 0x0ace6000 0 0x1000>,
+ <0 0x0ace8000 0 0x1000>,
+ <0 0x0acec000 0 0x4000>,
+ <0 0x0acc7000 0 0x2000>,
+ <0 0x0accb000 0 0x2000>,
+ <0 0x0ac62000 0 0x2a00>,
+ <0 0x0ac71000 0 0x2a00>;
+
+ reg-names = "csid0",
+ "csid1",
+ "csid2",
+ "csid_wrapper",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy4",
+ "vfe_lite0",
+ "vfe_lite1",
+ "vfe0",
+ "vfe1";
+
+ interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 359 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 122 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 = "csid0",
+ "csid1",
+ "csid2",
+ "csid_lite0",
+ "csid_lite1",
+ "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy4",
+ "vfe0",
+ "vfe1",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ power-domains = <&camcc CAM_CC_IFE_0_GDSC>,
+ <&camcc CAM_CC_IFE_1_GDSC>,
+ <&camcc CAM_CC_TITAN_TOP_GDSC>;
+
+ power-domain-names = "ife0",
+ "ife1",
+ "top";
+
+ clocks = <&camcc CAM_CC_CAMNOC_AXI_RT_CLK>,
+ <&camcc CAM_CC_CAMNOC_AXI_NRT_CLK>,
+ <&camcc CAM_CC_CORE_AHB_CLK>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_CPAS_FAST_AHB_CLK>,
+ <&camcc CAM_CC_CPAS_IFE_0_CLK>,
+ <&camcc CAM_CC_CPAS_IFE_1_CLK>,
+ <&camcc CAM_CC_CPAS_IFE_LITE_CLK>,
+ <&camcc CAM_CC_CPHY_RX_CLK_SRC>,
+ <&camcc CAM_CC_CSID_CLK>,
+ <&camcc CAM_CC_CSID_CSIPHY_RX_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_CSIPHY4_CLK>,
+ <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
+ <&gcc GCC_CAMERA_HF_AXI_CLK>,
+ <&gcc GCC_CAMERA_SF_AXI_CLK>,
+ <&camcc CAM_CC_IFE_0_CLK>,
+ <&camcc CAM_CC_IFE_0_FAST_AHB_CLK>,
+ <&camcc CAM_CC_IFE_1_CLK>,
+ <&camcc CAM_CC_IFE_1_FAST_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
+
+ clock-names = "camnoc_rt_axi",
+ "camnoc_nrt_axi",
+ "core_ahb",
+ "cpas_ahb",
+ "cpas_fast_ahb",
+ "cpas_vfe0",
+ "cpas_vfe1",
+ "cpas_vfe_lite",
+ "cphy_rx_clk_src",
+ "csid",
+ "csid_csiphy_rx",
+ "csiphy0",
+ "csiphy0_timer",
+ "csiphy1",
+ "csiphy1_timer",
+ "csiphy2",
+ "csiphy2_timer",
+ "csiphy4",
+ "csiphy4_timer",
+ "gcc_axi_hf",
+ "gcc_axi_sf",
+ "vfe0",
+ "vfe0_fast_ahb",
+ "vfe1",
+ "vfe1_fast_ahb",
+ "vfe_lite",
+ "vfe_lite_ahb",
+ "vfe_lite_cphy_rx",
+ "vfe_lite_csid";
+
+ iommus = <&apps_smmu 0x800 0x60>,
+ <&apps_smmu 0x820 0x60>,
+ <&apps_smmu 0x840 0x60>,
+ <&apps_smmu 0x860 0x60>,
+ <&apps_smmu 0x1800 0x60>,
+ <&apps_smmu 0x1820 0x60>,
+ <&apps_smmu 0x1840 0x60>,
+ <&apps_smmu 0x1860 0x60>,
+ <&apps_smmu 0x18a0 0x00>,
+ <&apps_smmu 0x18e0 0x00>,
+ <&apps_smmu 0x1900 0x00>,
+ <&apps_smmu 0x1980 0x20>,
+ <&apps_smmu 0x19a0 0x20>;
+
+ interconnects = <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_CAMERA_CFG 0>,
+ <&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt SLAVE_EBI1 0>,
+ <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI1 0>,
+ <&mmss_noc MASTER_CAMNOC_ICP 0 &mc_virt SLAVE_EBI1 0>;
+ interconnect-names = "cam_ahb",
+ "cam_hf_mnoc",
+ "cam_sf_mnoc",
+ "cam_sf_icp_mnoc";
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ port@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ port@2 {
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ port@3 {
+ reg = <3>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+ };
+
camcc: clock-controller@ade0000 {
compatible = "qcom,x1e80100-camcc";
reg = <0 0x0ade0000 0 0x20000>;
--
2.45.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition
2024-11-19 13:10 ` [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition Bryan O'Donoghue
@ 2024-11-19 14:44 ` Vladimir Zapolskiy
2024-11-19 15:14 ` Bryan O'Donoghue
2024-11-20 8:52 ` Krzysztof Kozlowski
2024-12-07 12:24 ` Konrad Dybcio
2 siblings, 1 reply; 31+ messages in thread
From: Vladimir Zapolskiy @ 2024-11-19 14:44 UTC (permalink / raw)
To: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
Hi Bryan,
On 11/19/24 15:10, Bryan O'Donoghue wrote:
> Add dtsi to describe the xe180100 CAMSS block
>
> 4 x CSIPHY
> 2 x CSID
> 2 x CSID Lite
> 2 x IFE
> 2 x IFE Lite
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 180 +++++++++++++++++++++++++++++++++
> 1 file changed, 180 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index c19754fdc7e0fa4f674ce19f813db77fe2615cf3..f23352493cb270c0fdc3c42add032286601db1e9 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -4730,6 +4730,186 @@ cci1_i2c1: i2c-bus@1 {
> };
> };
>
> + camss: camss@ac62000 {
> + compatible = "qcom,x1e80100-camss";
> +
> + reg = <0 0x0acb7000 0 0x2000>,
> + <0 0x0acb9000 0 0x2000>,
> + <0 0x0acbb000 0 0x2000>,
> + <0 0x0acb6000 0 0x1000>,
> + <0 0x0ace4000 0 0x1000>,
> + <0 0x0ace6000 0 0x1000>,
> + <0 0x0ace8000 0 0x1000>,
> + <0 0x0acec000 0 0x4000>,
> + <0 0x0acc7000 0 0x2000>,
> + <0 0x0accb000 0 0x2000>,
> + <0 0x0ac62000 0 0x2a00>,
> + <0 0x0ac71000 0 0x2a00>;
> +
> + reg-names = "csid0",
> + "csid1",
> + "csid2",
> + "csid_wrapper",
> + "csiphy0",
> + "csiphy1",
> + "csiphy2",
> + "csiphy4",
> + "vfe_lite0",
> + "vfe_lite1",
> + "vfe0",
> + "vfe1";
> +
> + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
I've forgot to mention that you need to correct the interrupt type
to rising edge, that's been disucssed.
> + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 359 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 122 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>;
> +
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition
2024-11-19 14:44 ` Vladimir Zapolskiy
@ 2024-11-19 15:14 ` Bryan O'Donoghue
0 siblings, 0 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-11-19 15:14 UTC (permalink / raw)
To: Vladimir Zapolskiy, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 19/11/2024 14:44, Vladimir Zapolskiy wrote:
>> + interrupts = <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
>
> I've forgot to mention that you need to correct the interrupt type
> to rising edge, that's been disucssed.
Ah yes, I forgot about that.
Thanks
---
bod
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition
2024-11-19 13:10 ` [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition Bryan O'Donoghue
2024-11-19 14:44 ` Vladimir Zapolskiy
@ 2024-11-20 8:52 ` Krzysztof Kozlowski
2024-12-07 12:24 ` Konrad Dybcio
2 siblings, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-20 8:52 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Loic Poulain, Robert Foss, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio,
linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On Tue, Nov 19, 2024 at 01:10:35PM +0000, Bryan O'Donoghue wrote:
> Add dtsi to describe the xe180100 CAMSS block
>
> 4 x CSIPHY
> 2 x CSID
> 2 x CSID Lite
> 2 x IFE
> 2 x IFE Lite
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 180 +++++++++++++++++++++++++++++++++
> 1 file changed, 180 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index c19754fdc7e0fa4f674ce19f813db77fe2615cf3..f23352493cb270c0fdc3c42add032286601db1e9 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -4730,6 +4730,186 @@ cci1_i2c1: i2c-bus@1 {
> };
> };
>
> + camss: camss@ac62000 {
> + compatible = "qcom,x1e80100-camss";
> +
> + reg = <0 0x0acb7000 0 0x2000>,
It does not look like you tested the DTS against bindings. Please run
'make dtbs_check W=1' (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition
2024-11-19 13:10 ` [PATCH 6/6] arm64: dts: qcom: x1e80100: Add CAMSS block definition Bryan O'Donoghue
2024-11-19 14:44 ` Vladimir Zapolskiy
2024-11-20 8:52 ` Krzysztof Kozlowski
@ 2024-12-07 12:24 ` Konrad Dybcio
2 siblings, 0 replies; 31+ messages in thread
From: Konrad Dybcio @ 2024-12-07 12:24 UTC (permalink / raw)
To: Bryan O'Donoghue, Loic Poulain, Robert Foss, Andi Shyti,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Todor Tomov,
Mauro Carvalho Chehab, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Vladimir Zapolskiy, Jagadeesh Kona, Konrad Dybcio
Cc: linux-i2c, linux-arm-msm, devicetree, linux-kernel, linux-media,
linux-clk
On 19.11.2024 2:10 PM, Bryan O'Donoghue wrote:
> Add dtsi to describe the xe180100 CAMSS block
>
> 4 x CSIPHY
> 2 x CSID
> 2 x CSID Lite
> 2 x IFE
> 2 x IFE Lite
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 180 +++++++++++++++++++++++++++++++++
> 1 file changed, 180 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index c19754fdc7e0fa4f674ce19f813db77fe2615cf3..f23352493cb270c0fdc3c42add032286601db1e9 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -4730,6 +4730,186 @@ cci1_i2c1: i2c-bus@1 {
> };
> };
>
> + camss: camss@ac62000 {
> + compatible = "qcom,x1e80100-camss";
> +
> + reg = <0 0x0acb7000 0 0x2000>,
> + <0 0x0acb9000 0 0x2000>,
> + <0 0x0acbb000 0 0x2000>,
> + <0 0x0acb6000 0 0x1000>,
> + <0 0x0ace4000 0 0x1000>,
> + <0 0x0ace6000 0 0x1000>,
> + <0 0x0ace8000 0 0x1000>,
> + <0 0x0acec000 0 0x4000>,
> + <0 0x0acc7000 0 0x2000>,
> + <0 0x0accb000 0 0x2000>,
> + <0 0x0ac62000 0 0x2a00>,
> + <0 0x0ac71000 0 0x2a00>;
> +
> + reg-names = "csid0",
Please remove the blank lines between x and x-names
> + "csid1",
> + "csid2",
> + "csid_wrapper",
This doesn't seem to match what the commit msg promises
[...]
> +
> + iommus = <&apps_smmu 0x800 0x60>,
> + <&apps_smmu 0x820 0x60>,
> + <&apps_smmu 0x840 0x60>,
> + <&apps_smmu 0x860 0x60>,
> + <&apps_smmu 0x1800 0x60>,
> + <&apps_smmu 0x1820 0x60>,
> + <&apps_smmu 0x1840 0x60>,
> + <&apps_smmu 0x1860 0x60>,
> + <&apps_smmu 0x18a0 0x00>,
> + <&apps_smmu 0x18e0 0x00>,
> + <&apps_smmu 0x1900 0x00>,
> + <&apps_smmu 0x1980 0x20>,
> + <&apps_smmu 0x19a0 0x20>;
>>> for pair in a:
... print(hex(pair[0] & ~pair[1]))
...
0x800
0x800
0x800
0x800
0x1800
0x1800
0x1800
0x1800
0x18a0
0x18e0
0x1900
0x1980
0x1980
Please remove the duplicates
> +
> + interconnects = <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_CAMERA_CFG 0>,
QCOM_ICC_TAG_ACTIVE_ONLY
> + <&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt SLAVE_EBI1 0>,
> + <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI1 0>,
> + <&mmss_noc MASTER_CAMNOC_ICP 0 &mc_virt SLAVE_EBI1 0>;
QCOM_ICC_TAG_ALWAYS
Konrad
^ permalink raw reply [flat|nested] 31+ messages in thread