Re: [PATCH v2 2/5] arm64: dts: renesas: sparrow-hawk: Add overlay for IMX219 on J1

[Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>]

Hi Niklas,

Thank you for the patch.

The comments below apply to 3/5 where applicable.

On Tue, Jul 01, 2025 at 01:26:09PM +0200, Niklas Söderlund wrote:
> Add an overlay to connect an IMX219 camera sensor to the J1 connector.
> The IMX219 utilizes 2 CSI-2 D-PHY lanes. This enables the video capture
> pipeline behind the CSI40 Rx to be enabled to process images from the
> sensor.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
> * Changes since v1
> - Drop 'status = "okay"' property for the fixed regulators.
> ---
>  arch/arm64/boot/dts/renesas/Makefile          |   3 +
>  ...8a779g3-sparrow-hawk-camera-j1-imx219.dtso | 118 ++++++++++++++++++
>  2 files changed, 121 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk-camera-j1-imx219.dtso
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index 47e46ef99d36..73218f7ec9af 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -96,7 +96,10 @@ dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g2-white-hawk-single-ard-audio-da7212.dtb
>  
>  DTC_FLAGS_r8a779g3-sparrow-hawk += -Wno-spi_bus_bridge
>  dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g3-sparrow-hawk.dtb
> +dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g3-sparrow-hawk-camera-j1-imx219.dtbo
>  dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g3-sparrow-hawk-fan-pwm.dtbo
> +r8a779g3-sparrow-hawk-camera-j1-imx219-dtbs := r8a779g3-sparrow-hawk.dtb r8a779g3-sparrow-hawk-camera-j1-imx219.dtbo
> +dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g3-sparrow-hawk-camera-j1-imx219.dtb
>  r8a779g3-sparrow-hawk-fan-pwm-dtbs := r8a779g3-sparrow-hawk.dtb r8a779g3-sparrow-hawk-fan-pwm.dtbo
>  dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g3-sparrow-hawk-fan-pwm.dtb
>  
> diff --git a/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk-camera-j1-imx219.dtso b/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk-camera-j1-imx219.dtso
> new file mode 100644
> index 000000000000..a9089d3a4b29
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk-camera-j1-imx219.dtso
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Device Tree Overlay for an IMX219 camera sensor in connector J1 on R-Car V4H
> + * ES3.0 Sparrow Hawk board.
> + *
> + * Copyright 2025 Renesas Electronics Corp.
> + * Copyright 2025 Niklas Söderlund <niklas.soderlund@xxxxxxxxxxxx>
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/media/video-interfaces.h>
> +
> +&{/} {
> +	clk_cam_j1: clk_cam_j1 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +		status = "okay";

No need for status.

> +	};
> +
> +	/* Page 29 / CSI_IF_CN / J1 */
> +	reg_cam_j1: reg_cam_j1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "reg_cam_j1";
> +		enable-active-high;
> +		gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;

No need for a pinctrl entry ?

> +	};
> +
> +	reg_cam_j1_dummy: reg_cam_j1_dummy {
> +		compatible = "regulator-fixed";
> +		regulator-name = "reg_cam_j1_dummy";
> +	};
> +};
> +
> +&i2c1 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	imx219_j1: imx219@10 {

cam@10, and drop the label (it's unused).

> +		compatible = "sony,imx219";
> +		reg = <0x10>;
> +		status = "okay";

Drop status.

> +
> +		clocks = <&clk_cam_j1>;
> +
> +		VANA-supply = <&reg_cam_j1>;
> +		VDIG-supply = <&reg_cam_j1_dummy>;
> +		VDDL-supply = <&reg_cam_j1_dummy>;

You could use the same regulator for the three supplies, and drop
reg_cam_j1_dummy.

> +
> +		orientation = <2>;
> +		rotation = <0>;
> +
> +		port {
> +			imx219_j1_out: endpoint {
> +				clock-noncontinuous;
> +				link-frequencies = /bits/ 64 <456000000>;
> +				data-lanes = <1 2>;
> +				remote-endpoint = <&csi40_in>;
> +			};
> +		};
> +	};
> +};
> +
> +/* Page 29 / CSI_IF_CN */
> +&csi40 {
> +	status = "okay";
> +
> +	ports {
> +		port {
> +			csi40_in: endpoint {
> +				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
> +				clock-lanes = <0>;
> +				data-lanes = <1 2>;
> +				remote-endpoint = <&imx219_j1_out>;
> +			};
> +		};
> +	};
> +};
> +
> +&isp0 {
> +	status = "okay";
> +};
> +
> +&vin00 {
> +	status = "okay";
> +};
> +
> +&vin01 {
> +	status = "okay";
> +};
> +
> +&vin02 {
> +	status = "okay";
> +};
> +
> +&vin03 {
> +	status = "okay";
> +};
> +
> +&vin04 {
> +	status = "okay";
> +};
> +
> +&vin05 {
> +	status = "okay";
> +};
> +
> +&vin06 {
> +	status = "okay";
> +};
> +
> +&vin07 {
> +	status = "okay";
> +};

-- 
Regards,

Laurent Pinchart