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

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

[Alexander Smirnov]

From: Aliaksandr Smirnou <support@pinefeat.co.uk>

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

There are no regulators on the board. The MCU is powered by 3.3V, which is
supplied either through the CSI connector or the serial connector directly
from the Raspberry Pi GPIO power pins. Hence, the driver lacks any regulator
handling. Please let me know if regulator support is still needed.

Changes in v2:
 in [PATCH 1/2] addressed review comments:
 - added optional vcc-supply property and example
 - made node name generic in the example
 - removed tree type and location from maintainers entry

 in [PATCH 2/2] addressed review comments:
 - wrapped devm_kzalloc line according to coding style
 - removed oddly formed i2c_device_id struct as not required
 - formatted of_device_id struct according to coding style

Link to v1: https://lore.kernel.org/all/20250810192609.11966-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   |  52 +++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   8 +
 drivers/media/i2c/Kconfig                     |   8 +
 drivers/media/i2c/Makefile                    |   1 +
 drivers/media/i2c/cef168.c                    | 337 ++++++++++++++++++
 drivers/media/i2c/cef168.h                    |  51 +++
 7 files changed, 459 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
 create mode 100644 drivers/media/i2c/cef168.c
 create mode 100644 drivers/media/i2c/cef168.h


base-commit: 2b38afce25c4e1b8f943ff4f0a2b51d6c40f2ed2
-- 
2.34.1

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

[Alexander Smirnov]

From: Aliaksandr Smirnou <support@pinefeat.co.uk>

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.

Signed-off-by: Aliaksandr Smirnou <support@pinefeat.co.uk>
---
 .../bindings/media/i2c/pinefeat,cef168.yaml   | 52 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 MAINTAINERS                                   |  6 +++
 3 files changed, 60 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..3563fb1d46db
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/pinefeat,cef168.yaml
@@ -0,0 +1,52 @@
+# 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. This driver enables controlling the lens
+  focus and aperture via the V4L2 (Video4Linux2) API.
+
+properties:
+  compatible:
+    enum:
+      - pinefeat,cef168
+
+  reg:
+    maxItems: 1
+
+  vcc-supply:
+    description: VDD 3v3 power supply
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        camera-lens@d {
+            compatible = "pinefeat,cef168";
+            reg = <0x0d>;
+            vcc-supply = <&vdd_3v3_reg>;
+        };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 77160cd47f54..dab27f769b0a 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1195,6 +1195,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 fe168477caa4..811c6a150029 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19985,6 +19985,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

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

[Alexander Smirnov]

From: Aliaksandr Smirnou <support@pinefeat.co.uk>

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 <support@pinefeat.co.uk>
---
 MAINTAINERS                |   2 +
 drivers/media/i2c/Kconfig  |   8 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/cef168.c | 337 +++++++++++++++++++++++++++++++++++++
 drivers/media/i2c/cef168.h |  51 ++++++
 5 files changed, 399 insertions(+)
 create mode 100644 drivers/media/i2c/cef168.c
 create mode 100644 drivers/media/i2c/cef168.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 811c6a150029..922efc000722 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19990,6 +19990,8 @@ 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
+F:	drivers/media/i2c/cef168.h
 
 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 6237fe804a5c..c4c3b03a0b98 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -791,6 +791,14 @@ 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"
