* [PATCH v2 1/4] i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter
2022-05-24 14:02 [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix Bryan O'Donoghue
@ 2022-05-24 14:02 ` Bryan O'Donoghue
2022-05-25 7:27 ` Vladimir Zapolskiy
2022-05-24 14:02 ` [PATCH v2 2/4] arm64: dts: qcom: sm8250: Disable camcc by default Bryan O'Donoghue
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Bryan O'Donoghue @ 2022-05-24 14:02 UTC (permalink / raw)
To: robert.foss, todor.too, agross, bjorn.andersson
Cc: vladimir.zapolskiy, mchehab, robh+dt, krzk+dt, linux-media,
linux-arm-msm, devicetree, mmitkov, jgrahsl, hfink,
Bryan O'Donoghue
When we compile-in the CCI along with the imx412 driver and run on the RB5
we see that i2c_add_adapter() causes the probe of the imx412 driver to
happen.
This probe tries to perform an i2c xfer() and the xfer() in i2c-qcom-cci.c
fails on pm_runtime_get() because the i2c-qcom-cci.c::probe() function has
not completed to pm_runtime_enable(dev).
Fix this sequence by ensuring pm_runtime_xxx() calls happen prior to adding
the i2c adapter.
Fixes: e517526195de ("i2c: Add Qualcomm CCI I2C driver")
Reported-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/i2c/busses/i2c-qcom-cci.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
index 5c7cc862f08f..90d02effeae9 100644
--- a/drivers/i2c/busses/i2c-qcom-cci.c
+++ b/drivers/i2c/busses/i2c-qcom-cci.c
@@ -638,6 +638,11 @@ static int cci_probe(struct platform_device *pdev)
if (ret < 0)
goto error;
+ pm_runtime_set_autosuspend_delay(dev, MSEC_PER_SEC);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
for (i = 0; i < cci->data->num_masters; i++) {
if (!cci->master[i].cci)
continue;
@@ -649,14 +654,13 @@ static int cci_probe(struct platform_device *pdev)
}
}
- pm_runtime_set_autosuspend_delay(dev, MSEC_PER_SEC);
- pm_runtime_use_autosuspend(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
-
return 0;
error_i2c:
+ pm_runtime_put(dev);
+ pm_runtime_disable(dev);
+ pm_runtime_dont_use_autosuspend(dev);
+
for (--i ; i >= 0; i--) {
if (cci->master[i].cci) {
i2c_del_adapter(&cci->master[i].adap);
--
2.36.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter
2022-05-24 14:02 ` [PATCH v2 1/4] i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter Bryan O'Donoghue
@ 2022-05-25 7:27 ` Vladimir Zapolskiy
0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Zapolskiy @ 2022-05-25 7:27 UTC (permalink / raw)
To: Bryan O'Donoghue, robert.foss, todor.too, agross, bjorn.andersson
Cc: mchehab, robh+dt, krzk+dt, linux-media, linux-arm-msm,
devicetree, mmitkov, jgrahsl, hfink
Hi Bryan,
On 5/24/22 17:02, Bryan O'Donoghue wrote:
> When we compile-in the CCI along with the imx412 driver and run on the RB5
> we see that i2c_add_adapter() causes the probe of the imx412 driver to
> happen.
>
> This probe tries to perform an i2c xfer() and the xfer() in i2c-qcom-cci.c
> fails on pm_runtime_get() because the i2c-qcom-cci.c::probe() function has
> not completed to pm_runtime_enable(dev).
>
> Fix this sequence by ensuring pm_runtime_xxx() calls happen prior to adding
> the i2c adapter.
>
it's a proper root cause disclosure and the fix, thank you.
> Fixes: e517526195de ("i2c: Add Qualcomm CCI I2C driver")
> Reported-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> drivers/i2c/busses/i2c-qcom-cci.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index 5c7cc862f08f..90d02effeae9 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -638,6 +638,11 @@ static int cci_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error;
>
> + pm_runtime_set_autosuspend_delay(dev, MSEC_PER_SEC);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> +
> for (i = 0; i < cci->data->num_masters; i++) {
> if (!cci->master[i].cci)
> continue;
> @@ -649,14 +654,13 @@ static int cci_probe(struct platform_device *pdev)
> }
> }
>
> - pm_runtime_set_autosuspend_delay(dev, MSEC_PER_SEC);
> - pm_runtime_use_autosuspend(dev);
> - pm_runtime_set_active(dev);
> - pm_runtime_enable(dev);
> -
> return 0;
>
> error_i2c:
> + pm_runtime_put(dev);
Here pm_runtime_put(dev) should be removed, there is no pm_runtime_get*(dev) above,
otherwise PM usage counter underflow issues are expected.
> + pm_runtime_disable(dev);
> + pm_runtime_dont_use_autosuspend(dev);
> +
> for (--i ; i >= 0; i--) {
> if (cci->master[i].cci) {
> i2c_del_adapter(&cci->master[i].adap);
With the correction stated above please feel free to add my tags:
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Tested-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] arm64: dts: qcom: sm8250: Disable camcc by default
2022-05-24 14:02 [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix Bryan O'Donoghue
2022-05-24 14:02 ` [PATCH v2 1/4] i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter Bryan O'Donoghue
@ 2022-05-24 14:02 ` Bryan O'Donoghue
2022-05-24 14:02 ` [PATCH v2 3/4] arm64: dts: qcom: sm8250: camss: Define ports address/size cells Bryan O'Donoghue
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Bryan O'Donoghue @ 2022-05-24 14:02 UTC (permalink / raw)
To: robert.foss, todor.too, agross, bjorn.andersson
Cc: vladimir.zapolskiy, mchehab, robh+dt, krzk+dt, linux-media,
linux-arm-msm, devicetree, mmitkov, jgrahsl, hfink,
Bryan O'Donoghue
From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
At the moment there are no changes in SM8250 board files, which require
camera clock controller to run, whenever it is needed for a particular
board, the status of camcc device node will be changed in a board file.
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index cf0c97bd5ad3..2bc11cad3a44 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3407,6 +3407,7 @@ camcc: clock-controller@ad00000 {
clock-names = "iface", "bi_tcxo", "bi_tcxo_ao", "sleep_clk";
power-domains = <&rpmhpd SM8250_MMCX>;
required-opps = <&rpmhpd_opp_low_svs>;
+ status = "disabled";
#clock-cells = <1>;
#reset-cells = <1>;
#power-domain-cells = <1>;
--
2.36.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] arm64: dts: qcom: sm8250: camss: Define ports address/size cells
2022-05-24 14:02 [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix Bryan O'Donoghue
2022-05-24 14:02 ` [PATCH v2 1/4] i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter Bryan O'Donoghue
2022-05-24 14:02 ` [PATCH v2 2/4] arm64: dts: qcom: sm8250: Disable camcc by default Bryan O'Donoghue
@ 2022-05-24 14:02 ` Bryan O'Donoghue
2022-05-25 7:31 ` Vladimir Zapolskiy
2022-05-24 14:02 ` [PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2 Bryan O'Donoghue
2022-07-21 11:38 ` [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix Benjamin Mugnier
4 siblings, 1 reply; 11+ messages in thread
From: Bryan O'Donoghue @ 2022-05-24 14:02 UTC (permalink / raw)
To: robert.foss, todor.too, agross, bjorn.andersson
Cc: vladimir.zapolskiy, mchehab, robh+dt, krzk+dt, linux-media,
linux-arm-msm, devicetree, mmitkov, jgrahsl, hfink,
Bryan O'Donoghue, Konrad Dybcio
The ports {} address and size cells definition is the same for every
derived 8250 board so, we should define it in the core sm8250.dtsi.
Suggested-by: Konrad Dybcio <konrad.dybcio@somainline.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 2bc11cad3a44..aa9a13364865 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3395,6 +3395,11 @@ camss: camss@ac6a000 {
"cam_hf_0_mnoc",
"cam_sf_0_mnoc",
"cam_sf_icp_mnoc";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
camcc: clock-controller@ad00000 {
--
2.36.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] arm64: dts: qcom: sm8250: camss: Define ports address/size cells
2022-05-24 14:02 ` [PATCH v2 3/4] arm64: dts: qcom: sm8250: camss: Define ports address/size cells Bryan O'Donoghue
@ 2022-05-25 7:31 ` Vladimir Zapolskiy
0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Zapolskiy @ 2022-05-25 7:31 UTC (permalink / raw)
To: Bryan O'Donoghue, robert.foss, todor.too, agross, bjorn.andersson
Cc: mchehab, robh+dt, krzk+dt, linux-media, linux-arm-msm,
devicetree, mmitkov, jgrahsl, hfink, Konrad Dybcio
On 5/24/22 17:02, Bryan O'Donoghue wrote:
> The ports {} address and size cells definition is the same for every
> derived 8250 board so, we should define it in the core sm8250.dtsi.
>
> Suggested-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 2bc11cad3a44..aa9a13364865 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -3395,6 +3395,11 @@ camss: camss@ac6a000 {
> "cam_hf_0_mnoc",
> "cam_sf_0_mnoc",
> "cam_sf_icp_mnoc";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> };
>
> camcc: clock-controller@ad00000 {
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2
2022-05-24 14:02 [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix Bryan O'Donoghue
` (2 preceding siblings ...)
2022-05-24 14:02 ` [PATCH v2 3/4] arm64: dts: qcom: sm8250: camss: Define ports address/size cells Bryan O'Donoghue
@ 2022-05-24 14:02 ` Bryan O'Donoghue
2022-05-24 16:21 ` Dmitry Baryshkov
2022-07-21 11:38 ` [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix Benjamin Mugnier
4 siblings, 1 reply; 11+ messages in thread
From: Bryan O'Donoghue @ 2022-05-24 14:02 UTC (permalink / raw)
To: robert.foss, todor.too, agross, bjorn.andersson
Cc: vladimir.zapolskiy, mchehab, robh+dt, krzk+dt, linux-media,
linux-arm-msm, devicetree, mmitkov, jgrahsl, hfink,
Bryan O'Donoghue
The IMX577 is on CCI1/CSI2 providing four lanes of camera data.
An example media-ctl pipeline is:
media-ctl --reset
media-ctl -v -d /dev/media0 -V '"imx412 '20-001a'":0[fmt:SRGGB10/4056x3040 field:none]'
media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/4056x3040]'
media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]'
media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]'
media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 60 ++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sm8250.dtsi | 33 +++++++++++++
2 files changed, 93 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index 0e63f707b911..756ddeb7530b 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -1294,3 +1294,63 @@ &qup_spi0_data_clk {
drive-strength = <6>;
bias-disable;
};
+
+&camcc {
+ status = "okay";
+};
+
+&camss {
+ status = "okay";
+ vdda-phy-supply = <&vreg_l5a_0p88>;
+ vdda-pll-supply = <&vreg_l9a_1p2>;
+
+ ports {
+ /* The port index denotes CSIPHY id i.e. csiphy2 */
+ port@2 {
+ reg = <2>;
+ csiphy2_ep: endpoint {
+ clock-lanes = <7>;
+ data-lanes = <0 1 2 3>;
+ remote-endpoint = <&imx412_ep>;
+ };
+
+ };
+ };
+};
+
+&cci1 {
+ status = "okay";
+};
+
+&cci1_i2c0 {
+ camera@1a {
+ /*
+ * rb5 ships with an imx577. camx code from qcom treats imx412
+ * and imx577 the same way. Absent better data do the same here.
+ */
+ compatible = "sony,imx412";
+ reg = <0x1a>;
+
+ reset-gpios = <&tlmm 78 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default", "suspend";
+ pinctrl-0 = <&cam2_default>;
+ pinctrl-1 = <&cam2_suspend>;
+
+ clocks = <&camcc CAM_CC_MCLK2_CLK>;
+ assigned-clocks = <&camcc CAM_CC_MCLK2_CLK>;
+ assigned-clock-rates = <24000000>;
+
+ dovdd-supply = <&vreg_l7f_1p8>;
+ avdd-supply = <&vdc_5v>;
+ dvdd-supply = <&vdc_5v>;
+
+ port {
+ imx412_ep: endpoint {
+ clock-lanes = <1>;
+ link-frequencies = /bits/ 64 <600000000>;
+ data-lanes = <1 2 3 4>;
+ remote-endpoint = <&csiphy2_ep>;
+ };
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index aa9a13364865..2b65ec2806d0 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3788,6 +3788,39 @@ tlmm: pinctrl@f100000 {
gpio-ranges = <&tlmm 0 0 181>;
wakeup-parent = <&pdc>;
+ cam2_default: cam2-default {
+ rst {
+ pins = "gpio78";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ mclk {
+ pins = "gpio96";
+ function = "cam_mclk";
+ drive-strength = <16>;
+ bias-disable;
+ };
+ };
+
+ cam2_suspend: cam2-suspend {
+ rst {
+ pins = "gpio78";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
+ output-low;
+ };
+
+ mclk {
+ pins = "gpio96";
+ function = "cam_mclk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+
cci0_default: cci0-default {
cci0_i2c0_default: cci0-i2c0-default {
/* SDA, SCL */
--
2.36.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2
2022-05-24 14:02 ` [PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2 Bryan O'Donoghue
@ 2022-05-24 16:21 ` Dmitry Baryshkov
2022-05-24 16:44 ` Bryan O'Donoghue
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2022-05-24 16:21 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: robert.foss, todor.too, agross, bjorn.andersson,
vladimir.zapolskiy, mchehab, robh+dt, krzk+dt, linux-media,
linux-arm-msm, devicetree, mmitkov, jgrahsl, hfink
On Tue, 24 May 2022 at 17:02, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> The IMX577 is on CCI1/CSI2 providing four lanes of camera data.
By default the RB5 doesn't employ the navigation mezzanine. Thus I
suggest adding a new DTS file that will include the qrb5165-rb5.dts
and extend it with camcc/camss setup.
I remember, this was discussed back and forth. I think it's time we
either create a working repo for mezzanines or push default setups
into the kernel.
I'd vote for the later option.
>
> An example media-ctl pipeline is:
>
> media-ctl --reset
> media-ctl -v -d /dev/media0 -V '"imx412 '20-001a'":0[fmt:SRGGB10/4056x3040 field:none]'
> media-ctl -V '"msm_csiphy2":0[fmt:SRGGB10/4056x3040]'
> media-ctl -V '"msm_csid0":0[fmt:SRGGB10/4056x3040]'
> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SRGGB10/4056x3040]'
> media-ctl -l '"msm_csiphy2":1->"msm_csid0":0[1]'
> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
>
> yavta -B capture-mplane -c -I -n 5 -f SRGGB10P -s 4056x3040 -F /dev/video0
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 60 ++++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 33 +++++++++++++
> 2 files changed, 93 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> index 0e63f707b911..756ddeb7530b 100644
> --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> @@ -1294,3 +1294,63 @@ &qup_spi0_data_clk {
> drive-strength = <6>;
> bias-disable;
> };
> +
> +&camcc {
> + status = "okay";
> +};
> +
> +&camss {
> + status = "okay";
> + vdda-phy-supply = <&vreg_l5a_0p88>;
> + vdda-pll-supply = <&vreg_l9a_1p2>;
> +
> + ports {
> + /* The port index denotes CSIPHY id i.e. csiphy2 */
> + port@2 {
> + reg = <2>;
> + csiphy2_ep: endpoint {
> + clock-lanes = <7>;
> + data-lanes = <0 1 2 3>;
> + remote-endpoint = <&imx412_ep>;
> + };
> +
> + };
> + };
> +};
> +
> +&cci1 {
> + status = "okay";
> +};
> +
> +&cci1_i2c0 {
> + camera@1a {
> + /*
> + * rb5 ships with an imx577. camx code from qcom treats imx412
> + * and imx577 the same way. Absent better data do the same here.
> + */
> + compatible = "sony,imx412";
> + reg = <0x1a>;
> +
> + reset-gpios = <&tlmm 78 GPIO_ACTIVE_LOW>;
> + pinctrl-names = "default", "suspend";
> + pinctrl-0 = <&cam2_default>;
> + pinctrl-1 = <&cam2_suspend>;
> +
> + clocks = <&camcc CAM_CC_MCLK2_CLK>;
> + assigned-clocks = <&camcc CAM_CC_MCLK2_CLK>;
> + assigned-clock-rates = <24000000>;
> +
> + dovdd-supply = <&vreg_l7f_1p8>;
> + avdd-supply = <&vdc_5v>;
> + dvdd-supply = <&vdc_5v>;
> +
> + port {
> + imx412_ep: endpoint {
> + clock-lanes = <1>;
> + link-frequencies = /bits/ 64 <600000000>;
> + data-lanes = <1 2 3 4>;
> + remote-endpoint = <&csiphy2_ep>;
> + };
> + };
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index aa9a13364865..2b65ec2806d0 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -3788,6 +3788,39 @@ tlmm: pinctrl@f100000 {
> gpio-ranges = <&tlmm 0 0 181>;
> wakeup-parent = <&pdc>;
>
> + cam2_default: cam2-default {
> + rst {
> + pins = "gpio78";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + mclk {
> + pins = "gpio96";
> + function = "cam_mclk";
> + drive-strength = <16>;
> + bias-disable;
> + };
> + };
> +
> + cam2_suspend: cam2-suspend {
> + rst {
> + pins = "gpio78";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-down;
> + output-low;
> + };
> +
> + mclk {
> + pins = "gpio96";
> + function = "cam_mclk";
> + drive-strength = <2>;
> + bias-disable;
> + };
> + };
> +
> cci0_default: cci0-default {
> cci0_i2c0_default: cci0-i2c0-default {
> /* SDA, SCL */
> --
> 2.36.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2
2022-05-24 16:21 ` Dmitry Baryshkov
@ 2022-05-24 16:44 ` Bryan O'Donoghue
0 siblings, 0 replies; 11+ messages in thread
From: Bryan O'Donoghue @ 2022-05-24 16:44 UTC (permalink / raw)
To: Dmitry Baryshkov, Bryan O'Donoghue
Cc: robert.foss, todor.too, agross, bjorn.andersson,
vladimir.zapolskiy, mchehab, robh+dt, krzk+dt, linux-media,
linux-arm-msm, devicetree, mmitkov, jgrahsl, hfink
On 24/05/2022 17:21, Dmitry Baryshkov wrote:
> On Tue, 24 May 2022 at 17:02, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> The IMX577 is on CCI1/CSI2 providing four lanes of camera data.
>
> By default the RB5 doesn't employ the navigation mezzanine. Thus I
> suggest adding a new DTS file that will include the qrb5165-rb5.dts
> and extend it with camcc/camss setup.
It makes sense to me.
I'll wait to hear from Robert and Bjorn. We can take the opportunity to
do it for RB3 too.
---
bod
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix
2022-05-24 14:02 [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix Bryan O'Donoghue
` (3 preceding siblings ...)
2022-05-24 14:02 ` [PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2 Bryan O'Donoghue
@ 2022-07-21 11:38 ` Benjamin Mugnier
2022-07-21 12:29 ` Bryan O'Donoghue
4 siblings, 1 reply; 11+ messages in thread
From: Benjamin Mugnier @ 2022-07-21 11:38 UTC (permalink / raw)
To: Bryan O'Donoghue, robert.foss, todor.too, agross, bjorn.andersson
Cc: vladimir.zapolskiy, mchehab, robh+dt, krzk+dt, linux-media,
linux-arm-msm, devicetree, mmitkov, jgrahsl, hfink
Hi Bryan,
On 24/05/2022 16:02, Bryan O'Donoghue wrote:
> V2:
>
> - Adds fix for bug identified by Vladimir
> The CCI i2c_adapter_add() and pm_runtime_enable() are racy.
> This is a generic problem not related to the rb5/imx577 but, for the sake
> of our conversation/review's context I'll add it into this series.
> - Include Vladimir's camcc patch
> I've also opted to include Vladimir's disable of camcc to make the enable
> of it in my patchset logical.
> - Move address/size cells Konrad
> - Remove newline in pin definitions - Konrad
> - Remove sensor 'status = "okay"' - Konrad
> - Add comment to qrb5165-rb5.dts re: imx412 and imx577 difference - Konrad
> - Move pin definitions to 8250 dtsi - Vladimir
> - Drop power domain from sensor definition - Vladimir
> - Correct to "add to cam2" not "cam1" in commit log - bod
>
> To make verification of the CCI race eaiser I've provided a defconfig both
> with and without modules enabled.
>
> Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=linux-next-24-05-22%2bimx577-rb5
> Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=linux-next-24-05-22%2bimx577-rb5-compiled-in
>
> git diff linaro/linux-next-22-05-22+imx577-rb5 linaro/linux-next-24-05-22+imx577-rb5
>
> V1:
> Linux-next now has everything we need to switch on this sensor both in the
> qcom DTS and in the imx412 driver.
>
> After this, no further dts or driver work is required to capture images on
> the RB5.
>
> Here's a bootable linux-next with a kernel config. I added Vladimir's
> power-domain changes on-top to verify nothing breaks for me.
>
> https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=linux-next-18-05-22%2bimx577-rb5
>
> Bryan O'Donoghue (3):
> i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter
> arm64: dts: qcom: sm8250: camss: Define ports address/size cells
> arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2
>
> Vladimir Zapolskiy (1):
> arm64: dts: qcom: sm8250: Disable camcc by default
>
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 60 ++++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 39 +++++++++++++++
> drivers/i2c/busses/i2c-qcom-cci.c | 14 ++++--
> 3 files changed, 108 insertions(+), 5 deletions(-)
>
I successfully tested this series with the st-vgxy61 sensor instead of the imx577. I can't provide comments on the device tree patch for the imx577 in 4/4.
For patches 1/4, 2/4, and 3/4:
Tested-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
Thanks again for your work.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix
2022-07-21 11:38 ` [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix Benjamin Mugnier
@ 2022-07-21 12:29 ` Bryan O'Donoghue
0 siblings, 0 replies; 11+ messages in thread
From: Bryan O'Donoghue @ 2022-07-21 12:29 UTC (permalink / raw)
To: Benjamin Mugnier, robert.foss, todor.too, agross, bjorn.andersson
Cc: vladimir.zapolskiy, mchehab, robh+dt, krzk+dt, linux-media,
linux-arm-msm, devicetree, mmitkov, jgrahsl, hfink
On 21/07/2022 12:38, Benjamin Mugnier wrote:
> Hi Bryan,
>
> On 24/05/2022 16:02, Bryan O'Donoghue wrote:
>> V2:
>>
>> - Adds fix for bug identified by Vladimir
>> The CCI i2c_adapter_add() and pm_runtime_enable() are racy.
>> This is a generic problem not related to the rb5/imx577 but, for the sake
>> of our conversation/review's context I'll add it into this series.
>> - Include Vladimir's camcc patch
>> I've also opted to include Vladimir's disable of camcc to make the enable
>> of it in my patchset logical.
>> - Move address/size cells Konrad
>> - Remove newline in pin definitions - Konrad
>> - Remove sensor 'status = "okay"' - Konrad
>> - Add comment to qrb5165-rb5.dts re: imx412 and imx577 difference - Konrad
>> - Move pin definitions to 8250 dtsi - Vladimir
>> - Drop power domain from sensor definition - Vladimir
>> - Correct to "add to cam2" not "cam1" in commit log - bod
>>
>> To make verification of the CCI race eaiser I've provided a defconfig both
>> with and without modules enabled.
>>
>> Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=linux-next-24-05-22%2bimx577-rb5
>> Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=linux-next-24-05-22%2bimx577-rb5-compiled-in
>>
>> git diff linaro/linux-next-22-05-22+imx577-rb5 linaro/linux-next-24-05-22+imx577-rb5
>>
>> V1:
>> Linux-next now has everything we need to switch on this sensor both in the
>> qcom DTS and in the imx412 driver.
>>
>> After this, no further dts or driver work is required to capture images on
>> the RB5.
>>
>> Here's a bootable linux-next with a kernel config. I added Vladimir's
>> power-domain changes on-top to verify nothing breaks for me.
>>
>> https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=linux-next-18-05-22%2bimx577-rb5
>>
>> Bryan O'Donoghue (3):
>> i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter
>> arm64: dts: qcom: sm8250: camss: Define ports address/size cells
>> arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2
>>
>> Vladimir Zapolskiy (1):
>> arm64: dts: qcom: sm8250: Disable camcc by default
>>
>> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 60 ++++++++++++++++++++++++
>> arch/arm64/boot/dts/qcom/sm8250.dtsi | 39 +++++++++++++++
>> drivers/i2c/busses/i2c-qcom-cci.c | 14 ++++--
>> 3 files changed, 108 insertions(+), 5 deletions(-)
>>
>
> I successfully tested this series with the st-vgxy61 sensor instead of the imx577. I can't provide comments on the device tree patch for the imx577 in 4/4.
> For patches 1/4, 2/4, and 3/4:
>
> Tested-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>
>
> Thanks again for your work.
Appreciated.
I'll V3 this series once we sort out the naming of the imx sensor.
https://patchwork.kernel.org/project/linux-media/list/?series=660483
^ permalink raw reply [flat|nested] 11+ messages in thread