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

From: Alen Karnil

Date: Wed May 27 2026 - 07:51:57 EST




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