Re: [PATCH v6 0/2] Pinefeat cef168 lens control board driver

[Alen Karnil <alen.karnil@ideasonboard.com>]

On 26/05/2026 21:35, Aliaksandr Smirnou wrote:
> On Tue, 26 May 2026 16:54:37 +0100, Alen Karnil wrote:
> 
>> I've been asked by Kieran Bingham to review your patches, I've
>> reproduced your work on setup with a Pi 5 with a 6.18 kernel and
>> I got a few questions
> 
> Hi Alen,
> 
> Thank you for resuming the work on the patch.
> 
>> I built the calibration application but it would not work out the
>> box, I can see that CEF168_V4L2_CID_CUSTOM is different between that
>> is in the patch and in the application? Which is the correct one,
>> does the  patch need updating?
> 
> The CEF168_V4L2_CID_CUSTOM value used in the patch is the correct one.
> 
> Although the repository currently defines it with a different value,
> this does not cause any issues for users because the driver is built
> and installed locally together with the calibration tool.
> 
> I can update the value in the cef168 repository at any time, so as long
> as users pull the latest version of the repository, everything will work
> correctly.
> 
> If the driver is eventually merged into the Linux kernel source tree,
> the repository will need to be updated accordingly anyway, because there
> will be no need to build the driver locally.
> 
>> The tool to modify the device tree also did not work with the
>> IMX477, I needed to manually modify my device tree to get the dtbo
>> to build,
> 
> I just double-checked the setup on a Raspberry Pi 5 running the latest
> 6.18 kernel from the stock Raspberry Pi OS, and everything works
> correctly. Are you using a customized Linux distribution?
> 
Strange, I've taken the latest RPi lite trxie OS, but I replaced the 
kernel from the rpi-6.18.y branch.

> Could you provide the build error logs and the overlay files generated
> by the tool before you modified them?
using the configure.sh tool with imx477
imx477_378-overlay.dtsi

```
// SPDX-License-Identifier: GPL-2.0-only
// Definitions for IMX477 camera module on VC I2C bus

/{
	compatible = "brcm,bcm2835";

	fragment@0 {
		target = <&i2c0if>;
		__overlay__ {
			status = "okay";
		};
	};

	clk_frag: fragment@1 {
		target = <&cam1_clk>;
		cam_clk: __overlay__ {
			clock-frequency = <24000000>;
			status = "okay";
		};
	};

	fragment@2 {
		target = <&i2c0mux>;
		__overlay__ {
			status = "okay";
		};
	};

	reg_frag: fragment@3 {
		target = <&cam1_reg>;
		cam_reg: __overlay__ {
			startup-delay-us = <300000>;
		};
	};

	fragment@4 {
		target = <&cam_node>;
		__overlay__ {
			lens-focus = <&vcm_node>;
		};
	};

	reg_alwayson_frag: fragment@99 {
		target = <&cam1_reg>;
		__dormant__ {
			regulator-always-on;
		};
	};

	i2c_frag: fragment@100 {
		target = <&i2c_csi_dsi>;
		__overlay__ {
			#address-cells = <1>;
			#size-cells = <0>;
			status = "okay";

			#include "imx477_378.dtsi"
		};
	};

	csi_frag: fragment@101 {
		target = <&csi1>;
		csi: __overlay__ {
			status = "okay";

			port {
				csi_ep: endpoint {
					remote-endpoint = <&cam_endpoint>;
					clock-lanes = <0>;
					data-lanes = <1 2>;
					clock-noncontinuous;
				};
			};
		};
	};

	fragment@102 {
		target = <&csi1>;
		__dormant__ {
			compatible = "brcm,bcm2835-unicam-legacy";
		};
	};

	__overrides__ {
		rotation = <&cam_node>,"rotation:0";
		orientation = <&cam_node>,"orientation:0";
		media-controller = <0>,"!102";
		cam0 = <&i2c_frag>, "target:0=",<&i2c_csi_dsi0>,
		       <&csi_frag>, "target:0=",<&csi0>,
		       <&clk_frag>, "target:0=",<&cam0_clk>,
		       <&reg_frag>, "target:0=",<&cam0_reg>,
		       <&reg_alwayson_frag>, "target:0=",<&cam0_reg>,
		       <&cam_node>, "clocks:0=",<&cam0_clk>,
		       <&cam_node>, "VANA-supply:0=",<&cam0_reg>;
		always-on = <0>, "+99";
		link-frequency = <&cam_endpoint>,"link-frequencies#0";
		vcm = <&vcm_node>, "status",
		      <0>, "=4";
	};
};

&cam_node {
	status = "okay";
};

&cam_endpoint {
	remote-endpoint = <&csi_ep>;
};

&vcm_node {
	status = "okay";
};
```

And imx477_378.dtsi