+	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 5873d29433ee..75a95f850f18 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..db3f97e7e9dc
--- /dev/null
+++ b/drivers/media/i2c/cef168.c
@@ -0,0 +1,337 @@
+// 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/pm_runtime.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include "cef168.h"
+
+/*
+ * cef168 device structure
+ */
+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;
+
+	val = cpu_to_le16(val);
+	char tx_data[4] = { cmd, (val & 0xff), (val >> 8) };
+
+	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_cpu(rx_data->moving_time);
+	rx_data->focus_position_min = le16_to_cpu(rx_data->focus_position_min);
+	rx_data->focus_position_max = le16_to_cpu(rx_data->focus_position_max);
+	rx_data->focus_position_cur = le16_to_cpu(rx_data->focus_position_cur);
+	rx_data->focus_distance_min = le16_to_cpu(rx_data->focus_distance_min);
+	rx_data->focus_distance_max = le16_to_cpu(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 0;
+	}
+
+	return -EINVAL;
+}
+
+static int cef168_get_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct cef168_device *dev = to_cef168(ctrl);
+	int rval;
+
+	if (ctrl->id != V4L2_CID_FOCUS_ABSOLUTE &&
+	    ctrl->id != CEF168_V4L2_CID_CUSTOM(data) &&
+	    ctrl->id != CEF168_V4L2_CID_CUSTOM(focus_range) &&
+	    ctrl->id != CEF168_V4L2_CID_CUSTOM(lens_id))
+		return -EINVAL;
+
+	struct cef168_data data;
+
+	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)le32_to_cpu(((u32)data.focus_position_min << 16) |
+					 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 int cef168_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	return pm_runtime_resume_and_get(sd->dev);
+}
+
+static int cef168_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	pm_runtime_put(sd->dev);
+	return 0;
+}
+
+static const struct v4l2_subdev_internal_ops cef168_int_ops = {
+	.open = cef168_open,
+	.close = cef168_close,
+};
+
+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 void cef168_subdev_cleanup(struct cef168_device *cef168_dev)
+{
+	v4l2_async_unregister_subdev(&cef168_dev->sd);
+	v4l2_ctrl_handler_free(&cef168_dev->ctrls);
+	media_entity_cleanup(&cef168_dev->sd.entity);
+}
+
+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);
+	v4l2_ctrl_new_custom(hdl, &cef168_calibrate_ctrl, NULL);
+	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 == NULL)
+		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;
+	cef168_dev->sd.internal_ops = &cef168_int_ops;
+
+	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);
+
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+	pm_runtime_idle(&client->dev);
+
+	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);
+
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+	cef168_subdev_cleanup(cef168_dev);
+}
+
+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/drivers/media/i2c/cef168.h b/drivers/media/i2c/cef168.h
new file mode 100644
index 000000000000..cdce1a19bda0
--- /dev/null
+++ b/drivers/media/i2c/cef168.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Pinefeat cef168 lens driver
+ *
+ * Copyright (c) 2025 Pinefeat LLP
+ */
+
+#ifndef CEF168_CEF168_H
+#define CEF168_CEF168_H
+
+#define CEF168_NAME "cef168"
+
+#define CEF168_V4L2_CID_CUSTOM(ctrl) \
+	((V4L2_CID_USER_BASE | 168) + custom_##ctrl)
+
+enum { custom_lens_id, custom_data, custom_focus_range, custom_calibrate };
+
+/**
+ * cef168 data structure
+ */
+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;
+
+/*
+ * cef168 I2C protocol commands
+ */
+#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
+
+#ifdef DECLARE_CRC8_TABLE
+DECLARE_CRC8_TABLE(cef168_crc8_table);
+#endif
+
+#endif //CEF168_CEF168_H
-- 
2.34.1

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

[Krzysztof Kozlowski]

On 11/08/2025 23:31, Alexander Smirnov wrote:
> +

Subject: missing media prefix.

See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18


> +properties:
> +  compatible:
> +    enum:
> +      - pinefeat,cef168
> +
> +  reg:
> +    maxItems: 1
> +
> +  vcc-supply:
> +    description: VDD 3v3 power supply


Property says VCC, description says VDD, datasheet says 5V (not 3.3V).

I guess this should be for the 5V case, no? If that's true, then I think
that 5V pin is not a controllable supply ever, because this product is
purely for RPi which has 5V pins directly tied to main power supply.


Best regards,
Krzysztof

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

[Aliaksandr Smirnou]

On Tue, 12 Aug 2025 09:13:22 +0200, Krzysztof Kozlowski wrote:
> Property says VCC, description says VDD, datasheet says 5V (not 3.3V).
>
> I guess this should be for the 5V case, no?

Thank you following up.

The 5V line is used exclusively to power the lens motor (through the
power switch). This 5V supply can come from the Raspberry Pi GPIO
header, a battery, or other sources. Importantly, this power source
is independent of the board’s MCU and its kernel driver.

Additionally, the board does not include any voltage regulators.
The MCU operates at 3.3V, which is supplied either via the CSI connector
or the serial connector directly from the Raspberry Pi GPIO 3.3V rail.
Therefore, the driver does not manage any regulator, which is why the
“vcc-supply” property was absent in the binding.

Would you like me to remove the “vcc-supply” property as it was
originally?

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

[kernel test robot]

Hi Alexander,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 2b38afce25c4e1b8f943ff4f0a2b51d6c40f2ed2]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexander-Smirnov/dt-bindings-Pinefeat-cef168-lens-control-board/20250812-053441
base:   2b38afce25c4e1b8f943ff4f0a2b51d6c40f2ed2
patch link:    https://lore.kernel.org/r/20250811213102.15703-3-aliaksandr.smirnou%40gmail.com
patch subject: [PATCH v2 2/2] media/i2c: Pinefeat cef168 lens control board driver
config: arm64-randconfig-r132-20250813 (https://download.01.org/0day-ci/archive/20250813/202508130917.FKIv8K4Z-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.5.0
reproduce: (https://download.01.org/0day-ci/archive/20250813/202508130917.FKIv8K4Z-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508130917.FKIv8K4Z-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/media/i2c/cef168.c:37:13: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [usertype] val @@     got restricted __le16 [usertype] @@
   drivers/media/i2c/cef168.c:37:13: sparse:     expected unsigned short [usertype] val
   drivers/media/i2c/cef168.c:37:13: sparse:     got restricted __le16 [usertype]
>> drivers/media/i2c/cef168.c:76:32: sparse: sparse: cast to restricted __le16
>> drivers/media/i2c/cef168.c:76:32: sparse: sparse: cast to restricted __le16
>> drivers/media/i2c/cef168.c:76:32: sparse: sparse: cast to restricted __le16
>> drivers/media/i2c/cef168.c:76:32: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:77:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:77:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:77:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:77:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:78:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:78:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:78:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:78:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:79:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:79:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:79:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:79:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:80:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:80:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:80:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:80:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:81:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:81:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:81:39: sparse: sparse: cast to restricted __le16
   drivers/media/i2c/cef168.c:81:39: sparse: sparse: cast to restricted __le16
>> drivers/media/i2c/cef168.c:133:30: sparse: sparse: cast to restricted __le32
>> drivers/media/i2c/cef168.c:133:30: sparse: sparse: cast to restricted __le32
>> drivers/media/i2c/cef168.c:133:30: sparse: sparse: cast to restricted __le32
>> drivers/media/i2c/cef168.c:133:30: sparse: sparse: cast to restricted __le32
>> drivers/media/i2c/cef168.c:133:30: sparse: sparse: cast to restricted __le32
>> drivers/media/i2c/cef168.c:133:30: sparse: sparse: cast to restricted __le32

vim +37 drivers/media/i2c/cef168.c

    31	
    32	static int cef168_i2c_write(struct cef168_device *cef168_dev, u8 cmd, u16 val)
    33	{
    34		struct i2c_client *client = v4l2_get_subdevdata(&cef168_dev->sd);
    35		int retry, ret;
    36	
  > 37		val = cpu_to_le16(val);
    38		char tx_data[4] = { cmd, (val & 0xff), (val >> 8) };
    39	
    40		tx_data[3] = crc8(cef168_crc8_table, tx_data, 3, CRC8_INIT_VALUE);
    41	
    42		for (retry = 0; retry < 3; retry++) {
    43			ret = i2c_master_send(client, tx_data, sizeof(tx_data));
    44			if (ret == sizeof(tx_data))
    45				return 0;
    46			else if (ret != -EIO && ret != -EREMOTEIO)
    47				break;
    48		}
    49	
    50		dev_err(&client->dev, "I2C write fail after %d retries, ret=%d\n",
    51			retry, ret);
    52		return -EIO;
    53	}
    54	
    55	static int cef168_i2c_read(struct cef168_device *cef168_dev,
    56				   struct cef168_data *rx_data)
    57	{
    58		struct i2c_client *client = v4l2_get_subdevdata(&cef168_dev->sd);
    59	
    60		int ret = i2c_master_recv(client, (char *)rx_data,
    61					  sizeof(struct cef168_data));
    62		if (ret != sizeof(struct cef168_data)) {
    63			dev_err(&client->dev, "I2C read fail, ret=%d\n", ret);
    64			return -EIO;
    65		}
    66	
    67		u8 computed_crc = crc8(cef168_crc8_table, (const u8 *)rx_data,
    68				       sizeof(struct cef168_data) - 1, CRC8_INIT_VALUE);
    69		if (computed_crc != rx_data->crc8) {
    70			dev_err(&client->dev,
    71				"CRC mismatch calculated=0x%02X read=0x%02X\n",
    72				computed_crc, rx_data->crc8);
    73			return -EIO;
    74		}
    75	
  > 76		rx_data->moving_time = le16_to_cpu(rx_data->moving_time);
    77		rx_data->focus_position_min = le16_to_cpu(rx_data->focus_position_min);
    78		rx_data->focus_position_max = le16_to_cpu(rx_data->focus_position_max);
    79		rx_data->focus_position_cur = le16_to_cpu(rx_data->focus_position_cur);
    80		rx_data->focus_distance_min = le16_to_cpu(rx_data->focus_distance_min);
    81		rx_data->focus_distance_max = le16_to_cpu(rx_data->focus_distance_max);
    82	
    83		return 0;
    84	}
    85	
    86	static int cef168_set_ctrl(struct v4l2_ctrl *ctrl)
    87	{
    88		struct cef168_device *dev = to_cef168(ctrl);
    89		u8 cmd;
    90	
    91		switch (ctrl->id) {
    92		case V4L2_CID_FOCUS_ABSOLUTE:
    93			return cef168_i2c_write(dev, INP_SET_FOCUS, ctrl->val);
    94		case V4L2_CID_FOCUS_RELATIVE:
    95			cmd = ctrl->val < 0 ? INP_SET_FOCUS_N : INP_SET_FOCUS_P;
    96			return cef168_i2c_write(dev, cmd, abs(ctrl->val));
    97		case V4L2_CID_IRIS_ABSOLUTE:
    98			return cef168_i2c_write(dev, INP_SET_APERTURE, ctrl->val);
    99		case V4L2_CID_IRIS_RELATIVE:
   100			cmd = ctrl->val < 0 ? INP_SET_APERTURE_N : INP_SET_APERTURE_P;
   101			return cef168_i2c_write(dev, cmd, abs(ctrl->val));
   102		case CEF168_V4L2_CID_CUSTOM(calibrate):
   103			return cef168_i2c_write(dev, INP_CALIBRATE, 0);
   104			return 0;
   105		}
   106	
   107		return -EINVAL;
   108	}
   109	
   110	static int cef168_get_ctrl(struct v4l2_ctrl *ctrl)
   111	{
   112		struct cef168_device *dev = to_cef168(ctrl);
   113		int rval;
   114	
   115		if (ctrl->id != V4L2_CID_FOCUS_ABSOLUTE &&
   116		    ctrl->id != CEF168_V4L2_CID_CUSTOM(data) &&
   117		    ctrl->id != CEF168_V4L2_CID_CUSTOM(focus_range) &&
   118		    ctrl->id != CEF168_V4L2_CID_CUSTOM(lens_id))
   119			return -EINVAL;
   120	
   121		struct cef168_data data;
   122	
   123		rval = cef168_i2c_read(dev, &data);
   124		if (rval < 0)
   125			return rval;
   126	
   127		switch (ctrl->id) {
   128		case V4L2_CID_FOCUS_ABSOLUTE:
   129			ctrl->val = data.focus_position_cur;
   130			return 0;
   131		case CEF168_V4L2_CID_CUSTOM(focus_range):
   132			ctrl->p_new.p_u32[0] =
 > 133				(u32)le32_to_cpu(((u32)data.focus_position_min << 16) |
   134						 data.focus_position_max);
   135			return 0;
   136		case CEF168_V4L2_CID_CUSTOM(lens_id):
   137			ctrl->p_new.p_u8[0] = data.lens_id;
   138			return 0;
   139		case CEF168_V4L2_CID_CUSTOM(data):
   140			memcpy(ctrl->p_new.p_u8, &data, sizeof(data));
   141			return 0;
   142		}
   143	
   144		return -EINVAL;
   145	}
   146	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Krzysztof Kozlowski]

On Mon, Aug 11, 2025 at 10:31:01PM +0100, Alexander Smirnov wrote:
> From: Aliaksandr Smirnou <support@pinefeat.co.uk>
> 
> Add the Device Tree schema and examples for the Pinefeat cef168 lens
>  control board. This board interfaces Canon EF & EF-S lenses with

Also, some stray indent is here.

>  non-Canon camera bodies, enabling electronic control of focus and
>  aperture via V4L2.
> 
> Signed-off-by: Aliaksandr Smirnou <support@pinefeat.co.uk>

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On Tue, Aug 12, 2025 at 08:37:22PM +0100, Aliaksandr Smirnou wrote:
> On Tue, 12 Aug 2025 09:13:22 +0200, Krzysztof Kozlowski wrote:
> > Property says VCC, description says VDD, datasheet says 5V (not 3.3V).
> >
> > I guess this should be for the 5V case, no?
> 
> Thank you following up.
> 
> The 5V line is used exclusively to power the lens motor (through the
> power switch). This 5V supply can come from the Raspberry Pi GPIO
> header, a battery, or other sources. Importantly, this power source
> is independent of the board’s MCU and its kernel driver.

You describe here the entire board, not the MCU only or lens motor
only...

> 
> Additionally, the board does not include any voltage regulators.
> The MCU operates at 3.3V, which is supplied either via the CSI connector
> or the serial connector directly from the Raspberry Pi GPIO 3.3V rail.
> Therefore, the driver does not manage any regulator, which is why the
> “vcc-supply” property was absent in the binding.
> 
> Would you like me to remove the “vcc-supply” property as it was
> originally?

Can the board be used outside of above setup? I understand so far this
is only for Rpi where both above supplies - 3.3 V and 5 V - are coming
from the header pins, so supplies would be totally redundant.

Best regards,
Krzysztof

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

[Aliaksandr Smirnou]

On Thu, 14 Aug 2025 11:04:10 +0200, Krzysztof Kozlowski wrote:
> You describe here the entire board, not the MCU only or lens motor
> only...

Since you asked about 5 V, I was explaining that this power source is not
relevant for the driver.

> Can the board be used outside of above setup? I understand so far this
> is only for Rpi where both above supplies - 3.3 V and 5 V - are coming
> from the header pins, so supplies would be totally redundant.

There are several variants of the board, differing only in physical size
and type of CSI connector, targeting different cameras. The board should
be compatible with any single-board computer that uses a similar CSI
connector pinout and MIPI signal lane assignment. For example, the NVIDIA
Jetson series replicates the Raspberry Pi camera and GPIO header pinout.
So yes, the board can be used outside of a Raspberry Pi setup.

As noted above, these supplies are redundant and were not included in the
driver description. Given that, is it acceptable to remove the vcc-supply
property?

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

[Krzysztof Kozlowski]

On 14/08/2025 22:10, Aliaksandr Smirnou wrote:
> On Thu, 14 Aug 2025 11:04:10 +0200, Krzysztof Kozlowski wrote:
>> You describe here the entire board, not the MCU only or lens motor
>> only...
> 
> Since you asked about 5 V, I was explaining that this power source is not
> relevant for the driver.
> 
>> Can the board be used outside of above setup? I understand so far this
>> is only for Rpi where both above supplies - 3.3 V and 5 V - are coming
>> from the header pins, so supplies would be totally redundant.
> 
> There are several variants of the board, differing only in physical size
> and type of CSI connector, targeting different cameras. The board should
> be compatible with any single-board computer that uses a similar CSI
> connector pinout and MIPI signal lane assignment. For example, the NVIDIA
> Jetson series replicates the Raspberry Pi camera and GPIO header pinout.
> So yes, the board can be used outside of a Raspberry Pi setup.

If they replicate also power coming from the header and CSI line, then
it's fine, but if the do not, then you would need a controllable
regulator supply.

> 
> As noted above, these supplies are redundant and were not included in the
> driver description. Given that, is it acceptable to remove the vcc-supply
> property?

Yeah, for simplicity. Please mention this rationale in the commit msg -
power supply is derived from fixed supplies (connector/header on RPi),
so representing it is redundant.


Best regards,
Krzysztof