[PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix

[PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix

[Bryan O'Donoghue]

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(-)

-- 
2.36.1

[PATCH v2 1/4] i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter

[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

[PATCH v2 2/4] arm64: dts: qcom: sm8250: Disable camcc by default

[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

[PATCH v2 3/4] arm64: dts: qcom: sm8250: camss: Define ports address/size cells

[Bryan O'Donoghue]

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

[PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2

[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

Re: [PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2

[Dmitry Baryshkov]

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

Re: [PATCH v2 4/4] arm64: dts: qcom: qrb5165-rb5: Enable the IMX577 on cam2

[Bryan O'Donoghue]

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

Re: [PATCH v2 1/4] i2c: qcom-cci: Fix ordering of pm_runtime_xx and i2c_add_adapter

[Vladimir Zapolskiy]

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

Re: [PATCH v2 3/4] arm64: dts: qcom: sm8250: camss: Define ports address/size cells

[Vladimir Zapolskiy]

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

Re: [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix

[Benjamin Mugnier]

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.

Re: [PATCH v2 0/4] Switch on IMX577 on RB5 with a new CCI fix

[Bryan O'Donoghue]

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