[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9cb4f94e-00e8-43d5-be7a-85dc1188e856@ti.com>
Date: Thu, 8 May 2025 17:46:06 +0530
From: Yemike Abhilash Chandra <y-abhilashchandra@...com>
To: "Kumar, Udit" <u-kumar1@...com>, <nm@...com>, <vigneshr@...com>,
<kristo@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>
CC: <vaishnav.a@...com>, <r-donadkar@...com>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <jai.luthra@...ux.dev>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] arm64: dts: ti: k3-j722s-evm: Add overlay for TEVI
OV5640
Hi Udit,
Thanks for the review.
On 07/05/25 13:50, Kumar, Udit wrote:
> Hello Vaishnav/Abhilash
>
> Thanks for patch
>
> On 5/5/2025 5:27 PM, Yemike Abhilash Chandra wrote:
>> From: Vaishnav Achath <vaishnav.a@...com>
>>
>> TechNexion TEVI OV5640 camera is a 5MP camera that can be used with
>> J722S EVM through the 22-pin CSI-RX connector. Add a reference overlay
>> for quad TEVI OV5640 modules on J722S EVM.
>>
>> Signed-off-by: Vaishnav Achath <vaishnav.a@...com>
>> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@...com>
>> ---
>> arch/arm64/boot/dts/ti/Makefile | 4 +
>> .../k3-j722s-evm-csi2-quad-tevi-ov5640.dtso | 358 ++++++++++++++++++
>> 2 files changed, 362 insertions(+)
>> create mode 100644
>> arch/arm64/boot/dts/ti/k3-j722s-evm-csi2-quad-tevi-ov5640.dtso
>>
>> diff --git a/arch/arm64/boot/dts/ti/Makefile
>> b/arch/arm64/boot/dts/ti/Makefile
>> index 829a3b028466..76b750e4b8a8 100644
>> --- a/arch/arm64/boot/dts/ti/Makefile
>> +++ b/arch/arm64/boot/dts/ti/Makefile
>> @@ -123,6 +123,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm-pcie1-ep.dtbo
>> dtb-$(CONFIG_ARCH_K3) += k3-am67a-beagley-ai.dtb
>> dtb-$(CONFIG_ARCH_K3) += k3-j722s-evm.dtb
>> dtb-$(CONFIG_ARCH_K3) += k3-j722s-evm-csi2-quad-rpi-cam-imx219.dtbo
>> [..]
>> diff --git
>> a/arch/arm64/boot/dts/ti/k3-j722s-evm-csi2-quad-tevi-ov5640.dtso
>> b/arch/arm64/boot/dts/ti/k3-j722s-evm-csi2-quad-tevi-ov5640.dtso
>> new file mode 100644
>> index 000000000000..537224ea60e3
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm-csi2-quad-tevi-ov5640.dtso
>> @@ -0,0 +1,358 @@
>> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
>> +/*
>> + * 4 x TEVI OV5640 MIPI Camera module on RPI camera connector.
>> + *
>> + * Copyright (C) 2024 Texas Instruments Incorporated -
>> https://www.ti.com/
>> + */
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include "k3-pinctrl.h"
>> +
>> +&{/} {
>> + clk_ov5640_fixed: clock-24000000 {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <24000000>;
>> + };
>> +
>
> Please check once , this is clock is 25M or 24M .
>
> As I see CDC6CE025000ADLXT/U28 is marked as 25M OSC
>
This is the crystal clock with in the sensor.
>> + reg_2p8v: regulator-2p8v {
>> + compatible = "regulator-fixed";
>> + regulator-name = "2P8V";
>> + regulator-min-microvolt = <2800000>;
>> + regulator-max-microvolt = <2800000>;
>> + vin-supply = <&vdd_sd_dv>;
>> + regulator-always-on;
>> + };
>> +
>> + reg_1p8v: regulator-1p8v {
>> + compatible = "regulator-fixed";
>> + regulator-name = "1P8V";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + vin-supply = <&vdd_sd_dv>;
>> + regulator-always-on;
>> + };
>> +
>> + reg_3p3v: regulator-3p3v {
>> + compatible = "regulator-fixed";
>> + regulator-name = "3P3V";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + vin-supply = <&vdd_sd_dv>;
>> + regulator-always-on;
>> + };
>
> Sorry but on connector QSH-020-01-L-D-DP-A-K/J27, I see only 3v3 available.
>
> May be I am missing something here but how you are getting 1v8 and 2v8
> supply
>
Yes, 3.3V is provided as the main input supply to the sensor.
However, required 1.8V and 2.8V rails from the 3.3V input.
Please refer to [1] for detailed information.
>
>> +};
>> +
>> +
>> +&main_pmx0 {
>> + cam0_reset_pins_default: cam0-default-reset-pins {
>> + pinctrl-single,pins = <
>> + J722S_IOPAD(0x03c, PIN_OUTPUT, 7)
>> + >;
>> + };
>> +
>
> Please mark GPIO you are using for this in comment
>
> example
>
> J722S_IOPAD(0x1dc, PIN_INPUT, 0) /* (C22) MCAN0_RX */
>
Sure. Will do that.
>
>> + cam1_reset_pins_default: cam1-default-reset-pins {
>> + pinctrl-single,pins = <
>> + J722S_IOPAD(0x044, PIN_OUTPUT, 7)
>> + >;
>> + };
>> +
>> + cam2_reset_pins_default: cam2-default-reset-pins {
>> + pinctrl-single,pins = <
>> + J722S_IOPAD(0x04c, PIN_OUTPUT, 7)
>> + >;
>> + };
>> +
>> + cam3_reset_pins_default: cam3-default-reset-pins {
>> + pinctrl-single,pins = <
>> + J722S_IOPAD(0x054, PIN_OUTPUT, 7)
>> + >;
>> + };
>> +};
>> +
>> +&exp1 {
>> + p06-hog{
>> + /* P06 - CSI01_MUX_SEL_2 */
>> + gpio-hog;
>> + gpios = <6 GPIO_ACTIVE_HIGH>;
>> + output-high;
>> + line-name = "CSI01_MUX_SEL_2";
>
> As per table on page 29
>
> CSI01_MUX_SEL_2 as low means, INPUT<-- A Port [CSI2 Connector]
>
> Please check, if you want to drive this line high or low
These are RPI connectors, It should be high.
>
>> + };
>> +
>> + p07-hog{
>> + /* P01 - CSI23_MUX_SEL_2 */
>> + gpio-hog;
>> + gpios = <7 GPIO_ACTIVE_HIGH>;
>> + output-high;
>> + line-name = "CSI23_MUX_SEL_2";
>> + };
>
> Same as for CSI01_MUX_SEL_2, table on page-30
>
>
>> +};
>> +
>> +&main_gpio0 {
>> + p15-hog {
>> + /* P15 - CSI2_CAMERA_GPIO1 */
>> + gpio-hog;
>> + gpios = <15 GPIO_ACTIVE_HIGH>;
>> + output-high;
>> + line-name = "CSI2_CAMERA_GPIO1";
>> + };
>> +
>> + p17-hog {
>> + /* P17 - CSI2_CAMERA_GPIO2 */
>> + gpio-hog;
>> + gpios = <17 GPIO_ACTIVE_HIGH>;
>> + output-high;
>> + line-name = "CSI2_CAMERA_GPIO2";
>> + };
>> +
>> + p19-hog {
>> + /* P19 - CSI2_CAMERA_GPIO3 */
>> + gpio-hog;
>> + gpios = <19 GPIO_ACTIVE_HIGH>;
>> + output-high;
>> + line-name = "CSI2_CAMERA_GPIO3";
>> + };
>> +
>> + p21-hog {
>> + /* P21 - CSI2_CAMERA_GPIO4 */
>> + gpio-hog;
>> + gpios = <21 GPIO_ACTIVE_HIGH>;
>> + output-high;
>> + line-name = "CSI2_CAMERA_GPIO4";
>> + };
>
> Please check once if P21 and rest above are pin number of SOC ?
>
Yes
>
>> +};
>> +
>> +&pca9543_0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + i2c@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + ov5640_0: camera@3c {
>> + compatible = "ovti,ov5640";
>> + reg = <0x3c>;
>> + clocks = <&clk_ov5640_fixed>;
>> + clock-names = "xclk";
>> +
>> + AVDD-supply = <®_2p8v>;
>> + DOVDD-supply = <®_1p8v>;
>> + DVDD-supply = <®_3p3v>;
>> +
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&cam0_reset_pins_default>;
>
> do you think, you should have reset-gpios as property for this ov5640
> and other nodes too ?
>
Thanks for pointing this out. I will add that in the next revision.
Thanks and Regards
Yemike Abhilash Chandra
[1]:https://www.technexion.com/wp-content/uploads/2023/09/product-brief_tevi-ov5640.pdf
>
>> +
>> [..]
Powered by blists - more mailing lists