* [PATCH v3 01/19] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding
[not found] <20211222003751.2461466-1-bryan.odonoghue@linaro.org>
@ 2021-12-22 0:37 ` Bryan O'Donoghue
2022-01-04 13:42 ` Vladimir Zapolskiy
0 siblings, 1 reply; 5+ messages in thread
From: Bryan O'Donoghue @ 2021-12-22 0:37 UTC (permalink / raw)
To: linux-arm-msm, linux-media, mchehab, hverkuil, robert.foss
Cc: jonathan, andrey.konovalov, todor.too, agross, bjorn.andersson,
jgrahsl, hfink, vladimir.zapolskiy, dmitry.baryshkov,
bryan.odonoghue, devicetree, Rob Herring
From: Jonathan Marek <jonathan@marek.ca>
Add bindings for qcom,sm8250-camss in order to support the camera
subsystem for SM8250.
Cc: devicetree@vger.kernel.org
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/media/qcom,sm8250-camss.yaml | 450 ++++++++++++++++++
1 file changed, 450 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
new file mode 100644
index 0000000000000..af877d61b607d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
@@ -0,0 +1,450 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/media/qcom,sm8250-camss.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm CAMSS ISP
+
+maintainers:
+ - Robert Foss <robert.foss@linaro.org>
+
+description: |
+ The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
+
+properties:
+ compatible:
+ const: qcom,sm8250-camss
+
+ clocks:
+ minItems: 37
+ maxItems: 37
+
+ clock-names:
+ items:
+ - const: cam_ahb_clk
+ - const: cam_hf_axi
+ - const: cam_sf_axi
+ - const: camnoc_axi
+ - const: camnoc_axi_src
+ - const: core_ahb
+ - const: cpas_ahb
+ - const: csiphy0
+ - const: csiphy0_timer
+ - const: csiphy1
+ - const: csiphy1_timer
+ - const: csiphy2
+ - const: csiphy2_timer
+ - const: csiphy3
+ - const: csiphy3_timer
+ - const: csiphy4
+ - const: csiphy4_timer
+ - const: csiphy5
+ - const: csiphy5_timer
+ - const: slow_ahb_src
+ - const: vfe0_ahb
+ - const: vfe0_axi
+ - const: vfe0
+ - const: vfe0_cphy_rx
+ - const: vfe0_csid
+ - const: vfe0_areg
+ - const: vfe1_ahb
+ - const: vfe1_axi
+ - const: vfe1
+ - const: vfe1_cphy_rx
+ - const: vfe1_csid
+ - const: vfe1_areg
+ - const: vfe_lite_ahb
+ - const: vfe_lite_axi
+ - const: vfe_lite
+ - const: vfe_lite_cphy_rx
+ - const: vfe_lite_csid
+
+ interrupts:
+ minItems: 14
+ maxItems: 14
+
+ interrupt-names:
+ items:
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: csiphy3
+ - const: csiphy4
+ - const: csiphy5
+ - const: csid0
+ - const: csid1
+ - const: csid2
+ - const: csid3
+ - const: vfe0
+ - const: vfe1
+ - const: vfe_lite0
+ - const: vfe_lite1
+
+ iommus:
+ minItems: 8
+ maxItems: 8
+
+ interconnects:
+ minItems: 4
+ maxItems: 4
+
+ interconnect-names:
+ items:
+ - const: cam_ahb
+ - const: cam_hf_0_mnoc
+ - const: cam_sf_0_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 GDSC - Titan ISP Block, Global Distributed Switch Controller.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ description:
+ CSI input ports.
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+ port@1:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+ port@2:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+ port@3:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+ port@4:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - clock-lanes
+ - data-lanes
+
+ port@5:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port for receiving CSI data.
+
+ 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:
+ minItems: 10
+ maxItems: 10
+
+ reg-names:
+ items:
+ - const: csiphy0
+ - const: csiphy1
+ - const: csiphy2
+ - const: csiphy3
+ - const: csiphy4
+ - const: csiphy5
+ - const: vfe0
+ - const: vfe1
+ - const: vfe_lite0
+ - const: vfe_lite1
+
+required:
+ - clock-names
+ - clocks
+ - compatible
+ - interconnects
+ - interconnect-names
+ - interrupts
+ - interrupt-names
+ - iommus
+ - power-domains
+ - reg
+ - reg-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/qcom,camcc-sm8250.h>
+ #include <dt-bindings/interconnect/qcom,sm8250.h>
+ #include <dt-bindings/clock/qcom,gcc-sm8250.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ camss: camss@ac6a000 {
+ compatible = "qcom,sm8250-camss";
+
+ reg = <0 0xac6a000 0 0x2000>,
+ <0 0xac6c000 0 0x2000>,
+ <0 0xac6e000 0 0x1000>,
+ <0 0xac70000 0 0x1000>,
+ <0 0xac72000 0 0x1000>,
+ <0 0xac74000 0 0x1000>,
+ <0 0xacb4000 0 0xd000>,
+ <0 0xacc3000 0 0xd000>,
+ <0 0xacd9000 0 0x2200>,
+ <0 0xacdb200 0 0x2200>;
+ reg-names = "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "vfe0",
+ "vfe1",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ interrupts = <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "csiphy0",
+ "csiphy1",
+ "csiphy2",
+ "csiphy3",
+ "csiphy4",
+ "csiphy5",
+ "csid0",
+ "csid1",
+ "csid2",
+ "csid3",
+ "vfe0",
+ "vfe1",
+ "vfe_lite0",
+ "vfe_lite1";
+
+ power-domains = <&camcc IFE_0_GDSC>,
+ <&camcc IFE_1_GDSC>,
+ <&camcc TITAN_TOP_GDSC>;
+
+ clocks = <&gcc GCC_CAMERA_AHB_CLK>,
+ <&gcc GCC_CAMERA_HF_AXI_CLK>,
+ <&gcc GCC_CAMERA_SF_AXI_CLK>,
+ <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+ <&camcc CAM_CC_CAMNOC_AXI_CLK_SRC>,
+ <&camcc CAM_CC_CORE_AHB_CLK>,
+ <&camcc CAM_CC_CPAS_AHB_CLK>,
+ <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY1_CLK>,
+ <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY2_CLK>,
+ <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY3_CLK>,
+ <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY4_CLK>,
+ <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
+ <&camcc CAM_CC_CSIPHY5_CLK>,
+ <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
+ <&camcc CAM_CC_SLOW_AHB_CLK_SRC>,
+ <&camcc CAM_CC_IFE_0_AHB_CLK>,
+ <&camcc CAM_CC_IFE_0_AXI_CLK>,
+ <&camcc CAM_CC_IFE_0_CLK>,
+ <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_0_CSID_CLK>,
+ <&camcc CAM_CC_IFE_0_AREG_CLK>,
+ <&camcc CAM_CC_IFE_1_AHB_CLK>,
+ <&camcc CAM_CC_IFE_1_AXI_CLK>,
+ <&camcc CAM_CC_IFE_1_CLK>,
+ <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_1_CSID_CLK>,
+ <&camcc CAM_CC_IFE_1_AREG_CLK>,
+ <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+ <&camcc CAM_CC_IFE_LITE_AXI_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+ <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
+ clock-names = "cam_ahb_clk",
+ "cam_hf_axi",
+ "cam_sf_axi",
+ "camnoc_axi",
+ "camnoc_axi_src",
+ "core_ahb",
+ "cpas_ahb",
+ "csiphy0",
+ "csiphy0_timer",
+ "csiphy1",
+ "csiphy1_timer",
+ "csiphy2",
+ "csiphy2_timer",
+ "csiphy3",
+ "csiphy3_timer",
+ "csiphy4",
+ "csiphy4_timer",
+ "csiphy5",
+ "csiphy5_timer",
+ "slow_ahb_src",
+ "vfe0_ahb",
+ "vfe0_axi",
+ "vfe0",
+ "vfe0_cphy_rx",
+ "vfe0_csid",
+ "vfe0_areg",
+ "vfe1_ahb",
+ "vfe1_axi",
+ "vfe1",
+ "vfe1_cphy_rx",
+ "vfe1_csid",
+ "vfe1_areg",
+ "vfe_lite_ahb",
+ "vfe_lite_axi",
+ "vfe_lite",
+ "vfe_lite_cphy_rx",
+ "vfe_lite_csid";
+
+ iommus = <&apps_smmu 0x800 0x400>,
+ <&apps_smmu 0x801 0x400>,
+ <&apps_smmu 0x840 0x400>,
+ <&apps_smmu 0x841 0x400>,
+ <&apps_smmu 0xC00 0x400>,
+ <&apps_smmu 0xC01 0x400>,
+ <&apps_smmu 0xC40 0x400>,
+ <&apps_smmu 0xC41 0x400>;
+
+ interconnects = <&gem_noc MASTER_AMPSS_M0 &config_noc SLAVE_CAMERA_CFG>,
+ <&mmss_noc MASTER_CAMNOC_HF &mc_virt SLAVE_EBI_CH0>,
+ <&mmss_noc MASTER_CAMNOC_SF &mc_virt SLAVE_EBI_CH0>,
+ <&mmss_noc MASTER_CAMNOC_ICP &mc_virt SLAVE_EBI_CH0>;
+ interconnect-names = "cam_ahb",
+ "cam_hf_0_mnoc",
+ "cam_sf_0_mnoc",
+ "cam_sf_icp_mnoc";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+ };
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 01/19] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding
2021-12-22 0:37 ` [PATCH v3 01/19] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding Bryan O'Donoghue
@ 2022-01-04 13:42 ` Vladimir Zapolskiy
2022-01-04 15:28 ` Bryan O'Donoghue
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Zapolskiy @ 2022-01-04 13:42 UTC (permalink / raw)
To: Bryan O'Donoghue, Robert Foss
Cc: jonathan, andrey.konovalov, todor.too, agross, bjorn.andersson,
jgrahsl, hfink, dmitry.baryshkov, devicetree, Rob Herring,
hverkuil, mchehab, linux-arm-msm, linux-media
Hi Bryan, Robert,
On 12/22/21 2:37 AM, Bryan O'Donoghue wrote:
> From: Jonathan Marek <jonathan@marek.ca>
>
> Add bindings for qcom,sm8250-camss in order to support the camera
> subsystem for SM8250.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> .../bindings/media/qcom,sm8250-camss.yaml | 450 ++++++++++++++++++
> 1 file changed, 450 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
<snip>
> +required:
> + - clock-names
> + - clocks
> + - compatible
> + - interconnects
> + - interconnect-names
> + - interrupts
> + - interrupt-names
> + - iommus
> + - power-domains
> + - reg
> + - reg-names
> +
> +additionalProperties: false
I've discovered that there is a noticeable difference between this bindings
and all the previous ones, for instance see qcom,sdm845-camss.yaml
There is no required 'vdda-supply' property on the list, and fwiw I believe
there should be two supply properties for 0p9 and 1p2 supplies in fact.
Similarly, two separate supplies should be present in sdm845 camss bindings.
At the moment the driver operates with 'vdda' supply only, commit 9e5d1581
introduced undocumented 'vdd_sec' for sdm660, but, if I'm not mistaken,
it's unused.
In my opinion now it's a convenient moment to add descriptions and support
of two vdda regulators, comments are more than welcome.
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 01/19] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding
2022-01-04 13:42 ` Vladimir Zapolskiy
@ 2022-01-04 15:28 ` Bryan O'Donoghue
2022-01-04 20:44 ` Bryan O'Donoghue
0 siblings, 1 reply; 5+ messages in thread
From: Bryan O'Donoghue @ 2022-01-04 15:28 UTC (permalink / raw)
To: Vladimir Zapolskiy, Robert Foss
Cc: jonathan, andrey.konovalov, todor.too, agross, bjorn.andersson,
jgrahsl, hfink, dmitry.baryshkov, devicetree, Rob Herring,
hverkuil, mchehab, linux-arm-msm, linux-media
On 04/01/2022 13:42, Vladimir Zapolskiy wrote:
> Hi Bryan, Robert,
>
> On 12/22/21 2:37 AM, Bryan O'Donoghue wrote:
>> From: Jonathan Marek <jonathan@marek.ca>
>>
>> Add bindings for qcom,sm8250-camss in order to support the camera
>> subsystem for SM8250.
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>> .../bindings/media/qcom,sm8250-camss.yaml | 450 ++++++++++++++++++
>> 1 file changed, 450 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
>>
>> diff --git
>> a/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
>> b/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
>
> <snip>
>
>> +required:
>> + - clock-names
>> + - clocks
>> + - compatible
>> + - interconnects
>> + - interconnect-names
>> + - interrupts
>> + - interrupt-names
>> + - iommus
>> + - power-domains
>> + - reg
>> + - reg-names
>> +
>> +additionalProperties: false
>
> I've discovered that there is a noticeable difference between this bindings
> and all the previous ones, for instance see qcom,sdm845-camss.yaml
>
> There is no required 'vdda-supply' property on the list, and fwiw I believe
> there should be two supply properties for 0p9 and 1p2 supplies in fact.
> Similarly, two separate supplies should be present in sdm845 camss
> bindings.
The 1p2 supply is defined in the camera sensor section as dvdd-supply
https://git.linaro.org/people/bryan.odonoghue/kernel.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?h=v5.16-rc6-sm8250-camss-imx577-only-v3
1p2 connects to dvdd-supply as per Miura(865-RB5)-Camera_NAV_Mezz page
10 - thundercomm registration required to view
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts::&cci_i2c2::camera@1a
{
dovdd-supply = <&vreg_s4a_1p8>;
avdd-supply = <&vreg_l7f_1p8>;
dvdd-supply = <&vreg_l9a_1p2>;
}
similarly to
arch/arm64/boot/dts/qcom/sdm845-db845c.dts::&cci_i2c0::camera@10 {
dovdd-supply = <&vreg_lvs1a_1p8>;
avdd-supply = <&cam0_avdd_2v8>;
dvdd-supply = <&cam0_dvdd_1v2>;
}
and
arch/arm64/boot/dts/qcom/apq8016-sbc.dts::&cci_i2c0::camera_rear@3b
{
vdddo-supply = <&camera_vdddo_1v8>;
vdda-supply = <&camera_vdda_2v8>;
vddd-supply = <&camera_vddd_1v5>; /* bod: here */
}
The IMX sensor needs to have the regulator_bulk_enable() stuff added,
I'll post this patch it works standalone
https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=v5.16-rc6-sm8250-camss-imx577-only-v3&id=e82fc1b29d9227cad3ad7dcab362c39dd4a63bdb
Simiar to 0c2c7a1e0d69 ("media: ov8856: Add devicetree support")
Downstream points the CPA to "camss-vdd-supply = <&titan_top_gdsc>;"
which is covered by our TITAN_TOP_GDSC power-domain and
"mipi-csi-vdd-supply = <&pm8150_l9>;"
regulator-pm8150-l9 == rpmh-regulator-ldoa9 == ldoa9 == pmic5-ldo
aka l9a upstream
"dvdd-supply = <&vreg_l9a_1p2>;"
vreg_l9a_1p2: ldo9 {
regulator-name = "vreg_l9a_1p2";
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
Not sure I see on the schematic or in the downstream dts how 0p9 is
connected to the camera - seems to be a pcie and or mdss-dsi regulator.
If vdda-supply is a required property of the camera and not the sensor
then what regulator do you think it should point to ?
> At the moment the driver operates with 'vdda' supply only, commit 9e5d1581
> introduced undocumented 'vdd_sec' for sdm660, but, if I'm not mistaken,
> it's unused.
I agree with you there - vdd_sec is either unspecified in the
Documentation or not required.
---
bod
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 01/19] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding
2022-01-04 15:28 ` Bryan O'Donoghue
@ 2022-01-04 20:44 ` Bryan O'Donoghue
2022-01-05 1:15 ` Bryan O'Donoghue
0 siblings, 1 reply; 5+ messages in thread
From: Bryan O'Donoghue @ 2022-01-04 20:44 UTC (permalink / raw)
To: Vladimir Zapolskiy, Robert Foss
Cc: jonathan, andrey.konovalov, todor.too, agross, bjorn.andersson,
jgrahsl, hfink, dmitry.baryshkov, devicetree, Rob Herring,
hverkuil, mchehab, linux-arm-msm, linux-media
On 04/01/2022 15:28, Bryan O'Donoghue wrote:
> On 04/01/2022 13:42, Vladimir Zapolskiy wrote:
>> Hi Bryan, Robert,
>>
>> On 12/22/21 2:37 AM, Bryan O'Donoghue wrote:
>>> From: Jonathan Marek <jonathan@marek.ca>
>>>
>>> Add bindings for qcom,sm8250-camss in order to support the camera
>>> subsystem for SM8250.
>>>
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>> ---
>>> .../bindings/media/qcom,sm8250-camss.yaml | 450 ++++++++++++++++++
>>> 1 file changed, 450 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
>>> b/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
>>
>> <snip>
>>
>>> +required:
>>> + - clock-names
>>> + - clocks
>>> + - compatible
>>> + - interconnects
>>> + - interconnect-names
>>> + - interrupts
>>> + - interrupt-names
>>> + - iommus
>>> + - power-domains
>>> + - reg
>>> + - reg-names
>>> +
>>> +additionalProperties: false
>>
>> I've discovered that there is a noticeable difference between this
>> bindings
>> and all the previous ones, for instance see qcom,sdm845-camss.yaml
>>
>> There is no required 'vdda-supply' property on the list, and fwiw I
>> believe
>> there should be two supply properties for 0p9 and 1p2 supplies in fact.
>> Similarly, two separate supplies should be present in sdm845 camss
>> bindings.
>
> The 1p2 supply is defined in the camera sensor section as dvdd-supply
>
> https://git.linaro.org/people/bryan.odonoghue/kernel.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?h=v5.16-rc6-sm8250-camss-imx577-only-v3
>
>
> 1p2 connects to dvdd-supply as per Miura(865-RB5)-Camera_NAV_Mezz page
> 10 - thundercomm registration required to view
>
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts::&cci_i2c2::camera@1a
> {
> dovdd-supply = <&vreg_s4a_1p8>;
> avdd-supply = <&vreg_l7f_1p8>;
> dvdd-supply = <&vreg_l9a_1p2>;
> }
>
> similarly to
>
> arch/arm64/boot/dts/qcom/sdm845-db845c.dts::&cci_i2c0::camera@10 {
> dovdd-supply = <&vreg_lvs1a_1p8>;
> avdd-supply = <&cam0_avdd_2v8>;
> dvdd-supply = <&cam0_dvdd_1v2>;
> }
>
> and
>
> arch/arm64/boot/dts/qcom/apq8016-sbc.dts::&cci_i2c0::camera_rear@3b
> {
> vdddo-supply = <&camera_vdddo_1v8>;
> vdda-supply = <&camera_vdda_2v8>;
> vddd-supply = <&camera_vddd_1v5>; /* bod: here */
> }
>
> The IMX sensor needs to have the regulator_bulk_enable() stuff added,
> I'll post this patch it works standalone
>
> https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=v5.16-rc6-sm8250-camss-imx577-only-v3&id=e82fc1b29d9227cad3ad7dcab362c39dd4a63bdb
>
>
> Simiar to 0c2c7a1e0d69 ("media: ov8856: Add devicetree support")
>
> Downstream points the CPA to "camss-vdd-supply = <&titan_top_gdsc>;"
> which is covered by our TITAN_TOP_GDSC power-domain and
> "mipi-csi-vdd-supply = <&pm8150_l9>;"
>
> regulator-pm8150-l9 == rpmh-regulator-ldoa9 == ldoa9 == pmic5-ldo
>
> aka l9a upstream
>
> "dvdd-supply = <&vreg_l9a_1p2>;"
>
> vreg_l9a_1p2: ldo9 {
> regulator-name = "vreg_l9a_1p2";
> regulator-min-microvolt = <1200000>;
> regulator-max-microvolt = <1200000>;
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> };
>
> Not sure I see on the schematic or in the downstream dts how 0p9 is
> connected to the camera - seems to be a pcie and or mdss-dsi regulator.
>
> If vdda-supply is a required property of the camera and not the sensor
> then what regulator do you think it should point to ?
>
>> At the moment the driver operates with 'vdda' supply only, commit
>> 9e5d1581
>> introduced undocumented 'vdd_sec' for sdm660, but, if I'm not mistaken,
>> it's unused.
>
> I agree with you there - vdd_sec is either unspecified in the
> Documentation or not required.
>
> ---
> bod
Keep guessing there BOD.
Having a deep dive on the schematics of the RB3 and the RB5 there are a
number of broken assumptions on regulators that "just happen to work"
I've corrected these for RB5 and will presently validate on RB3, I've
added in regulator_bulk_enable and regulator_bulk_disable which should
capture what we need to do for sm8250 and 630 - once we fixup the DTS
for 630
Note: I don't have schematics for 630 or to my knowledge DTS either - I
haven't looked all that hard, perhaps its easy to find.
The CSI vdda enable is still required in the sensor but @
dovdd-supply = <&vreg_l7f_1p8>; not dovdd-supply = <&vreg_s4a_1p8>;
The sensor stuff hasn't been submitted upstream but still my v3 tree is
wrong.
On RB5 if we switched off USB, UFS and PCIe there's no way the camera
could come up.
The patch in the series I picked up from @Jonathan to drop regulator
enable for rb3 is also wrong.
I've got the RB5 part working a bit better - need to fixup for RB3 and
will resend
git.linaro.org/people/bryan.odonoghue/kernel.git /
v5.16-rc6-sm8250-camss-imx577-only-v4
Thanks for the ping @Vladimir.
Please don't apply V3 for now
---
bod
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 01/19] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding
2022-01-04 20:44 ` Bryan O'Donoghue
@ 2022-01-05 1:15 ` Bryan O'Donoghue
0 siblings, 0 replies; 5+ messages in thread
From: Bryan O'Donoghue @ 2022-01-05 1:15 UTC (permalink / raw)
To: Vladimir Zapolskiy, Robert Foss
Cc: jonathan, andrey.konovalov, todor.too, agross, bjorn.andersson,
jgrahsl, hfink, dmitry.baryshkov, devicetree, Rob Herring,
hverkuil, mchehab, linux-arm-msm, linux-media
On 04/01/2022 20:44, Bryan O'Donoghue wrote:
> Please don't apply V3 for now
I see the pull-request is already active
That's no problem I'll make a new patchset to
- Add regulator-bulk support to camss
- Fixing RB3/SDM845
- Fixing RB5/SM8250
- Fixing SDM660
These three platforms work fine as is we just
need to fixup the description of the regulators
- And add in the sensors for the RB5 which
is where most of the changes to my branch are
and have no been submitted to this list yet
Seems less confusing that way. I'll see if I can get that patchset out
tomorrow
---
bod
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-05 1:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20211222003751.2461466-1-bryan.odonoghue@linaro.org>
2021-12-22 0:37 ` [PATCH v3 01/19] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding Bryan O'Donoghue
2022-01-04 13:42 ` Vladimir Zapolskiy
2022-01-04 15:28 ` Bryan O'Donoghue
2022-01-04 20:44 ` Bryan O'Donoghue
2022-01-05 1:15 ` Bryan O'Donoghue
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).