[PATCH v5 0/2] Pinefeat cef168 lens control board driver

[PATCH v5 0/2] Pinefeat cef168 lens control board driver

[Aliaksandr Smirnou]

This patch series adds support for the Pinefeat adapter, which interfaces
Canon EF and EF-S lenses to non-Canon camera bodies. The cef168 circuit
control board provides an I2C interface for electronic focus and aperture
control. The driver integrates with the V4L2 sub-device API.

For more information about the product, see:
https://github.com/pinefeat/cef168

Changes in v5:
 - rebased onto the latest master branch;

Changes in v4:
 - removed driver mention from the hardware documentation;
 - added named email in commit signed-off-by;
 - added select CRC8 in Kconfig;
 - removed header file;
 - moved variable declaration at the beginning of the function;
 - removed kerneldoc from structures;
 - removed control id check as the core handles this for us;
 - removed Power Management Runtime (pm_runtime_*) calls as redundant;
 - reserved v4l2 controls in linux header file;

Link to v4: https://lore.kernel.org/all/20250830111500.53169-1-asmirnou@pinefeat.co.uk/
Link to v3: https://lore.kernel.org/all/20250817130549.7766-1-support@pinefeat.co.uk/

Patches:
  dt-bindings: Pinefeat cef168 lens control board
  media: i2c: Pinefeat cef168 lens control board driver

 .../bindings/media/i2c/pinefeat,cef168.yaml   |  47 +++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 drivers/media/i2c/Kconfig                     |   9 +
 drivers/media/i2c/Makefile                    |   1 +
 drivers/media/i2c/cef168.c                    | 331 ++++++++++++++++++
 include/uapi/linux/v4l2-controls.h            |   6 +
 7 files changed, 403 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
 create mode 100644 drivers/media/i2c/cef168.c


base-commit: 6093a688a07da07808f0122f9aa2a3eed250d853
-- 
2.34.1

[PATCH v5 1/2] dt-bindings: Pinefeat cef168 lens control board

[Aliaksandr Smirnou]

Add the Device Tree schema and examples for the Pinefeat cef168 lens
control board. This board interfaces Canon EF & EF-S lenses with
non-Canon camera bodies, enabling electronic control of focus and
aperture via V4L2.

Power supply is derived from fixed supplies via connector or GPIO
header. Therefore, the driver does not manage any regulator, so
representing any supply in the binding is redundant.

Signed-off-by: Aliaksandr Smirnou <asmirnou@pinefeat.co.uk>
---
 .../bindings/media/i2c/pinefeat,cef168.yaml   | 47 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 MAINTAINERS                                   |  6 +++
 3 files changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml b/Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
new file mode 100644
index 000000000000..1295b1f4edeb
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2025 Pinefeat LLP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/pinefeat,cef168.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Pinefeat cef168 lens driver
+
+maintainers:
+  - Aliaksandr Smirnou <support@pinefeat.co.uk>
+
+description: |
+  Pinefeat produces an adapter designed to interface between
+  Canon EF & EF-S lenses and non-Canon camera bodies, incorporating
+  features for electronic focus and aperture adjustment. The cef168
+  circuit board, included with the adapter, provides a software
+  programming interface that allows control of lens focus and
+  aperture positions.
+
+properties:
+  compatible:
+    enum:
+      - pinefeat,cef168
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        camera-lens@d {
+            compatible = "pinefeat,cef168";
+            reg = <0x0d>;
+        };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index f1d1882009ba..4f50c35ed670 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1241,6 +1241,8 @@ patternProperties:
     description: Picochip Ltd
   "^pine64,.*":
     description: Pine64
+  "^pinefeat,.*":
+    description: Pinefeat LLP
   "^pineriver,.*":
     description: Shenzhen PineRiver Designs Co., Ltd.
   "^pixcir,.*":
diff --git a/MAINTAINERS b/MAINTAINERS
index 5a2cde903910..a59cd27caf11 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20332,6 +20332,12 @@ S:	Supported
 F:	Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml
 F:	drivers/input/keyboard/pinephone-keyboard.c
 
+PINEFEAT CEF168 LENS DRIVER
+M:	Aliaksandr Smirnou <support@pinefeat.co.uk>
+L:	linux-media@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
+
 PLANTOWER PMS7003 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <tduszyns@gmail.com>
 S:	Maintained
-- 
2.34.1

Re: [PATCH v5 1/2] dt-bindings: Pinefeat cef168 lens control board

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

On Sun, Oct 05, 2025 at 02:32:27PM +0100, Aliaksandr Smirnou wrote:
> Add the Device Tree schema and examples for the Pinefeat cef168 lens
> control board. This board interfaces Canon EF & EF-S lenses with
> non-Canon camera bodies, enabling electronic control of focus and
> aperture via V4L2.
> 
> Power supply is derived from fixed supplies via connector or GPIO
> header. Therefore, the driver does not manage any regulator, so
> representing any supply in the binding is redundant.
> 
> Signed-off-by: Aliaksandr Smirnou <asmirnou@pinefeat.co.uk>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v5 2/2] media: i2c: Pinefeat cef168 lens control board driver

[Aliaksandr Smirnou]

Add support for the Pinefeat cef168 lens control board that provides
electronic focus and aperture control for Canon EF & EF-S lenses on
non-Canon camera bodies.

Signed-off-by: Aliaksandr Smirnou <asmirnou@pinefeat.co.uk>
---
 MAINTAINERS                        |   1 +
 drivers/media/i2c/Kconfig          |   9 +
 drivers/media/i2c/Makefile         |   1 +
 drivers/media/i2c/cef168.c         | 331 +++++++++++++++++++++++++++++
 include/uapi/linux/v4l2-controls.h |   6 +
 5 files changed, 348 insertions(+)
 create mode 100644 drivers/media/i2c/cef168.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a59cd27caf11..0cf3b3a35827 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20337,6 +20337,7 @@ M:	Aliaksandr Smirnou <support@pinefeat.co.uk>
 L:	linux-media@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
+F:	drivers/media/i2c/cef168.c
 
 PLANTOWER PMS7003 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <tduszyns@gmail.com>
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cdd7ba5da0d5..694b2571de37 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -823,6 +823,15 @@ config VIDEO_AK7375
 	  capability. This is designed for linear control of
 	  voice coil motors, controlled via I2C serial interface.
 
