[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0a52b6eb-1fc5-4fd6-9016-72e1cb97fcc4@ti.com>
Date: Thu, 8 Feb 2024 14:23:48 +0530
From: Vaishnav Achath <vaishnav.a@...com>
To: Jai Luthra <j-luthra@...com>
CC: <nm@...com>, <vigneshr@...com>, <kristo@...nel.org>, <robh+dt@...nel.org>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<kernel@...gutronix.de>, <linux-arm-kernel@...ts.infradead.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<u-kumar1@...com>, <j-choudhary@...com>
Subject: Re: [PATCH 9/9] arm64: dts: ti: k3-am69-sk: Add overlay for IMX219
Hi Jai, Andrew,
On 31/01/24 13:45, Jai Luthra wrote:
> Hi Vaishnav,
>
> Thanks for the patch.
>
> On Jan 29, 2024 at 18:57:42 +0530, Vaishnav Achath wrote:
>> RPi v2 Camera (IMX219) is an 8MP camera that can be used with SK-AM69
>> through the 22-pin CSI-RX connector.
>>
>> Same overlay can be used across AM68 SK, TDA4VM SK boards that have a
>> 15/22-pin FFC connector. Also enable build testing and symbols for
>> all the three platforms.
>
> In that case, the overlay should be named something generic following
> how it is done for SK-AM62* family of boards.
>
> IMO, it would also make more sense to have separate overlays for CSI0
> and CSI1 ports, unless there are no usecases to plug-in different
> sensors on the board.
>
> Even if that is done later, at least this overlay should be named in a
> way to make it clear it enables 2x IMX219 on both CSI-RX ports, to avoid
> confusion.
>
> So, something like k3-<jacinto?>-sk-csi2-dual-imx219.dtso
>
> This will leave room for a CSI1 only overlay to be called
> k3-<>-sk-csi2-1-<sensor>.dtso
>
Thank you for the review, for now I have renamed the overlay to the
first supported platform(j721e-sk), like AM62X family it is difficult
to pick a common name for these platforms (TDA4VM SK, AM68 SK, AM69 SK),
also jacinto-*.dtso gives the wrong impression that these might work
with the EVMs, so for now I have picked the name to follow the first
supported platfor j721e-sk and clarified in comments and commit message
that this can be reused with other platforms.
>>
>> Signed-off-by: Vaishnav Achath <vaishnav.a@...com>
>> ---
>> arch/arm64/boot/dts/ti/Makefile | 6 +
>> .../boot/dts/ti/k3-am69-sk-csi2-imx219.dtso | 124 ++++++++++++++++++
>> 2 files changed, 130 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/ti/k3-am69-sk-csi2-imx219.dtso
>>
>> diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
>> index 52c1dc910308..9fc8d68f7d26 100644
>> --- a/arch/arm64/boot/dts/ti/Makefile
>> +++ b/arch/arm64/boot/dts/ti/Makefile
>> @@ -80,6 +80,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-j721s2-evm-pcie1-ep.dtbo
>>
>> # Boards with J784s4 SoC
>> dtb-$(CONFIG_ARCH_K3) += k3-am69-sk.dtb
>> +dtb-$(CONFIG_ARCH_K3) += k3-am69-sk-csi2-imx219.dtbo
>> dtb-$(CONFIG_ARCH_K3) += k3-j784s4-evm.dtb
>>
>> # Build time test only, enabled by CONFIG_OF_ALL_DTBS
>> @@ -105,6 +106,8 @@ k3-am642-tqma64xxl-mbax4xxl-sdcard-dtbs := \
>> k3-am642-tqma64xxl-mbax4xxl.dtb k3-am64-tqma64xxl-mbax4xxl-sdcard.dtbo
>> k3-am642-tqma64xxl-mbax4xxl-wlan-dtbs := \
>> k3-am642-tqma64xxl-mbax4xxl.dtb k3-am64-tqma64xxl-mbax4xxl-wlan.dtbo
>> +k3-am69-sk-csi2-imx219-dtbs := k3-am69-sk.dtb \
>> + k3-am69-sk-csi2-imx219.dtbo
>
> Andrew already pointed out that the base dtb should be added in the dtb-
> += entry at the end of this section.
>
> Along with that, I think the overlay should be build-tested for each
> base DTB (AM68, TDA4VM) it is applicable on. Same is already done for
> the shared IMX219/OV5640 overlays between SK-AM62 and SK-AM62A.
>
Fixed in V2:
https://lore.kernel.org/all/20240208084254.295289-1-vaishnav.a@ti.com/
Thanks and Regards,
Vaishnav
>> k3-j721e-evm-pcie0-ep-dtbs := k3-j721e-common-proc-board.dtb \
>> k3-j721e-evm-pcie0-ep.dtbo
>> k3-j721s2-evm-pcie1-ep-dtbs := k3-j721s2-common-proc-board.dtb \
>> @@ -130,5 +133,8 @@ DTC_FLAGS_k3-am62-lp-sk += -@
>> DTC_FLAGS_k3-am62a7-sk += -@
>> DTC_FLAGS_k3-am642-tqma64xxl-mbax4xxl += -@
>> DTC_FLAGS_k3-am6548-iot2050-advanced-m2 += -@
>> +DTC_FLAGS_k3-am68-sk-base-board += -@
>> +DTC_FLAGS_k3-am69-sk += -@
>> DTC_FLAGS_k3-j721e-common-proc-board += -@
>> DTC_FLAGS_k3-j721s2-common-proc-board += -@
>> +DTC_FLAGS_k3-j721e-sk += -@
>> diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk-csi2-imx219.dtso b/arch/arm64/boot/dts/ti/k3-am69-sk-csi2-imx219.dtso
>> new file mode 100644
>> index 000000000000..4cd1d8d5004a
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-am69-sk-csi2-imx219.dtso
>> @@ -0,0 +1,124 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/**
>> + * DT Overlay for RPi Camera V2.1 (Sony IMX219) interfaced with CSI2 on AM68-SK board.
>
> Overlay name says AM69 but comment says AM68 here.
>
>> + * https://datasheets.raspberrypi.org/camera/camera-v2-schematic.pdf
>> + *
>> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
>
> s/2023/2024
>
>> + */
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include "k3-pinctrl.h"
>> +
>> +&{/} {
>> + clk_imx219_fixed: imx219-xclk {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <24000000>;
>> + };
>> +};
>> +
>> +&csi_mux {
>> + idle-state = <1>;
>> +};
>> +
>> +/* CAM0 I2C */
>> +&cam0_i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + imx219_0: imx219_0@10 {
>> + compatible = "sony,imx219";
>> + reg = <0x10>;
>> +
>> + clocks = <&clk_imx219_fixed>;
>> + clock-names = "xclk";
>> +
>> + port {
>> + csi2_cam0: endpoint {
>> + remote-endpoint = <&csi2rx0_in_sensor>;
>> + link-frequencies = /bits/ 64 <456000000>;
>> + clock-lanes = <0>;
>> + data-lanes = <1 2>;
>> + };
>> + };
>> + };
>> +};
>> +
>> +/* CAM1 I2C */
>> +&cam1_i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + imx219_1: imx219_1@10 {
>> + compatible = "sony,imx219";
>> + reg = <0x10>;
>> +
>> + clocks = <&clk_imx219_fixed>;
>> + clock-names = "xclk";
>> +
>> + port {
>> + csi2_cam1: endpoint {
>> + remote-endpoint = <&csi2rx1_in_sensor>;
>> + link-frequencies = /bits/ 64 <456000000>;
>> + clock-lanes = <0>;
>> + data-lanes = <1 2>;
>> + };
>> + };
>> + };
>> +};
>> +
>> +
>> +&cdns_csi2rx0 {
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + csi0_port0: port@0 {
>> + reg = <0>;
>> + status = "okay";
>> +
>> + csi2rx0_in_sensor: endpoint {
>> + remote-endpoint = <&csi2_cam0>;
>> + bus-type = <4>; /* CSI2 DPHY. */
>> + clock-lanes = <0>;
>> + data-lanes = <1 2>;
>> + };
>> + };
>> + };
>> +};
>> +
>> +&dphy0 {
>> + status = "okay";
>> +};
>> +
>> +&ti_csi2rx0 {
>> + status = "okay";
>> +};
>> +
>> +&cdns_csi2rx1 {
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + csi1_port0: port@0 {
>> + reg = <0>;
>> + status = "okay";
>> +
>> + csi2rx1_in_sensor: endpoint {
>> + remote-endpoint = <&csi2_cam1>;
>> + bus-type = <4>; /* CSI2 DPHY. */
>> + clock-lanes = <0>;
>> + data-lanes = <1 2>;
>> + };
>> + };
>> + };
>> +};
>> +
>> +&dphy1 {
>> + status = "okay";
>> +};
>> +
>> +&ti_csi2rx1 {
>> + status = "okay";
>> +};
>> \ No newline at end of file
>> --
>> 2.34.1
>>
>
Powered by blists - more mailing lists