[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <22479c27-9265-4994-8974-9739ecbae5ee@linaro.org>
Date: Tue, 17 Dec 2024 16:40:12 +0200
From: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>
To: Vikram Sharma <quic_vikramsa@...cinc.com>, rfoss@...nel.org,
todor.too@...il.com, bryan.odonoghue@...aro.org, mchehab@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
akapatra@...cinc.com, hariramp@...cinc.com, andersson@...nel.org,
konradybcio@...nel.org, hverkuil-cisco@...all.nl,
cros-qcom-dts-watchers@...omium.org, catalin.marinas@....com, will@...nel.org
Cc: linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel@...cinc.com,
Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Subject: Re: [PATCH v10 4/4] arm64: dts: qcom:
qcs6490-rb3gen2-vision-mezzanine: Add vision mezzanine
On 12/17/24 16:06, Vikram Sharma wrote:
> The Vision Mezzanine for the RB3 ships with an imx577 camera sensor.
> Enable the IMX577 on the vision mezzanine.
>
> An example media-ctl pipeline for the imx577 is:
>
> media-ctl --reset
> media-ctl -v -V '"imx577 '19-001a'":0[fmt:SRGGB10/4056x3040 field:none]'
> media-ctl -V '"msm_csiphy3":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_csiphy3":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: Hariram Purushothaman <quic_hariramp@...cinc.com>
> Signed-off-by: Vikram Sharma <quic_vikramsa@...cinc.com>
> Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@...cinc.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 4 +
> .../qcs6490-rb3gen2-vision-mezzanine.dtso | 109 ++++++++++++++++++
> 2 files changed, 113 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtso
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 4686f2a8ddd8..a7e88fcabded 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -115,6 +115,10 @@ dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs615-ride.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs6490-rb3gen2.dtb
> +
> +qcs6490-rb3gen2-vision-mezzanine-dtbs := qcs6490-rb3gen2.dtb qcs6490-rb3gen2-vision-mezzanine.dtbo
> +
> +dtb-$(CONFIG_ARCH_QCOM) += qcs6490-rb3gen2-vision-mezzanine.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs8300-ride.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs8550-aim300-aiot.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs9100-ride.dtb
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtso
> new file mode 100644
> index 000000000000..7782c4aee576
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtso
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +/*
> + * Camera Sensor overlay on top of rb3gen2 core kit.
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/clock/qcom,camcc-sc7280.h>
Please sort the header files alphabetically.
> +
> +/dts-v1/;
> +/plugin/;
> +
Please put these two lines right after the comments header.
> +&camss {
> + vdda-phy-supply = <&vreg_l10c_0p88>;
> + vdda-pll-supply = <&vreg_l6b_1p2>;
> +
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* The port index denotes CSIPHY id i.e. csiphy3 */
> + port@3 {
> + reg = <3>;
> +
> + csiphy3_ep: endpoint {
> + clock-lanes = <7>;
> + data-lanes = <0 1 2 3>;
> + remote-endpoint = <&imx577_ep>;
> + };
> + };
> + };
> +};
> +
> +&cci1 {
> + status = "okay";
> +};
> +
> +&cci1_i2c1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + camera@1a {
> + compatible = "sony,imx577";
> +
> + 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_MCLK3_CLK>;
> + assigned-clocks = <&camcc CAM_CC_MCLK3_CLK>;
> + assigned-clock-rates = <24000000>;
> +
> + dovdd-supply = <&vreg_l18b_1p8>;
Please remove double space before '='.
> + avdd-supply = <&vph_pwr>;
> + dvdd-supply = <&vph_pwr>;
> +
> + port {
> + imx577_ep: endpoint {
> + clock-lanes = <7>;
It is an invalid property/value of the sensor, please remove it.
> + link-frequencies = /bits/ 64 <600000000>;
> + data-lanes = <0 1 2 3>;
data-lanes = <1 2 3 4>;
> + remote-endpoint = <&csiphy3_ep>;
> + };
> + };
> + };
> +};
> +
> +&tlmm {
> + cam2_default: cam2-default-state {
> + mclk-pins {
> + pins = "gpio67";
> + function = "cam_mclk";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + rst-pins {
> + pins = "gpio78";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> + };
> +
> + cam2_suspend: cam2-suspend-state {
> + mclk-pins {
> + pins = "gpio67";
> + function = "cam_mclk";
> + drive-strength = <2>;
> + bias-pull-down;
> + };
> +
> + rst-pins {
> + pins = "gpio78";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-down;
> + output-low;
> + };
I have doubts that it's proper to embed a reset gpio into driver's
pinctrl suspend/resume power management.
Konrad, can you please confirm that it's really accepted?
I'd rather ask to remove this reset pin control.
> + };
> +};
--
Best wishes,
Vladimir
Powered by blists - more mailing lists