| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3e6ab7aa-c606-49bc-8cc5-24c8a1edc081@quicinc.com>
Date: Tue, 17 Dec 2024 23:19:11 +0530
From: Vikram Sharma <quic_vikramsa@...cinc.com>
To: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>, <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/2024 8:10 PM, Vladimir Zapolskiy wrote:
> 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.
Thanks for your comments Vladimir.
Will take care in next version.
>
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +
>
> Please put these two lines right after the comments header.
ACK.
>
>> +&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 '='.
Sure.
>
>> + 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.
Will check more on this internally and respond back.
>
>> + link-frequencies = /bits/ 64 <600000000>;
>> + data-lanes = <0 1 2 3>;
>
> data-lanes = <1 2 3 4>;
Will check more on this internally and respond back
>
>> + 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.
Will discuss this with Konrad and respond.
>> + };
>> +};
>
> --
> Best wishes,
> Vladimir
Best Regards,
Vikram
Powered by blists - more mailing lists