+config VIDEO_CEF168
+	tristate "CEF168 lens control support"
+	select CRC8
+	help
+	  This is a driver for the CEF168 lens control board.
+	  The board provides an I2C interface for electronic focus
+	  and aperture control of EF and EF-S lenses. The driver
+	  integrates with the V4L2 sub-device API.
+
 config VIDEO_DW9714
 	tristate "DW9714 lens voice coil support"
 	depends on GPIOLIB
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 57cdd8dc96f6..2e8f0a968352 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_VIDEO_BT856) += bt856.o
 obj-$(CONFIG_VIDEO_BT866) += bt866.o
 obj-$(CONFIG_VIDEO_CCS) += ccs/
 obj-$(CONFIG_VIDEO_CCS_PLL) += ccs-pll.o
+obj-$(CONFIG_VIDEO_CEF168) += cef168.o
 obj-$(CONFIG_VIDEO_CS3308) += cs3308.o
 obj-$(CONFIG_VIDEO_CS5345) += cs5345.o
 obj-$(CONFIG_VIDEO_CS53L32A) += cs53l32a.o
diff --git a/drivers/media/i2c/cef168.c b/drivers/media/i2c/cef168.c
new file mode 100644
index 000000000000..cfcef476f09d
--- /dev/null
+++ b/drivers/media/i2c/cef168.c
@@ -0,0 +1,331 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2025 Pinefeat LLP
+
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/v4l2-controls.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+
+#define CEF168_NAME "cef168"
+
+#define CEF168_V4L2_CID_CUSTOM(ctrl) \
+	(V4L2_CID_USER_CEF168_BASE + custom_##ctrl)
+
+enum { custom_lens_id, custom_data, custom_focus_range, custom_calibrate };
+
+#define INP_CALIBRATE 0x22
+#define INP_SET_FOCUS 0x80
+#define INP_SET_FOCUS_P 0x81
+#define INP_SET_FOCUS_N 0x82
+#define INP_SET_APERTURE 0x7A
+#define INP_SET_APERTURE_P 0x7B
+#define INP_SET_APERTURE_N 0x7C
+
+#define CEF_CRC8_POLYNOMIAL 168
+
+DECLARE_CRC8_TABLE(cef168_crc8_table);
+
+struct cef168_data {
+	__u8 lens_id;
+	__u8 moving : 1;
+	__u8 calibrating : 2;
+	__u16 moving_time;
+	__u16 focus_position_min;
+	__u16 focus_position_max;
+	__u16 focus_position_cur;
+	__u16 focus_distance_min;
+	__u16 focus_distance_max;
+	__u8 crc8;
+} __packed;
+
+struct cef168_device {
+	struct v4l2_ctrl_handler ctrls;
+	struct v4l2_subdev sd;
+};
+
+static inline struct cef168_device *to_cef168(struct v4l2_ctrl *ctrl)
+{
+	return container_of(ctrl->handler, struct cef168_device, ctrls);
+}
+
+static inline struct cef168_device *sd_to_cef168(struct v4l2_subdev *subdev)
+{
+	return container_of(subdev, struct cef168_device, sd);
+}
+
+static int cef168_i2c_write(struct cef168_device *cef168_dev, u8 cmd, u16 val)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&cef168_dev->sd);
+	int retry, ret;
+
+	__le16 le_data = cpu_to_le16(val);
+	char tx_data[4] = { cmd, ((u8 *)&le_data)[0], ((u8 *)&le_data)[1] };
+
+	tx_data[3] = crc8(cef168_crc8_table, tx_data, 3, CRC8_INIT_VALUE);
+
+	for (retry = 0; retry < 3; retry++) {
+		ret = i2c_master_send(client, tx_data, sizeof(tx_data));
+		if (ret == sizeof(tx_data))
+			return 0;
+		else if (ret != -EIO && ret != -EREMOTEIO)
+			break;
+	}
+
+	dev_err(&client->dev, "I2C write fail after %d retries, ret=%d\n",
+		retry, ret);
+	return -EIO;
+}
+
+static int cef168_i2c_read(struct cef168_device *cef168_dev,
+			   struct cef168_data *rx_data)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&cef168_dev->sd);
+
+	int ret = i2c_master_recv(client, (char *)rx_data,
+				  sizeof(struct cef168_data));
+	if (ret != sizeof(struct cef168_data)) {
+		dev_err(&client->dev, "I2C read fail, ret=%d\n", ret);
+		return -EIO;
+	}
+
+	u8 computed_crc = crc8(cef168_crc8_table, (const u8 *)rx_data,
+			       sizeof(struct cef168_data) - 1, CRC8_INIT_VALUE);
+	if (computed_crc != rx_data->crc8) {
+		dev_err(&client->dev,
+			"CRC mismatch calculated=0x%02X read=0x%02X\n",
+			computed_crc, rx_data->crc8);
+		return -EIO;
+	}
+
+	rx_data->moving_time = le16_to_cpup((__le16 *)&rx_data->moving_time);
+	rx_data->focus_position_min = le16_to_cpup((__le16 *)&rx_data->focus_position_min);
+	rx_data->focus_position_max = le16_to_cpup((__le16 *)&rx_data->focus_position_max);
+	rx_data->focus_position_cur = le16_to_cpup((__le16 *)&rx_data->focus_position_cur);
+	rx_data->focus_distance_min = le16_to_cpup((__le16 *)&rx_data->focus_distance_min);
+	rx_data->focus_distance_max = le16_to_cpup((__le16 *)&rx_data->focus_distance_max);
+
+	return 0;
+}
+
+static int cef168_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct cef168_device *dev = to_cef168(ctrl);
+	u8 cmd;
+
+	switch (ctrl->id) {
+	case V4L2_CID_FOCUS_ABSOLUTE:
+		return cef168_i2c_write(dev, INP_SET_FOCUS, ctrl->val);
+	case V4L2_CID_FOCUS_RELATIVE:
+		cmd = ctrl->val < 0 ? INP_SET_FOCUS_N : INP_SET_FOCUS_P;
+		return cef168_i2c_write(dev, cmd, abs(ctrl->val));
+	case V4L2_CID_IRIS_ABSOLUTE:
+		return cef168_i2c_write(dev, INP_SET_APERTURE, ctrl->val);
+	case V4L2_CID_IRIS_RELATIVE:
+		cmd = ctrl->val < 0 ? INP_SET_APERTURE_N : INP_SET_APERTURE_P;
+		return cef168_i2c_write(dev, cmd, abs(ctrl->val));
+	case CEF168_V4L2_CID_CUSTOM(calibrate):
+		return cef168_i2c_write(dev, INP_CALIBRATE, 0);
+	}
+
+	return -EINVAL;
+}
+
+static int cef168_get_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct cef168_data data;
+	struct cef168_device *dev = to_cef168(ctrl);
+	int rval;
+
+	rval = cef168_i2c_read(dev, &data);
+	if (rval < 0)
+		return rval;
+
+	switch (ctrl->id) {
+	case V4L2_CID_FOCUS_ABSOLUTE:
+		ctrl->val = data.focus_position_cur;
+		return 0;
+	case CEF168_V4L2_CID_CUSTOM(focus_range):
+		ctrl->p_new.p_u32[0] = ((u32)data.focus_position_min << 16) |
+				       (u32)data.focus_position_max;
+		return 0;
+	case CEF168_V4L2_CID_CUSTOM(lens_id):
+		ctrl->p_new.p_u8[0] = data.lens_id;
+		return 0;
+	case CEF168_V4L2_CID_CUSTOM(data):
+		memcpy(ctrl->p_new.p_u8, &data, sizeof(data));
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static const struct v4l2_ctrl_ops cef168_ctrl_ops = {
+	.g_volatile_ctrl = cef168_get_ctrl,
+	.s_ctrl = cef168_set_ctrl,
+};
+
+static const struct v4l2_ctrl_config cef168_lens_id_ctrl = {
+	.ops = &cef168_ctrl_ops,
+	.id = CEF168_V4L2_CID_CUSTOM(lens_id),
+	.type = V4L2_CTRL_TYPE_U8,
+	.name = "Lens ID",
+	.min = 0,
+	.max = U8_MAX,
+	.step = 1,
+	.def = 0,
+	.flags = V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY,
+};
+
+static const struct v4l2_ctrl_config cef168_focus_range_ctrl = {
+	.ops = &cef168_ctrl_ops,
+	.id = CEF168_V4L2_CID_CUSTOM(focus_range),
+	.type = V4L2_CTRL_TYPE_U32,
+	.name = "Focus Range",
+	.min = 0,
+	.max = U32_MAX,
+	.step = 1,
+	.def = 0,
+	.flags = V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY,
+};
+
+static const struct v4l2_ctrl_config cef168_data_ctrl = {
+	.ops = &cef168_ctrl_ops,
+	.id = CEF168_V4L2_CID_CUSTOM(data),
+	.type = V4L2_CTRL_TYPE_U8,
+	.name = "Data",
+	.min = 0,
+	.max = U8_MAX,
+	.step = 1,
+	.def = 0,
+	.dims = { sizeof(struct cef168_data) / sizeof(u8) },
+	.elem_size = sizeof(u8),
+	.flags = V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY,
+};
+
+static const struct v4l2_ctrl_config cef168_calibrate_ctrl = {
+	.ops = &cef168_ctrl_ops,
+	.id = CEF168_V4L2_CID_CUSTOM(calibrate),
+	.type = V4L2_CTRL_TYPE_BUTTON,
+	.name = "Calibrate",
+};
+
+static const struct v4l2_subdev_core_ops cef168_core_ops = {
+	.log_status = v4l2_ctrl_subdev_log_status,
+	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
+static const struct v4l2_subdev_ops cef168_ops = {
+	.core = &cef168_core_ops,
+};
+
+static int cef168_init_controls(struct cef168_device *dev)
+{
+	struct v4l2_ctrl *ctrl;
+	struct v4l2_ctrl_handler *hdl = &dev->ctrls;
+	const struct v4l2_ctrl_ops *ops = &cef168_ctrl_ops;
+
+	v4l2_ctrl_handler_init(hdl, 8);
+
+	ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE, 0, S16_MAX,
+				 1, 0);
+	if (ctrl)
+		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
+			       V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
+	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_RELATIVE, S16_MIN, S16_MAX,
+			  1, 0);
+	ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_IRIS_ABSOLUTE, 0, S16_MAX,
+				 1, 0);
+	if (ctrl)
+		ctrl->flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
+			       V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
+	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_IRIS_RELATIVE, S16_MIN, S16_MAX, 1,
+			  0);
+	ctrl = v4l2_ctrl_new_custom(hdl, &cef168_calibrate_ctrl, NULL);
+	if (ctrl)
+		ctrl->flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
+			       V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
+	v4l2_ctrl_new_custom(hdl, &cef168_focus_range_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &cef168_data_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &cef168_lens_id_ctrl, NULL);
+
+	if (hdl->error)
+		dev_err(dev->sd.dev, "%s fail error: 0x%x\n", __func__,
+			hdl->error);
+	dev->sd.ctrl_handler = hdl;
+	return hdl->error;
+}
+
+static int cef168_probe(struct i2c_client *client)
+{
+	struct cef168_device *cef168_dev;
+	int rval;
+
+	cef168_dev = devm_kzalloc(&client->dev, sizeof(*cef168_dev),
+				  GFP_KERNEL);
+	if (!cef168_dev)
+		return -ENOMEM;
+
+	v4l2_i2c_subdev_init(&cef168_dev->sd, client, &cef168_ops);
+	cef168_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
+				V4L2_SUBDEV_FL_HAS_EVENTS;
+
+	rval = cef168_init_controls(cef168_dev);
+	if (rval)
+		goto err_cleanup;
+
+	rval = media_entity_pads_init(&cef168_dev->sd.entity, 0, NULL);
+	if (rval < 0)
+		goto err_cleanup;
+
+	cef168_dev->sd.entity.function = MEDIA_ENT_F_LENS;
+
+	rval = v4l2_async_register_subdev(&cef168_dev->sd);
+	if (rval < 0)
+		goto err_cleanup;
+
+	crc8_populate_msb(cef168_crc8_table, CEF_CRC8_POLYNOMIAL);
+
+	return 0;
+
+err_cleanup:
+	v4l2_ctrl_handler_free(&cef168_dev->ctrls);
+	media_entity_cleanup(&cef168_dev->sd.entity);
+
+	return rval;
+}
+
+static void cef168_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct cef168_device *cef168_dev = sd_to_cef168(sd);
+
+	v4l2_async_unregister_subdev(&cef168_dev->sd);
+	v4l2_ctrl_handler_free(&cef168_dev->ctrls);
+	media_entity_cleanup(&cef168_dev->sd.entity);
+}
+
+static const struct of_device_id cef168_of_table[] = {
+	{ .compatible = "pinefeat,cef168" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, cef168_of_table);
+
+static struct i2c_driver cef168_i2c_driver = {
+	.driver = {
+		.name = CEF168_NAME,
+		.of_match_table = cef168_of_table,
+	},
+	.probe = cef168_probe,
+	.remove = cef168_remove,
+};
+
+module_i2c_driver(cef168_i2c_driver);
+
+MODULE_AUTHOR("support@pinefeat.co.uk>");
+MODULE_DESCRIPTION("CEF168 lens driver");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 2d30107e047e..f8ca4f8c89af 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -228,6 +228,12 @@ enum v4l2_colorfx {
  */
 #define V4L2_CID_USER_RKISP1_BASE		(V4L2_CID_USER_BASE + 0x1220)
 
+/*
+ * The base for Pinefeat CEF168 driver controls.
+ * We reserve 16 controls for this driver.
+ */
+#define V4L2_CID_USER_CEF168_BASE		(V4L2_CID_USER_BASE + 0x1230)
+
 /* MPEG-class control IDs */
 /* The MPEG controls are applicable to all codec controls
  * and the 'MPEG' part of the define is historical */
-- 
2.34.1

Re: [PATCH v5 2/2] media: i2c: Pinefeat cef168 lens control board driver

[Kieran Bingham]

Hi Aliaksandr,

Quoting Aliaksandr Smirnou (2025-10-05 14:32:28)
> Add support for the Pinefeat cef168 lens control board that provides
> electronic focus and aperture control for Canon EF & EF-S lenses on
> non-Canon camera bodies.

I'm really looking forward to trying this device out sometime. Very
interesting piece of kit - Thank you for working directly to upstream
the support! That's really awesome.

> Signed-off-by: Aliaksandr Smirnou <asmirnou@pinefeat.co.uk>
> ---
>  MAINTAINERS                        |   1 +
>  drivers/media/i2c/Kconfig          |   9 +
>  drivers/media/i2c/Makefile         |   1 +
>  drivers/media/i2c/cef168.c         | 331 +++++++++++++++++++++++++++++
>  include/uapi/linux/v4l2-controls.h |   6 +
>  5 files changed, 348 insertions(+)
>  create mode 100644 drivers/media/i2c/cef168.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a59cd27caf11..0cf3b3a35827 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20337,6 +20337,7 @@ M:      Aliaksandr Smirnou <support@pinefeat.co.uk>
>  L:     linux-media@vger.kernel.org
>  S:     Supported
>  F:     Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
> +F:     drivers/media/i2c/cef168.c
>  
>  PLANTOWER PMS7003 AIR POLLUTION SENSOR DRIVER
>  M:     Tomasz Duszynski <tduszyns@gmail.com>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index cdd7ba5da0d5..694b2571de37 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -823,6 +823,15 @@ config VIDEO_AK7375
>           capability. This is designed for linear control of
>           voice coil motors, controlled via I2C serial interface.
>  
> +config VIDEO_CEF168
> +       tristate "CEF168 lens control support"
> +       select CRC8
> +       help
> +         This is a driver for the CEF168 lens control board.
> +         The board provides an I2C interface for electronic focus
> +         and aperture control of EF and EF-S lenses. The driver
> +         integrates with the V4L2 sub-device API.
> +
>  config VIDEO_DW9714
>         tristate "DW9714 lens voice coil support"
>         depends on GPIOLIB
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 57cdd8dc96f6..2e8f0a968352 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_VIDEO_BT856) += bt856.o
>  obj-$(CONFIG_VIDEO_BT866) += bt866.o
>  obj-$(CONFIG_VIDEO_CCS) += ccs/
>  obj-$(CONFIG_VIDEO_CCS_PLL) += ccs-pll.o
> +obj-$(CONFIG_VIDEO_CEF168) += cef168.o
>  obj-$(CONFIG_VIDEO_CS3308) += cs3308.o
>  obj-$(CONFIG_VIDEO_CS5345) += cs5345.o
>  obj-$(CONFIG_VIDEO_CS53L32A) += cs53l32a.o
> diff --git a/drivers/media/i2c/cef168.c b/drivers/media/i2c/cef168.c
> new file mode 100644
> index 000000000000..cfcef476f09d
> --- /dev/null
> +++ b/drivers/media/i2c/cef168.c
> @@ -0,0 +1,331 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2025 Pinefeat LLP
> +
> +#include <linux/crc8.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/v4l2-controls.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-event.h>
> +
> +#define CEF168_NAME "cef168"
> +
> +#define CEF168_V4L2_CID_CUSTOM(ctrl) \
> +       (V4L2_CID_USER_CEF168_BASE + custom_##ctrl)
> +
> +enum { custom_lens_id, custom_data, custom_focus_range, custom_calibrate };
> +
> +#define INP_CALIBRATE 0x22
> +#define INP_SET_FOCUS 0x80
> +#define INP_SET_FOCUS_P 0x81
> +#define INP_SET_FOCUS_N 0x82
> +#define INP_SET_APERTURE 0x7A
> +#define INP_SET_APERTURE_P 0x7B
> +#define INP_SET_APERTURE_N 0x7C
> +
> +#define CEF_CRC8_POLYNOMIAL 168
> +
> +DECLARE_CRC8_TABLE(cef168_crc8_table);
> +
> +struct cef168_data {
> +       __u8 lens_id;
> +       __u8 moving : 1;
> +       __u8 calibrating : 2;
> +       __u16 moving_time;
> +       __u16 focus_position_min;
> +       __u16 focus_position_max;
> +       __u16 focus_position_cur;
> +       __u16 focus_distance_min;
> +       __u16 focus_distance_max;
> +       __u8 crc8;
> +} __packed;
> +
> +struct cef168_device {
> +       struct v4l2_ctrl_handler ctrls;
> +       struct v4l2_subdev sd;
> +};
> +
> +static inline struct cef168_device *to_cef168(struct v4l2_ctrl *ctrl)
> +{
> +       return container_of(ctrl->handler, struct cef168_device, ctrls);
> +}
> +
> +static inline struct cef168_device *sd_to_cef168(struct v4l2_subdev *subdev)
> +{
> +       return container_of(subdev, struct cef168_device, sd);
> +}
> +
> +static int cef168_i2c_write(struct cef168_device *cef168_dev, u8 cmd, u16 val)
> +{
> +       struct i2c_client *client = v4l2_get_subdevdata(&cef168_dev->sd);
> +       int retry, ret;
> +
> +       __le16 le_data = cpu_to_le16(val);
> +       char tx_data[4] = { cmd, ((u8 *)&le_data)[0], ((u8 *)&le_data)[1] };
> +
> +       tx_data[3] = crc8(cef168_crc8_table, tx_data, 3, CRC8_INIT_VALUE);
> +
> +       for (retry = 0; retry < 3; retry++) {
> +               ret = i2c_master_send(client, tx_data, sizeof(tx_data));
> +               if (ret == sizeof(tx_data))
> +                       return 0;
> +               else if (ret != -EIO && ret != -EREMOTEIO)
> +                       break;
> +       }
> +
> +       dev_err(&client->dev, "I2C write fail after %d retries, ret=%d\n",
> +               retry, ret);
> +       return -EIO;
> +}
> +
> +static int cef168_i2c_read(struct cef168_device *cef168_dev,
> +                          struct cef168_data *rx_data)
> +{
> +       struct i2c_client *client = v4l2_get_subdevdata(&cef168_dev->sd);
> +
> +       int ret = i2c_master_recv(client, (char *)rx_data,
> +                                 sizeof(struct cef168_data));
> +       if (ret != sizeof(struct cef168_data)) {
> +               dev_err(&client->dev, "I2C read fail, ret=%d\n", ret);
> +               return -EIO;
> +       }
> +
> +       u8 computed_crc = crc8(cef168_crc8_table, (const u8 *)rx_data,
> +                              sizeof(struct cef168_data) - 1, CRC8_INIT_VALUE);
> +       if (computed_crc != rx_data->crc8) {
> +               dev_err(&client->dev,
> +                       "CRC mismatch calculated=0x%02X read=0x%02X\n",
> +                       computed_crc, rx_data->crc8);
> +               return -EIO;
> +       }
> +
> +       rx_data->moving_time = le16_to_cpup((__le16 *)&rx_data->moving_time);
> +       rx_data->focus_position_min = le16_to_cpup((__le16 *)&rx_data->focus_position_min);
> +       rx_data->focus_position_max = le16_to_cpup((__le16 *)&rx_data->focus_position_max);
> +       rx_data->focus_position_cur = le16_to_cpup((__le16 *)&rx_data->focus_position_cur);
> +       rx_data->focus_distance_min = le16_to_cpup((__le16 *)&rx_data->focus_distance_min);
> +       rx_data->focus_distance_max = le16_to_cpup((__le16 *)&rx_data->focus_distance_max);

What is the focus distance in this case? Is it a measured distance that
could be applied to focus?

> +
> +       return 0;
> +}
> +
> +static int cef168_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +       struct cef168_device *dev = to_cef168(ctrl);
> +       u8 cmd;
> +
> +       switch (ctrl->id) {
> +       case V4L2_CID_FOCUS_ABSOLUTE:
> +               return cef168_i2c_write(dev, INP_SET_FOCUS, ctrl->val);
> +       case V4L2_CID_FOCUS_RELATIVE:
> +               cmd = ctrl->val < 0 ? INP_SET_FOCUS_N : INP_SET_FOCUS_P;
> +               return cef168_i2c_write(dev, cmd, abs(ctrl->val));
> +       case V4L2_CID_IRIS_ABSOLUTE:
> +               return cef168_i2c_write(dev, INP_SET_APERTURE, ctrl->val);
> +       case V4L2_CID_IRIS_RELATIVE:
> +               cmd = ctrl->val < 0 ? INP_SET_APERTURE_N : INP_SET_APERTURE_P;
> +               return cef168_i2c_write(dev, cmd, abs(ctrl->val));
> +       case CEF168_V4L2_CID_CUSTOM(calibrate):
> +               return cef168_i2c_write(dev, INP_CALIBRATE, 0);

Is there any documentation on how to use this control?

> +       }
> +
> +       return -EINVAL;
> +}
> +
> +static int cef168_get_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +       struct cef168_data data;
> +       struct cef168_device *dev = to_cef168(ctrl);
> +       int rval;
> +
> +       rval = cef168_i2c_read(dev, &data);
> +       if (rval < 0)
> +               return rval;
> +
> +       switch (ctrl->id) {
> +       case V4L2_CID_FOCUS_ABSOLUTE:
> +               ctrl->val = data.focus_position_cur;
> +               return 0;
> +       case CEF168_V4L2_CID_CUSTOM(focus_range):
> +               ctrl->p_new.p_u32[0] = ((u32)data.focus_position_min << 16) |
> +                                      (u32)data.focus_position_max;

Is this really a custom control ? Is there any way to convey this
through the min/max of the FOCUS_ABSOLUTE control ?

> +               return 0;
> +       case CEF168_V4L2_CID_CUSTOM(lens_id):
> +               ctrl->p_new.p_u8[0] = data.lens_id;
> +               return 0;

Is this a specific individual ID value for the connected lens (i.e.
every lens has a custom id?) or is it a reference to the lens
model/type?

Is this something we could use in libcamera for instance to select an
appropriate tuning file per lens (type) ?


> +       case CEF168_V4L2_CID_CUSTOM(data):
> +               memcpy(ctrl->p_new.p_u8, &data, sizeof(data));
> +               return 0;
> +       }
> +
> +       return -EINVAL;
> +}
> +
> +static const struct v4l2_ctrl_ops cef168_ctrl_ops = {
> +       .g_volatile_ctrl = cef168_get_ctrl,
> +       .s_ctrl = cef168_set_ctrl,
> +};
> +
> +static const struct v4l2_ctrl_config cef168_lens_id_ctrl = {
> +       .ops = &cef168_ctrl_ops,
> +       .id = CEF168_V4L2_CID_CUSTOM(lens_id),
> +       .type = V4L2_CTRL_TYPE_U8,
> +       .name = "Lens ID",
> +       .min = 0,
> +       .max = U8_MAX,
> +       .step = 1,
> +       .def = 0,
> +       .flags = V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY,
> +};
> +
> +static const struct v4l2_ctrl_config cef168_focus_range_ctrl = {
> +       .ops = &cef168_ctrl_ops,
> +       .id = CEF168_V4L2_CID_CUSTOM(focus_range),
> +       .type = V4L2_CTRL_TYPE_U32,
> +       .name = "Focus Range",
> +       .min = 0,
> +       .max = U32_MAX,
> +       .step = 1,
> +       .def = 0,
> +       .flags = V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY,
> +};
> +
> +static const struct v4l2_ctrl_config cef168_data_ctrl = {
> +       .ops = &cef168_ctrl_ops,
> +       .id = CEF168_V4L2_CID_CUSTOM(data),
> +       .type = V4L2_CTRL_TYPE_U8,
> +       .name = "Data",
> +       .min = 0,
> +       .max = U8_MAX,
> +       .step = 1,
> +       .def = 0,
> +       .dims = { sizeof(struct cef168_data) / sizeof(u8) },
> +       .elem_size = sizeof(u8),
> +       .flags = V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY,
> +};
> +
> +static const struct v4l2_ctrl_config cef168_calibrate_ctrl = {
> +       .ops = &cef168_ctrl_ops,
> +       .id = CEF168_V4L2_CID_CUSTOM(calibrate),
> +       .type = V4L2_CTRL_TYPE_BUTTON,
> +       .name = "Calibrate",
> +};
> +
> +static const struct v4l2_subdev_core_ops cef168_core_ops = {
> +       .log_status = v4l2_ctrl_subdev_log_status,
> +       .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> +       .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
> +
> +static const struct v4l2_subdev_ops cef168_ops = {
> +       .core = &cef168_core_ops,
> +};
> +
> +static int cef168_init_controls(struct cef168_device *dev)
> +{
> +       struct v4l2_ctrl *ctrl;
> +       struct v4l2_ctrl_handler *hdl = &dev->ctrls;
> +       const struct v4l2_ctrl_ops *ops = &cef168_ctrl_ops;
> +
> +       v4l2_ctrl_handler_init(hdl, 8);
> +
> +       ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE, 0, S16_MAX,
> +                                1, 0);
> +       if (ctrl)
> +               ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
> +                              V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
> +       v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_RELATIVE, S16_MIN, S16_MAX,
> +                         1, 0);
> +       ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_IRIS_ABSOLUTE, 0, S16_MAX,
> +                                1, 0);
> +       if (ctrl)
> +               ctrl->flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
> +                              V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
> +       v4l2_ctrl_new_std(hdl, ops, V4L2_CID_IRIS_RELATIVE, S16_MIN, S16_MAX, 1,
> +                         0);
> +       ctrl = v4l2_ctrl_new_custom(hdl, &cef168_calibrate_ctrl, NULL);
> +       if (ctrl)
> +               ctrl->flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
> +                              V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
> +       v4l2_ctrl_new_custom(hdl, &cef168_focus_range_ctrl, NULL);
> +       v4l2_ctrl_new_custom(hdl, &cef168_data_ctrl, NULL);
> +       v4l2_ctrl_new_custom(hdl, &cef168_lens_id_ctrl, NULL);
> +
> +       if (hdl->error)
> +               dev_err(dev->sd.dev, "%s fail error: 0x%x\n", __func__,
> +                       hdl->error);
> +       dev->sd.ctrl_handler = hdl;
> +       return hdl->error;
> +}
> +
> +static int cef168_probe(struct i2c_client *client)
> +{
> +       struct cef168_device *cef168_dev;
> +       int rval;
> +
> +       cef168_dev = devm_kzalloc(&client->dev, sizeof(*cef168_dev),
> +                                 GFP_KERNEL);
> +       if (!cef168_dev)
> +               return -ENOMEM;
> +
> +       v4l2_i2c_subdev_init(&cef168_dev->sd, client, &cef168_ops);
> +       cef168_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> +                               V4L2_SUBDEV_FL_HAS_EVENTS;
> +
> +       rval = cef168_init_controls(cef168_dev);
> +       if (rval)
> +               goto err_cleanup;
> +
> +       rval = media_entity_pads_init(&cef168_dev->sd.entity, 0, NULL);
> +       if (rval < 0)
> +               goto err_cleanup;
> +
> +       cef168_dev->sd.entity.function = MEDIA_ENT_F_LENS;
> +
> +       rval = v4l2_async_register_subdev(&cef168_dev->sd);
> +       if (rval < 0)
> +               goto err_cleanup;
> +
> +       crc8_populate_msb(cef168_crc8_table, CEF_CRC8_POLYNOMIAL);
> +
> +       return 0;
> +
> +err_cleanup:
> +       v4l2_ctrl_handler_free(&cef168_dev->ctrls);
> +       media_entity_cleanup(&cef168_dev->sd.entity);
> +
> +       return rval;
> +}
> +
> +static void cef168_remove(struct i2c_client *client)
> +{
> +       struct v4l2_subdev *sd = i2c_get_clientdata(client);
> +       struct cef168_device *cef168_dev = sd_to_cef168(sd);
> +
> +       v4l2_async_unregister_subdev(&cef168_dev->sd);
> +       v4l2_ctrl_handler_free(&cef168_dev->ctrls);
> +       media_entity_cleanup(&cef168_dev->sd.entity);
> +}
> +
> +static const struct of_device_id cef168_of_table[] = {
> +       { .compatible = "pinefeat,cef168" },
> +       { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, cef168_of_table);
> +
> +static struct i2c_driver cef168_i2c_driver = {
> +       .driver = {
> +               .name = CEF168_NAME,
> +               .of_match_table = cef168_of_table,
> +       },
> +       .probe = cef168_probe,
> +       .remove = cef168_remove,
> +};
> +
> +module_i2c_driver(cef168_i2c_driver);
> +
> +MODULE_AUTHOR("support@pinefeat.co.uk>");
> +MODULE_DESCRIPTION("CEF168 lens driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 2d30107e047e..f8ca4f8c89af 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -228,6 +228,12 @@ enum v4l2_colorfx {
>   */
>  #define V4L2_CID_USER_RKISP1_BASE              (V4L2_CID_USER_BASE + 0x1220)
>  
> +/*
> + * The base for Pinefeat CEF168 driver controls.
> + * We reserve 16 controls for this driver.
> + */
> +#define V4L2_CID_USER_CEF168_BASE              (V4L2_CID_USER_BASE + 0x1230)
> +
>  /* MPEG-class control IDs */
>  /* The MPEG controls are applicable to all codec controls
>   * and the 'MPEG' part of the define is historical */
> -- 
> 2.34.1
>

Re: [PATCH v5 2/2] media: i2c: Pinefeat cef168 lens control board driver

[Aliaksandr Smirnou]

Hi Kieran,

On Sun, 05 Oct 2025 15:32:45 +0100, Kieran Bingham wrote:
> I'm really looking forward to trying this device out sometime. Very
> interesting piece of kit - Thank you for working directly to upstream
> the support! That's really awesome.

Thanks! I appreciate that. It's been interesting work, and it's great
to see the device getting some attention. I'm looking forward to hearing
your impressions once you get a chance to try it out.

> > +  rx_data->focus_distance_min = le16_to_cpup((__le16 *)&rx_data->focus_distance_min);
> > +  rx_data->focus_distance_max = le16_to_cpup((__le16 *)&rx_data->focus_distance_max);
>
> What is the focus distance in this case? Is it a measured distance that
> could be applied to focus?

The focus distance reported by the lens is the distance between the
camera's sensor and the subject currently in focus, measured in meters.
It correlates with the focus position - higher positions correspond to
greater distances - but the relationship is non-linear.

Libcamera's autofocus algorithm works in dioptres, which are the inverse
of distance. In the driver, reading the distance from the lens allows
the creation of a piecewise linear (PWL) function that maps inverse
distance to the hardware lens setting.

> > +       case CEF168_V4L2_CID_CUSTOM(calibrate):
> > +               return cef168_i2c_write(dev, INP_CALIBRATE, 0);
> 
> Is there any documentation on how to use this control?

The control performs a calibration action operated by the controller
to determine the total number of focus steps. The action is invoked
by the calibration utility, which moves the lens, reads the changing
focus distance, and finally generates a PWL function to be included
in the Libcamera tuning file.

End users are not expected to trigger this control manually through
the V4L2 API, as it is intended for use by the calibration tool during
system setup. For this reason, separate user-facing documentation for
the control has not been added.

In other use cases, calibration can be performed simply by toggling
the AF/MF switch on the lens three times within 15 seconds.

> > +       case CEF168_V4L2_CID_CUSTOM(focus_range):
> > +               ctrl->p_new.p_u32[0] = ((u32)data.focus_position_min << 16) |
> > +                                      (u32)data.focus_position_max;
> 
> Is this really a custom control ? Is there any way to convey this
> through the min/max of the FOCUS_ABSOLUTE control ?

We aimed to minimize the use of custom controls, but in this case one
is necessary.

When the driver is already loaded and initialized, the focus range of
the lens may vary depending on the lens state and configuration
options. Because the control's operational range is defined during
driver probe, the Linux V4L2 API does not allow the minimum and
maximum values to be changed while the driver is running.

The maximum focus position is not known until calibration is
performed, so initially the position is set to zero. The focus range
may vary slightly between calibration runs, as it is derived from
counting motor steps, and its precision depends on the lens's focusing
mechanics.

Additionally, some lenses provide a two-position switch that changes
the minimum focusing distance, significantly reducing the focus range
 - for example, from 2100 to 800.

> > +       case CEF168_V4L2_CID_CUSTOM(lens_id):
> > +               ctrl->p_new.p_u8[0] = data.lens_id;
> > +               return 0;
> 
> Is this a specific individual ID value for the connected lens (i.e.
> every lens has a custom id?) or is it a reference to the lens
> model/type?
> 
> Is this something we could use in libcamera for instance to select an
> appropriate tuning file per lens (type) ?

It represents the ID of the lens model/type. Correct, it's purpose to
assist in selecting the appropriate autofocus algorithm settings.

For calibrated lenses, the controller stores the focus range in EEPROM,
allowing lenses to be swapped without repeating calibration. The
Libcamera tuning file includes an autofocus algorithm section that is
linked to the lens's focus range, focus distance, and focusing speed.
Therefore, reading the lens ID when loading the corresponding tuning
file after a lens swap is both convenient and necessary.

It can be read as follows:

v4l2-ctl -d /dev/v4l-subdev3 -C lens_id

Re: [PATCH v5 2/2] media: i2c: Pinefeat cef168 lens control board driver

[Kieran Bingham]

Quoting Aliaksandr Smirnou (2025-10-05 19:00:25)
> Hi Kieran,
> 
> On Sun, 05 Oct 2025 15:32:45 +0100, Kieran Bingham wrote:
> > I'm really looking forward to trying this device out sometime. Very
> > interesting piece of kit - Thank you for working directly to upstream
> > the support! That's really awesome.
> 
> Thanks! I appreciate that. It's been interesting work, and it's great
> to see the device getting some attention. I'm looking forward to hearing
> your impressions once you get a chance to try it out.
> 
> > > +  rx_data->focus_distance_min = le16_to_cpup((__le16 *)&rx_data->focus_distance_min);
> > > +  rx_data->focus_distance_max = le16_to_cpup((__le16 *)&rx_data->focus_distance_max);
> >
> > What is the focus distance in this case? Is it a measured distance that
> > could be applied to focus?
> 
> The focus distance reported by the lens is the distance between the
> camera's sensor and the subject currently in focus, measured in meters.
> It correlates with the focus position - higher positions correspond to
> greater distances - but the relationship is non-linear.

What's measuring this distance ? Something in the lens, rather than the
sensor?

> 
> Libcamera's autofocus algorithm works in dioptres, which are the inverse
> of distance. In the driver, reading the distance from the lens allows
> the creation of a piecewise linear (PWL) function that maps inverse
> distance to the hardware lens setting.
> 
> > > +       case CEF168_V4L2_CID_CUSTOM(calibrate):
> > > +               return cef168_i2c_write(dev, INP_CALIBRATE, 0);
> > 
> > Is there any documentation on how to use this control?
> 
> The control performs a calibration action operated by the controller
> to determine the total number of focus steps. The action is invoked
> by the calibration utility, which moves the lens, reads the changing
> focus distance, and finally generates a PWL function to be included
> in the Libcamera tuning file.
> 
> End users are not expected to trigger this control manually through
> the V4L2 API, as it is intended for use by the calibration tool during
> system setup. For this reason, separate user-facing documentation for
> the control has not been added.
> 
> In other use cases, calibration can be performed simply by toggling
> the AF/MF switch on the lens three times within 15 seconds.
> 
> > > +       case CEF168_V4L2_CID_CUSTOM(focus_range):
> > > +               ctrl->p_new.p_u32[0] = ((u32)data.focus_position_min << 16) |
> > > +                                      (u32)data.focus_position_max;
> > 
> > Is this really a custom control ? Is there any way to convey this
> > through the min/max of the FOCUS_ABSOLUTE control ?
> 
> We aimed to minimize the use of custom controls, but in this case one
> is necessary.
> 
> When the driver is already loaded and initialized, the focus range of
> the lens may vary depending on the lens state and configuration
> options. Because the control's operational range is defined during
> driver probe, the Linux V4L2 API does not allow the minimum and
> maximum values to be changed while the driver is running.

Have you tried with __v4l2_ctrl_modify_range() ?

We should make sure this also generates an event that libcamera can
subscribe to to make sure it knows there's been an update.


> The maximum focus position is not known until calibration is
> performed, so initially the position is set to zero. The focus range
> may vary slightly between calibration runs, as it is derived from
> counting motor steps, and its precision depends on the lens's focusing
> mechanics.
> 
> Additionally, some lenses provide a two-position switch that changes
> the minimum focusing distance, significantly reducing the focus range
>  - for example, from 2100 to 800.

I see, so a user interaction can update it too. Does the driver have to
poll to get this update? Or does it just find out on the next read ?

> 
> > > +       case CEF168_V4L2_CID_CUSTOM(lens_id):
> > > +               ctrl->p_new.p_u8[0] = data.lens_id;
> > > +               return 0;
> > 
> > Is this a specific individual ID value for the connected lens (i.e.
> > every lens has a custom id?) or is it a reference to the lens
> > model/type?
> > 
> > Is this something we could use in libcamera for instance to select an
> > appropriate tuning file per lens (type) ?
> 
> It represents the ID of the lens model/type. Correct, it's purpose to
> assist in selecting the appropriate autofocus algorithm settings.

We're also pushing for a core control for this I think. That's something
we 'need' in libcamera globally for all cameras. Unfortunately I can't
point you at an existing control yet ;-(


> For calibrated lenses, the controller stores the focus range in EEPROM,
> allowing lenses to be swapped without repeating calibration. The
> Libcamera tuning file includes an autofocus algorithm section that is
> linked to the lens's focus range, focus distance, and focusing speed.
> Therefore, reading the lens ID when loading the corresponding tuning
> file after a lens swap is both convenient and necessary.
> 
> It can be read as follows:
> 
> v4l2-ctl -d /dev/v4l-subdev3 -C lens_id

I'll see if I can find time to order a kit, and get a compatible lens. I
have a Nikon DSLR though so I don't have lenses to match this yet :-(

--
Kieran

Re: [PATCH v5 2/2] media: i2c: Pinefeat cef168 lens control board driver

[Aliaksandr Smirnou]

On Sun, 05 Oct 2025 20:58:36 +0100, Kieran Bingham wrote:

> > The focus distance reported by the lens is the distance between the
> > camera's sensor and the subject currently in focus, measured in meters.
> > It correlates with the focus position - higher positions correspond to
> > greater distances - but the relationship is non-linear.
> 
> What's measuring this distance ? Something in the lens, rather than the
> sensor?

It doesn't actively measure the distance. The lens contains a distance
encoder that maps motor steps to an approximate focus distance, calibrated
for that lens model/type at the factory.

> > the Linux V4L2 API does not allow the minimum and
> > maximum values to be changed while the driver is running.
> 
> Have you tried with __v4l2_ctrl_modify_range() ?
> 
> We should make sure this also generates an event that libcamera can
> subscribe to to make sure it knows there's been an update.

Thanks. The unlocked variant of this function works. Previously I tried
its locked variant that led to locking issues, that's why I though it
is not possible.

I'll use that approach and get rid of the custom control.

> I see, so a user interaction can update it too. Does the driver have to
> poll to get this update? Or does it just find out on the next read ?  

The driver reads all lens data at once, so it knows the focus range on
the next read. 

I hope it is ok to modify the focus range straight in g_volatile_ctrl,
when the focus value is requested and read from the lens.

> We're also pushing for a core control for this I think. That's something
> we 'need' in libcamera globally for all cameras. Unfortunately I can't
> point you at an existing control yet ;-(

Good to know. Then for now, I'll keep the custom control for the lens ID,
and switch to a common one once it becomes available.

One suggestion was to use the device name to store the lens ID, but I
couldn't find a way to modify the name after probe, since the lens might
be disconnected and replaced with another. Also, for I2C devices, the
device name appears to be assigned by the kernel.

> I'll see if I can find time to order a kit, and get a compatible lens. I
> have a Nikon DSLR though so I don't have lenses to match this yet :-(

We will be happy to provide a product sample for free, just let us know
the delivery address by email.