Hi Laurent,
Thanks for your comments.
On 2025-07-07 15:34:09 +0300, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Fri, Jul 04, 2025 at 12:07:33PM +0200, Niklas Söderlund wrote:
> > Add an overlay to connect an IMX462 camera sensor to the J1 connector.
> > The IMX462 utilizes 4 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 v2
> > - Use the same regulator for all three supplies.
> > - Drop 'status = "okay"' property for the fixed clock.
> > - Drop 'status = "okay"' property for the sensor node.
> > - Drop unused label for sensor node.
> >
> > * Changes since v1
> > - Drop 'status = "okay"' property for the fixed regulators.
> > ---
> > arch/arm64/boot/dts/renesas/Makefile | 3 +
> > ...8a779g3-sparrow-hawk-camera-j1-imx462.dtso | 112 ++++++++++++++++++
> > 2 files changed, 115 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk-camera-j1-imx462.dtso
> >
> > diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> > index 12803c4fbc80..79d174077ddc 100644
> > --- a/arch/arm64/boot/dts/renesas/Makefile
> > +++ b/arch/arm64/boot/dts/renesas/Makefile
> > @@ -97,10 +97,13 @@ 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-camera-j1-imx462.dtbo
> > dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g3-sparrow-hawk-camera-j2-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-camera-j1-imx462-dtbs := r8a779g3-sparrow-hawk.dtb r8a779g3-sparrow-hawk-camera-j1-imx462.dtbo
> > +dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g3-sparrow-hawk-camera-j1-imx462.dtb
> > r8a779g3-sparrow-hawk-camera-j2-imx219-dtbs := r8a779g3-sparrow-hawk.dtb r8a779g3-sparrow-hawk-camera-j2-imx219.dtbo
> > dtb-$(CONFIG_ARCH_R8A779G0) += r8a779g3-sparrow-hawk-camera-j2-imx219.dtb
> > r8a779g3-sparrow-hawk-fan-pwm-dtbs := r8a779g3-sparrow-hawk.dtb r8a779g3-sparrow-hawk-fan-pwm.dtbo
> > diff --git a/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk-camera-j1-imx462.dtso b/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk-camera-j1-imx462.dtso
> > new file mode 100644
> > index 000000000000..63813ed216db
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk-camera-j1-imx462.dtso
> > @@ -0,0 +1,112 @@
> > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +/*
> > + * Device Tree Overlay for an IMX462 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>;
> > + };
> > +
> > + /* 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>;
> > + };
> > +};
> > +
> > +&i2c1 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + status = "okay";
> > +
> > + cam@1a {
> > + compatible = "sony,imx462lqr";
> > + reg = <0x1a>;
> > +
> > + clocks = <&clk_cam_j1>;
> > + clock-names = "xclk";
> > + clock-frequency = <37125000>;
>
> Usage of the clock-frequency property in camera sensors is deprecated.
> I'm working on patches that will fix that in the imx290 driver. In the
> meantime, you can apply this change for testing:
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index fbf7eba3d71d..6a6cf37d62f9 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -1426,14 +1426,8 @@ static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
> static int imx290_init_clk(struct imx290 *imx290)
> {
> u32 xclk_freq;
> - int ret;
>
> - ret = device_property_read_u32(imx290->dev, "clock-frequency",
> - &xclk_freq);
> - if (ret) {
> - dev_err(imx290->dev, "Could not get xclk frequency\n");
> - return ret;
> - }
> + xclk_freq = clk_get_rate(imx290->xclk);
>
> /* external clock must be 37.125 MHz or 74.25MHz */
> switch (xclk_freq) {
> @@ -1449,12 +1443,6 @@ static int imx290_init_clk(struct imx290 *imx290)
> return -EINVAL;
> }
>
> - ret = clk_set_rate(imx290->xclk, xclk_freq);
> - if (ret) {
> - dev_err(imx290->dev, "Could not set xclk frequency\n");
> - return ret;
> - }
> -
> return 0;
> }
>
>
> If you need to set the external clock rate, the assigned-clocks and
> assigned-clock-rates properties should be used instead. In this specific
> case, the external clock is a fixed frequency clock, so you can't change
> its rate.
Wit this change applied, correct clock in clk_cam_j1, and the
clock-frequency popery removed the camera still works. But of course the
bindings verification fails as clock-frequency is a mandatory property.
> This leads to a second comment: the clock-frequency property
> of the clk_cam_j1 node doesn't match the frequency you list here.
Woops, bad copy-past, fixed. Thanks for spotting.
>
> These comments apply to patch 5/5.
>
> > +
> > + vdddo-supply = <®_cam_j1>;
> > + vdda-supply = <®_cam_j1>;
> > + vddd-supply = <®_cam_j1>;
> > +
> > + orientation = <2>;
> > + rotation = <0>;
> > +
> > + port {
> > + imx462_j1_out: endpoint {
> > + link-frequencies = /bits/ 64 <222750000 148500000>;
> > + data-lanes = <1 2 3 4>;
> > + 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 3 4>;
> > + remote-endpoint = <&imx462_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
--
Kind Regards,
Niklas Söderlund