```
cam_node: imx477@1a {
	reg = <0x1a>;
	status = "disabled";

	clocks = <&cam1_clk>;
	clock-names = "xclk";

	VANA-supply = <&cam1_reg>;	/* 2.8v */
	VDIG-supply = <&cam_dummy_reg>;	/* 1.05v */
	VDDL-supply = <&cam_dummy_reg>;	/* 1.8v */

	rotation = <180>;
	orientation = <2>;

	port {
		cam_endpoint: endpoint {
			clock-lanes = <0>;
			data-lanes = <1 2>;
			clock-noncontinuous;
			link-frequencies =
				/bits/ 64 <450000000>;
		};
	};
};

vcm_node: cef168@d {
	compatible = "pinefeat,cef168";
	reg = <0x0d>;
	status = "disabled";
	vcc-supply = <&vdd_3v3_reg>;
};
```

If put these into my kernel build I get:

$ make dtbs
   DTCO     arch/arm64/boot/dts/overlays/camera-mux-2port.dtbo
arch/arm64/boot/dts/overlays/imx477_378.dtsi:26.20-31.3: ERROR 
(duplicate_label): /fragment@200/__overlay__/pca@70/i2c@1/cef168@d: 
Duplicate label 'vcm_node' on 
/fragment@200/__overlay__/pca@70/i2c@1/cef168@d and 
/fragment@200/__overlay__/pca@70/i2c@0/cef168@d
ERROR: Input tree has errors, aborting (use -f to force output)
make[3]: *** [scripts/Makefile.dtbs:142: 
arch/arm64/boot/dts/overlays/camera-mux-2port.dtbo] Error 2
make[2]: *** [scripts/Makefile.build:544: arch/arm64/boot/dts/overlays] 
Error 2
make[1]: *** [/home/admin/linux/Makefile:1498: dtbs] Error 2
make: *** [Makefile:248: __sub-make] Error 2

> 
> The modifications applied by the tool are shown in this diff:
> 
> https://gist.github.com/pinefeat/1b3a258a193754f073e171b1ed33a0cc
> 
> Were the generated files different in your setup?
I will include my working dts* for the imx477

imx477_378-overlay.dtsi
```
// SPDX-License-Identifier: GPL-2.0-only
// Definitions for IMX477 camera module on VC I2C bus

/{
	compatible = "brcm,bcm2835";

	fragment@0 {
		target = <&i2c0if>;
		__overlay__ {
			status = "okay";
		};
	};

	clk_frag: fragment@1 {
		target = <&cam1_clk>;
		cam_clk: __overlay__ {
			clock-frequency = <24000000>;
			status = "okay";
		};
	};

	fragment@2 {
		target = <&i2c0mux>;
		__overlay__ {
			status = "okay";
		};
	};

	reg_frag: fragment@3 {
		target = <&cam1_reg>;
		cam_reg: __overlay__ {
			startup-delay-us = <300000>;
		};
	};

	reg_alwayson_frag: fragment@99 {
		target = <&cam1_reg>;
		__dormant__ {
			regulator-always-on;
		};
	};

	i2c_frag: fragment@100 {
		target = <&i2c_csi_dsi>;
		__overlay__ {
			#address-cells = <1>;
			#size-cells = <0>;
			status = "okay";

			#include "imx477_378.dtsi"

			vcm: cef168@d {
				compatible = "pinefeat,cef168";
				reg = <0x0d>;
				status = "disabled";
				vcc-supply = <&vdd_3v3_reg>;
			};
		};
	};

	csi_frag: fragment@101 {
		target = <&csi1>;
		csi: __overlay__ {
			status = "okay";

			port {
				csi_ep: endpoint {
					remote-endpoint = <&cam_endpoint>;
					clock-lanes = <0>;
					data-lanes = <1 2>;
					clock-noncontinuous;
				};
			};
		};
	};

	fragment@102 {
		target = <&csi1>;
		__dormant__ {
			compatible = "brcm,bcm2835-unicam-legacy";
		};
	};

	__overrides__ {
		rotation = <&cam_node>,"rotation:0";
		orientation = <&cam_node>,"orientation:0";
		media-controller = <0>,"!102";
		cam0 = <&i2c_frag>, "target:0=",<&i2c_csi_dsi0>,
		       <&csi_frag>, "target:0=",<&csi0>,
		       <&clk_frag>, "target:0=",<&cam0_clk>,
		       <&reg_frag>, "target:0=",<&cam0_reg>,
		       <&reg_alwayson_frag>, "target:0=",<&cam0_reg>,
		       <&cam_node>, "clocks:0=",<&cam0_clk>,
		       <&cam_node>, "VANA-supply:0=",<&cam0_reg>;
		always-on = <0>, "+99";
		vcm = <&vcm>, "status=okay",
		      <&cam_node>,"lens-focus:0=", <&vcm>;
		link-frequency = <&cam_endpoint>,"link-frequencies#0";
	};

};

&cam_node {
	status = "okay";
};

&cam_endpoint {
	remote-endpoint = <&csi_ep>;
};
```

Also I needed to add `vcm` to my dtoverlay for the pi's config.txt
> 
>> there needs to be some further discussions on how we could
>> improve the user experience when it comes to enabling the vcm with
>> camera sensors.
> 
> I agree. I started the discussion on the Raspberry Pi forum, but the
> driver needs to be merged first before moving forward with that.
Sure that makes sense, lets focus on the driver for now.
> 
> https://forums.raspberrypi.com/viewtopic.php?p=2318070#p2318070

Also you will most likely need to update the patch since the line 
offsets have moved to work 6.18