[PATCH 0/3] Add Vision Components MIPI Camera Module support

[PATCH 0/3] Add Vision Components MIPI Camera Module support

[Richard Leitner]

This series adds support for Vision Components MIPI camera modules by
adding a device driver for the controller chip found on those modules.

The controller is an I2C-accessible FPGA that provides three main
functions for the attached image sensor:

 - power sequencing and control of the module supply
 - a fixed clock output used as the sensor input clock
 - routing of I2C traffic between the host and the sensor

The initial version of this driver was written by Laurent Pinchart and
is available at: gitlab.com/ideasonboard/nxp/linux.git in the
v7.0/sensors/vcmipi branch. Therefore I've added Laurent and me as a
maintainer for this driver and left Laurent as MODULE_AUTHOR. If that's
incorrect or not desired, please feel free to reach out! Thanks

This series was tested using ov9282 camera modules from Vision
Components. Those were flashed with different FPGA firmware versions and
connected to an i.MX8MP SoC.

Any feedback is warmly welcome.

Thanks!

Link: https://lore.kernel.org/all/20260728092856.GB1494774@killaraus.ideasonboard.com/
Link: https://gitlab.com/ideasonboard/nxp/linux/-/commits/v7.0/sensors/vcmipi
Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
Richard Leitner (3):
      dt-bindings: vendor-prefixes: add Vision Components GmbH
      dt-bindings: media: i2c: Add vision-components,mipi-module-controller
      media: i2c: vc-mipi: add Vision Components MIPI Module Controller

 .../vision-components,mipi-module-controller.yaml  |  92 ++++
 .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
 MAINTAINERS                                        |   8 +
 drivers/media/i2c/Kconfig                          |  13 +
 drivers/media/i2c/Makefile                         |   1 +
 drivers/media/i2c/vc-mipi.c                        | 541 +++++++++++++++++++++
 6 files changed, 657 insertions(+)
---
base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
change-id: 20260903-vc-mipi-ctrl-6d442cefcc40

Best regards,
--  
Richard Leitner <richard.leitner@linux.dev>

[PATCH 1/3] dt-bindings: vendor-prefixes: add Vision Components GmbH

[Richard Leitner]

Add vendor prefix for Vision Components GmbH, a manufacturer of camera
modules. The prefix will be used in the Vision Components MIPI Camera
Controller driver.

Link: https://www.vision-components.com
Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index ba20029693734..b853fb9338527 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1845,6 +1845,8 @@ patternProperties:
     description: Vishay Intertechnology, Inc
   "^visionox,.*":
     description: Visionox
+  "^vision-components,.*":
+    description: Vision Components GmbH
   "^vitesse,.*":
     description: Vitesse Semiconductor Corporation
   "^vivante,.*":

-- 
2.53.0

[PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller

[Richard Leitner]

Add bindings for the Vision Components MIPI Camera Module Controller.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 .../vision-components,mipi-module-controller.yaml  | 92 ++++++++++++++++++++++
 MAINTAINERS                                        |  7 ++
 2 files changed, 99 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
new file mode 100644
index 0000000000000..2a031aea68457
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/vision-components,mipi-module-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Vision Components MIPI Camera Module Controller
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+  - Richard Leitner <richard.leitner@linux.dev>
+
+description: |-
+  The MIPI camera module controller is an FPGA-based, I2C-accessible system
+  controller found on MIPI camera modules from Vision Components. It abstracts
+  the camera sensor behind a unified register interface, and controls and
+  sequences the on-board power supplies and clocks.
+
+  The camera sensor abstraction is optional. The controller exposes a tunneled
+  downstream I2C bus used by the attached image sensor. The controller node
+  acts as the upstream device on the host bus, while child node below the
+  controller represent the sensor device reachable through the tunnel.
+
+properties:
+  compatible:
+    const: vision-components,mipi-module-controller
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 0
+
+  clock-frequency:
+    description: Frequency of the sensor clock provided by the module
+
+  vcc-supply:
+    description: Power supply of the module (3.3V)
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - clock-frequency
+  - vcc-supply
+  - '#address-cells'
+  - '#size-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        vc_mipi_ctrl: controller@10 {
+            compatible = "vision-components,mipi-module-controller";
+            reg = <0x10>;
+            #clock-cells = <0>;
+            clock-frequency = <37125000>;
+            vcc-supply = <&cam_3v3>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            i2c@0 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                vc_mipi_sensor: camera@60 {
+                    compatible = "ovti,ov9281";
+                    reg = <0x60>;
+
+                    clocks = <&vc_mipi_ctrl>;
+                    clock-names = "inck";
+
+                    avdd-supply = <&vc_mipi_ctrl>;
+                    dvdd-supply = <&vc_mipi_ctrl>;
+                    dovdd-supply = <&vc_mipi_ctrl>;
+                };
+            };
+        };
+    };
+...
+
diff --git a/MAINTAINERS b/MAINTAINERS
index c2414447892c2..3ee3fec098831 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29106,6 +29106,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/vishay,veml6075.yaml
 F:	drivers/iio/light/veml6075.c
 
+VISION COMPONENTS MIPI MODULE CONTROLLER DRIVER
+M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+M:	Richard Leitner <richard.leitner@linux.dev>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
+
 VISL VIRTUAL STATELESS DECODER DRIVER
 M:	Daniel Almeida <daniel.almeida@collabora.com>
 L:	linux-media@vger.kernel.org

-- 
2.53.0

[PATCH 3/3] media: i2c: vc-mipi: add Vision Components MIPI Module Controller

[Richard Leitner]

Add support for the Vision Components MIPI camera module controller.
This is basically an FPGA sitting on most of the Vision Components MIPI
camera modules. It provides three main functions for the attached image
sensor:

  - power sequencing and control of the module supply
  - a fixed clock output used as the sensor input clock
  - tunneling of I2C traffic between the host and the sensor

The driver registers a regulator for the sensor supply, exposes the
module clock as a clock provider, and creates a I2C adapter so that the
downstream sensor can be accessed as a normal I2C device. This is
modeled after the hardware design, where the FPGA is connected to two
separate I2C busses. One towards the host, one towards the sensor.

There are known firmware quirks affecting I2C ROM reading with
auto-increment, as well as multi-byte reads from the sensor. Both are
handled in this driver.

The initial version of this driver was written by Laurent Pinchart
<laurent.pinchart@ideasonboard.com>.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 MAINTAINERS                 |   1 +
 drivers/media/i2c/Kconfig   |  13 ++
 drivers/media/i2c/Makefile  |   1 +
 drivers/media/i2c/vc-mipi.c | 541 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 556 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3ee3fec098831..916cebee2f8b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29112,6 +29112,7 @@ M:	Richard Leitner <richard.leitner@linux.dev>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
+F:	drivers/media/i2c/vc-mipi.c
 
 VISL VIRTUAL STATELESS DECODER DRIVER
 M:	Daniel Almeida <daniel.almeida@collabora.com>
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 5c52007f9cbeb..6ec43f06c24d0 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -868,6 +868,19 @@ config VIDEO_VGXY61
 	  This is a Video4Linux2 sensor driver for the ST VGXY61
 	  camera sensor.
 
+config VIDEO_VC_MIPI
+	tristate "Vision Components MIPI Module support"
+	depends on I2C
+	help
+	  This is a driver for the camera module controller found in the Vision
+	  Components MIPI modules.
+
+	  The controller manages the power supply, reset signal and I2C
+	  communication to the actual camera sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called vc-mipi.
+
 source "drivers/media/i2c/ccs/Kconfig"
 source "drivers/media/i2c/et8ek8/Kconfig"
 
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d04bd5724552e..c768e63cd2cf1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -166,6 +166,7 @@ obj-$(CONFIG_VIDEO_TW9910) += tw9910.o
 obj-$(CONFIG_VIDEO_UDA1342) += uda1342.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
+obj-$(CONFIG_VIDEO_VC_MIPI) += vc-mipi.o
 obj-$(CONFIG_VIDEO_VD55G1) += vd55g1.o
 obj-$(CONFIG_VIDEO_VD56G3) += vd56g3.o
 obj-$(CONFIG_VIDEO_VGXY61) += vgxy61.o
diff --git a/drivers/media/i2c/vc-mipi.c b/drivers/media/i2c/vc-mipi.c
new file mode 100644
index 0000000000000..1ca7f94c17d3c
--- /dev/null
+++ b/drivers/media/i2c/vc-mipi.c
@@ -0,0 +1,541 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Vision Components MIPI Module Controller
+ *
+ * Copyright 2021 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+
+#define VC_MIPI_REG_RESET			0x0100
+#define VC_MIPI_REG_RESET_POWER_DOWN		BIT(1)
+#define VC_MIPI_REG_RESET_RESET			BIT(0)
+
+#define VC_MIPI_REG_STATUS			0x0101
+#define VC_MIPI_REG_STATUS_OFF			0x00
+#define VC_MIPI_REG_STATUS_ERROR		0x01
+#define VC_MIPI_REG_STATUS_ON			0x80
+
+#define VC_MIPI_REG_MODE			0x0102
+#define VC_MIPI_REG_MODE_STREAM_10BIT		0x00
+#define VC_MIPI_REG_MODE_STREAM_8BIT		0x01
+#define VC_MIPI_REG_MODE_EXTTRIG_10BIT		0x02
+#define VC_MIPI_REG_MODE_EXTTRIG_8BIT		0x03
+#define VC_MIPI_REG_MODE_TESTIMG_10BIT		0xF0
+#define VC_MIPI_REG_MODE_TESTIMG_8BIT		0xF1
+
+#define VC_MIPI_REG_IO				0x0103
+#define VC_MIPI_REG_IO_XTRIG_ACTIVE_LOW		BIT(6)
+#define VC_MIPI_REG_IO_FS_ACTIVE_LOW		BIT(5)
+#define VC_MIPI_REG_IO_FLASH_ACTIVE_LOW		BIT(4)
+#define VC_MIPI_REG_IO_XTRIG_ENABLE		BIT(3)
+#define VC_MIPI_REG_IO_EXCHANGE_FLASH_TRIGGER	BIT(2)
+#define VC_MIPI_REG_IO_FS_ON_TRIGGER_ENABLE	BIT(1)
+#define VC_MIPI_REG_IO_FLASH_ENABLE		BIT(0)
+#define VC_MIPI_REG_IO_DEFAULT			VC_MIPI_REG_IO_XTRIG_ENABLE
+
+#define VC_MIPI_REG_MODULE_ADDR			0x0104
+#define VC_MIPI_REG_MODULE_ADDR_DEFAULT		0x10
+
+#define VC_MIPI_REG_SENSOR_ADDR			0x0105
+#define VC_MIPI_REG_SENSOR_ADDR_DEFAULT		0x60
+
+#define VC_MIPI_REG_OUTPUT_OVERRIDE		0x0106
+#define VC_MIPI_REG_OUTPUT_OVERRIDE_TRIGGER_OVR	BIT(5)
+#define VC_MIPI_REG_OUTPUT_OVERRIDE_FLASH_OVR	BIT(4)
+#define VC_MIPI_REG_OUTPUT_OVERRIDE_TRIGGER_VAL	BIT(1)
+#define VC_MIPI_REG_OUTPUT_OVERRIDE_FLASH_VAL	BIT(0)
+
+#define VC_MIPI_REG_INPUT			0x0107
+#define VC_MIPI_REG_INPUT_TRIGGER		BIT(1)
+#define VC_MIPI_REG_INPUT_FLASH			BIT(0)
+
+#define VC_MIPI_REG_EXTTRIG			0x0108
+#define VC_MIPI_REG_EXTTRIG_DISABLE		0x00
+#define VC_MIPI_REG_EXTTRIG_ENABLE		BIT(0)
+
+#define VC_MIPI_REG_ROM				0x1000
+
+struct vc_mipi_ctrl {
+	struct device *dev;
+	struct regmap *regmap;
+	struct regulator *supply;
+	struct clk_hw *clk_hw;
+	struct i2c_adapter i2c_tunnel;
+	bool enabled;
+};
+
+/* -----------------------------------------------------------------------------
+ * Regulator
+ */
+
+static int vc_mipi_regulator_enable(struct regulator_dev *rdev)
+{
+	struct vc_mipi_ctrl *ctrl = rdev_get_drvdata(rdev);
+	unsigned int val;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(ctrl->dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(rdev->regmap, VC_MIPI_REG_RESET, 0);
+	if (ret < 0)
+		goto error;
+
+	msleep(500);
+
+	ret = regmap_read(rdev->regmap, VC_MIPI_REG_STATUS, &val);
+	if (ret < 0)
+		goto error;
+
+	if (val != VC_MIPI_REG_STATUS_ON) {
+		dev_err(&rdev->dev, "Sensor failed to initialize (0x%02x)\n",
+			val);
+		ret = -EIO;
+		goto error;
+	}
+
+	ret = regmap_write(rdev->regmap, VC_MIPI_REG_IO,
+			   (VC_MIPI_REG_IO_DEFAULT | VC_MIPI_REG_IO_FLASH_ENABLE));
+	if (ret < 0) {
+		dev_err(&rdev->dev, "Failed to enable flash: %d\n", ret);
+		goto error;
+	}
+
+	ret = regmap_write(ctrl->regmap, VC_MIPI_REG_EXTTRIG,
+			   VC_MIPI_REG_EXTTRIG_DISABLE);
+	if (ret < 0) {
+		dev_err(&rdev->dev,
+			"Failed to set up external flash trigger: %d\n", ret);
+		goto error;
+	}
+
+	ctrl->enabled = true;
+
+	return 0;
+
+error:
+	pm_runtime_mark_last_busy(ctrl->dev);
+	pm_runtime_put_autosuspend(ctrl->dev);
+	return ret;
+}
+
+static int vc_mipi_regulator_disable(struct regulator_dev *rdev)
+{
+	struct vc_mipi_ctrl *ctrl = rdev_get_drvdata(rdev);
+	int ret;
+
+	ret = regmap_write(ctrl->regmap, VC_MIPI_REG_EXTTRIG,
+			   VC_MIPI_REG_EXTTRIG_DISABLE);
+	if (ret < 0) {
+		dev_err(&rdev->dev,
+			"Failed to set up external flash trigger: %d\n", ret);
+		goto autosuspend;
+	}
+
+	ret = regmap_write(rdev->regmap, VC_MIPI_REG_IO,
+			   (VC_MIPI_REG_IO_DEFAULT | VC_MIPI_REG_IO_FLASH_ENABLE));
+	if (ret < 0) {
+		dev_err(&rdev->dev, "Failed to disable flash: %d\n", ret);
+		goto autosuspend;
+	}
+
+	ret = regmap_write(rdev->regmap, VC_MIPI_REG_RESET,
+			   VC_MIPI_REG_RESET_POWER_DOWN |
+			   VC_MIPI_REG_RESET_RESET);
+
+autosuspend:
+	pm_runtime_mark_last_busy(ctrl->dev);
+	pm_runtime_put_autosuspend(ctrl->dev);
+
+	ctrl->enabled = false;
+
+	return ret;
+}
+
+static int vc_mipi_regulator_is_enabled(struct regulator_dev *rdev)
+{
+	struct vc_mipi_ctrl *ctrl = rdev_get_drvdata(rdev);
+
+	return ctrl->enabled;
+}
+
+static const struct regulator_ops vc_mipi_regulator_ops = {
+	.enable = vc_mipi_regulator_enable,
+	.disable = vc_mipi_regulator_disable,
+	.is_enabled = vc_mipi_regulator_is_enabled,
+};
+
+static const struct regulator_desc vc_mipi_regulator = {
+	.name = "vc-mipi",
+	.ops = &vc_mipi_regulator_ops,
+	.type = REGULATOR_VOLTAGE,
+	.owner = THIS_MODULE,
+};
+
+static const struct regulator_init_data vc_mipi_regulator_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+};
+
+static int vc_mipi_regulator_init(struct vc_mipi_ctrl *ctrl)
+{
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+
+	config.dev = ctrl->dev;
+	config.init_data = &vc_mipi_regulator_init_data;
+	config.driver_data = ctrl;
+	config.of_node = ctrl->dev->of_node;
+	config.regmap = ctrl->regmap;
+
+	rdev = devm_regulator_register(ctrl->dev, &vc_mipi_regulator, &config);
+	if (IS_ERR(rdev))
+		return PTR_ERR(rdev);
+
+	return 0;
+}
+
+/* -----------------------------------------------------------------------------
+ * Clock
+ */
+
+static int vc_mipi_clk_init(struct vc_mipi_ctrl *ctrl)
+{
+	char name[20];
+	u32 freq;
+	int ret;
+
+	ret = of_property_read_u32(ctrl->dev->of_node, "clock-frequency",
+				   &freq);
+	if (ret < 0) {
+		dev_err(ctrl->dev, "Failed to retrieve clock frequency: %d\n",
+			ret);
+		return ret;
+	}
+
+	/*
+	 * As this is an I2C device, the device name will be in the form
+	 * 'bus-addr', where bus is an integer and addr a 4 characters hex
+	 * value. 20 bytes should be enough as there shouldn't be more than 100
+	 * I2C buses.
+	 */
+	snprintf(name, sizeof(name), "vc-mipi-%s-clk", dev_name(ctrl->dev));
+	ctrl->clk_hw = clk_hw_register_fixed_rate(ctrl->dev, name, NULL, 0,
+						  freq);
+	if (IS_ERR(ctrl->clk_hw))
+		return PTR_ERR(ctrl->clk_hw);
+
+	ret = devm_of_clk_add_hw_provider(ctrl->dev, of_clk_hw_simple_get,
+					  ctrl->clk_hw);
+	if (ret < 0) {
+		clk_hw_unregister_fixed_rate(ctrl->clk_hw);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void vc_mipi_clk_cleanup(struct vc_mipi_ctrl *ctrl)
+{
+	clk_hw_unregister_fixed_rate(ctrl->clk_hw);
+}
+
+/* -----------------------------------------------------------------------------
+ * Power management
+ */
+
+static int vc_mipi_power_on(struct device *dev)
+{
+	struct vc_mipi_ctrl *ctrl = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regulator_enable(ctrl->supply);
+	if (ret < 0) {
+		dev_err(ctrl->dev, "Failed to enable vcc supply: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int vc_mipi_power_off(struct device *dev)
+{
+	struct vc_mipi_ctrl *ctrl = dev_get_drvdata(dev);
+
+	regulator_disable(ctrl->supply);
+
+	return 0;
+}
+
+static const struct dev_pm_ops vc_mipi_pm_ops = {
+	SET_RUNTIME_PM_OPS(vc_mipi_power_off, vc_mipi_power_on, NULL)
+};
+
+/* -----------------------------------------------------------------------------
+ * I2C Proxy
+ */
+
+static int vc_mipi_i2c_tunnel_xfer(struct i2c_adapter *i2c_adapter, struct i2c_msg *msgs, int num)
+{
+	struct i2c_client *i2c = i2c_get_adapdata(i2c_adapter);
+	struct i2c_adapter *parent = i2c->adapter;
+	int ret;
+
+	i2c_lock_bus(parent, I2C_LOCK_SEGMENT);
+	ret = __i2c_transfer(i2c->adapter, msgs, num);
+	i2c_unlock_bus(parent, I2C_LOCK_SEGMENT);
+
+	if (ret < 0)
+		return ret;
+	if (ret != num)
+		return -EIO;
+	return ret;
+}
+
+static u32 vc_mipi_i2c_tunnel_functionality(struct i2c_adapter *i2c_adapter)
+{
+	return I2C_FUNC_I2C;
+}
+
+static const struct i2c_algorithm vc_mipi_i2c_tunnel_algorithm = {
+	.master_xfer   = vc_mipi_i2c_tunnel_xfer,
+	.functionality = vc_mipi_i2c_tunnel_functionality,
+};
+
+/* Controller firmware had a bug which prevented reading more than one byte
+ * via I2C from the sensor. This was (according to the vendor) fixed somewhere
+ * in 2026. Nonetheless we must support also modules running old Firmware.
+ * Therefore set up the i2c quirks accordingly.
+ *
+ * NOTE: This is different from the "I2C reads with address increment return
+ *       the first byte twice" bug, which affects the controller, not the
+ *       sensor.
+ */
+static const struct i2c_adapter_quirks vc_mipi_i2c_tunnel_quirks = {
+	.max_read_len = 1,
+};
+
+static struct device_node *vc_mipi_i2c_find_tunnel_bus(struct device *dev)
+{
+	struct device_node *child;
+
+	for_each_child_of_node(dev->of_node, child) {
+		u32 addr_cells, size_cells;
+
+		if (of_property_read_u32(child, "#address-cells", &addr_cells))
+			continue;
+		if (of_property_read_u32(child, "#size-cells", &size_cells))
+			continue;
+		if (addr_cells != 1 || size_cells != 0)
+			continue;
+
+		return of_node_get(child);
+	}
+
+	return NULL;
+}
+
+static int vc_mipi_i2c_tunnel_add(struct i2c_client *i2c)
+{
+	struct vc_mipi_ctrl *ctrl = i2c_get_clientdata(i2c);
+	struct i2c_adapter *i2c_adapter = &ctrl->i2c_tunnel;
+	struct device_node *bus_node;
+
+	bus_node = vc_mipi_i2c_find_tunnel_bus(ctrl->dev);
+	if (!bus_node)
+		return -EINVAL;
+
+	strscpy(i2c_adapter->name, "vc-mipi-i2c", sizeof(i2c_adapter->name));
+	i2c_adapter->owner       = THIS_MODULE;
+	i2c_adapter->algo        = &vc_mipi_i2c_tunnel_algorithm;
+	i2c_adapter->quirks      = &vc_mipi_i2c_tunnel_quirks;
+	i2c_adapter->dev.parent  = ctrl->dev;
+	i2c_adapter->dev.of_node = bus_node;
+
+	i2c_set_adapdata(i2c_adapter, i2c);
+	return i2c_add_adapter(&ctrl->i2c_tunnel);
+}
+
+static void vc_mipi_i2c_tunnel_del(struct i2c_client *i2c)
+{
+	struct vc_mipi_ctrl *ctrl = i2c_get_clientdata(i2c);
+
+	i2c_del_adapter(&ctrl->i2c_tunnel);
+}
+
+/* -----------------------------------------------------------------------------
+ * Probe & Remove
+ */
+
+/*
+ * Due to a bug in the firmware, I2C reads with address increment return the
+ * first byte twice. The dummy byte at the beginning of the ROM descriptor
+ * works around the issue, at the cost of requiring unaligned accesses.
+ */
+struct vc_mipi_descriptor_rom {
+	u8 dummy;
+	u8 magic[12];
+	u8 manufacturer[32];
+	__le16 mipi_mid;
+	u8 sensor_manufacturer[8];
+	u8 sensor_model[16];
+	__le16 module_id;
+	__le16 module_rev;
+} __packed;
+
+static int vc_mipi_identify(struct vc_mipi_ctrl *ctrl)
+{
+	struct vc_mipi_descriptor_rom rom;
+	unsigned int addr;
+	int ret;
+
+	ret = regmap_raw_read(ctrl->regmap, VC_MIPI_REG_ROM, &rom, sizeof(rom));
+	if (ret < 0) {
+		dev_err(ctrl->dev, "Failed to read ROM: %d\n", ret);
+		return ret;
+	}
+
+	if (memcmp(&rom.magic, "mipi-module", sizeof(rom.magic))) {
+		dev_err(ctrl->dev, "Invalid ROM magic value\n");
+		print_hex_dump(KERN_INFO, "rom: ", DUMP_PREFIX_OFFSET, 16, 1,
+			       &rom, sizeof(rom), true);
+		return -EINVAL;
+	}
+
+	ret = regmap_read(ctrl->regmap, VC_MIPI_REG_SENSOR_ADDR, &addr);
+	if (ret < 0) {
+		dev_err(ctrl->dev, "Failed to read sensor address: %d\n", ret);
+		return ret;
+	}
+
+	dev_info(ctrl->dev, "%.8s %.16s (%04x:%04x @0x%02x)\n",
+		 rom.sensor_manufacturer, rom.sensor_model,
+		 le16_to_cpu(rom.module_id), le16_to_cpu(rom.module_rev), addr);
+
+	return 0;
+}
+
+static const struct regmap_config vc_mipi_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.cache_type = REGCACHE_NONE,
+};
+
+static int vc_mipi_i2c_probe(struct i2c_client *i2c)
+{
+	struct vc_mipi_ctrl *ctrl;
+	int ret;
+
+	ctrl = devm_kzalloc(&i2c->dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->dev = &i2c->dev;
+
+	i2c_set_clientdata(i2c, ctrl);
+
+	ctrl->supply = devm_regulator_get(ctrl->dev, "vcc");
+	if (IS_ERR(ctrl->supply)) {
+		ret = PTR_ERR(ctrl->supply);
+		dev_err(ctrl->dev, "Failed to get vcc supply: %d\n", ret);
+		return ret;
+	}
+
+	ctrl->regmap = devm_regmap_init_i2c(i2c, &vc_mipi_regmap_config);
+	if (IS_ERR(ctrl->regmap)) {
+		ret = PTR_ERR(ctrl->regmap);
+		dev_err(ctrl->dev, "Failed to init regmap: %d\n", ret);
+		return ret;
+	}
+
+	ret = vc_mipi_power_on(ctrl->dev);
+	if (ret < 0)
+		return ret;
+
+	ret = vc_mipi_identify(ctrl);
+	if (ret < 0)
+		goto err_power;
+
+	ret = vc_mipi_regulator_init(ctrl);
+	if (ret < 0) {
+		dev_err(ctrl->dev, "Failed to register regulator\n");
+		goto err_power;
+	}
+
+	ret = vc_mipi_clk_init(ctrl);
+	if (ret < 0) {
+		dev_err(ctrl->dev, "Failed to register clock\n");
+		goto err_power;
+	}
+
+	/* Enable runtime PM and turn off the device. */
+	pm_runtime_set_active(ctrl->dev);
+	pm_runtime_get_noresume(ctrl->dev);
+	pm_runtime_enable(ctrl->dev);
+	pm_runtime_set_autosuspend_delay(ctrl->dev, 1000);
+	pm_runtime_use_autosuspend(ctrl->dev);
+	pm_runtime_mark_last_busy(ctrl->dev);
+	pm_runtime_put_autosuspend(ctrl->dev);
+
+	/* As a last step create the proxied downstream I2C adapter */
+	ret = vc_mipi_i2c_tunnel_add(i2c);
+	if (ret < 0) {
+		dev_err(ctrl->dev, "Failed to register i2c tunnel adapter: %d\n", ret);
+		goto err_power;
+	}
+
+	return 0;
+
+err_power:
+	vc_mipi_power_off(ctrl->dev);
+	return ret;
+}
+
+static void vc_mipi_i2c_remove(struct i2c_client *i2c)
+{
+	struct vc_mipi_ctrl *ctrl = i2c_get_clientdata(i2c);
+
+	vc_mipi_i2c_tunnel_del(i2c);
+
+	vc_mipi_clk_cleanup(ctrl);
+
+	pm_runtime_disable(ctrl->dev);
+	if (!pm_runtime_status_suspended(ctrl->dev))
+		vc_mipi_power_off(ctrl->dev);
+	pm_runtime_set_suspended(ctrl->dev);
+}
+
+static const struct of_device_id vc_mipi_dt_ids[] = {
+	{ .compatible = "vision-components,mipi-module-controller" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, vc_mipi_dt_ids);
+
+static struct i2c_driver vc_mipi_driver = {
+	.driver = {
+		.name = "vc-mipi",
+		.of_match_table = vc_mipi_dt_ids,
+		.pm = &vc_mipi_pm_ops,
+	},
+	.probe = vc_mipi_i2c_probe,
+	.remove = vc_mipi_i2c_remove,
+};
+
+module_i2c_driver(vc_mipi_driver);
+
+MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
+MODULE_DESCRIPTION("Driver for the Vision Components MIPI Module Controller");
+MODULE_LICENSE("GPL");

-- 
2.53.0

Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller

[Rob Herring (Arm)]

On Tue, 15 Sep 2026 22:20:24 +0200, Richard Leitner wrote:
> Add bindings for the Vision Components MIPI Camera Module Controller.
> 
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> ---
>  .../vision-components,mipi-module-controller.yaml  | 92 ++++++++++++++++++++++
>  MAINTAINERS                                        |  7 ++
>  2 files changed, 99 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.example.dts:32.23-47.19: Warning (unit_address_vs_reg): /example-0/i2c/controller@10/i2c@0: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.example.dtb: controller@10 (vision-components,mipi-module-controller): Unevaluated properties are not allowed ('i2c@0' was unexpected)
	from schema $id: http://devicetree.org/schemas/media/i2c/vision-components,mipi-module-controller.yaml
Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.example.dtb: camera@60 (ovti,ov9281): 'clock-names' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov9282.yaml
Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.example.dtb: camera@60 (ovti,ov9281): 'port' is a required property
	from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov9282.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260915-vc-mipi-ctrl-v1-2-8a42b693d889@linux.dev

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller

[Krzysztof Kozlowski]

On Tue, Sep 15, 2026 at 10:20:24PM +0200, Richard Leitner wrote:
> Add bindings for the Vision Components MIPI Camera Module Controller.
> 
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> ---
>  .../vision-components,mipi-module-controller.yaml  | 92 ++++++++++++++++++++++
>  MAINTAINERS                                        |  7 ++
>  2 files changed, 99 insertions(+)

This fails tests, so a very brief review / a few comments:

> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
> new file mode 100644
> index 0000000000000..2a031aea68457
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/vision-components,mipi-module-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Vision Components MIPI Camera Module Controller
> +
> +maintainers:
> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +  - Richard Leitner <richard.leitner@linux.dev>
> +
> +description: |-
> +  The MIPI camera module controller is an FPGA-based, I2C-accessible system
> +  controller found on MIPI camera modules from Vision Components. It abstracts
> +  the camera sensor behind a unified register interface, and controls and
> +  sequences the on-board power supplies and clocks.
> +
> +  The camera sensor abstraction is optional. The controller exposes a tunneled
> +  downstream I2C bus used by the attached image sensor. The controller node
> +  acts as the upstream device on the host bus, while child node below the
> +  controller represent the sensor device reachable through the tunnel.
> +
> +properties:
> +  compatible:
> +    const: vision-components,mipi-module-controller

There is no model name, no version, nothing identifying it better?
Compatible must be specific to the device (see also writing bindings).

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 0
> +
> +  clock-frequency:
> +    description: Frequency of the sensor clock provided by the module

Drop, implied by the compatible

> +
> +  vcc-supply:
> +    description: Power supply of the module (3.3V)
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

No children allowed, so why these two?

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#clock-cells'
> +  - clock-frequency
> +  - vcc-supply
> +  - '#address-cells'
> +  - '#size-cells'
> +
> +unevaluatedProperties: false

additionalProperties instead, see writing bindings or writing schema.
Unless you miss here some other schema $ref.

> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        vc_mipi_ctrl: controller@10 {
> +            compatible = "vision-components,mipi-module-controller";
> +            reg = <0x10>;
> +            #clock-cells = <0>;
> +            clock-frequency = <37125000>;
> +            vcc-supply = <&cam_3v3>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            i2c@0 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                vc_mipi_sensor: camera@60 {
> +                    compatible = "ovti,ov9281";
> +                    reg = <0x60>;

Why having the child abstraction if it is completely abstracted? I don't
fully get the explanation from description. Completely optional means no
benefits, no point in it, no?

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller

[Richard Leitner]

Hi Krzysztof,

thanks for the review!

On Wed, Sep 16, 2026 at 09:00:46AM +0200, Krzysztof Kozlowski wrote:
> On Tue, Sep 15, 2026 at 10:20:24PM +0200, Richard Leitner wrote:
> > Add bindings for the Vision Components MIPI Camera Module Controller.
> > 
> > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > ---
> >  .../vision-components,mipi-module-controller.yaml  | 92 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  7 ++
> >  2 files changed, 99 insertions(+)
> 
> This fails tests, so a very brief review / a few comments:

Mea culpa, I simply failed to run the dtb check before submitting. Sorry.
Will not happen again.

> 
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
> > new file mode 100644
> > index 0000000000000..2a031aea68457
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
> > @@ -0,0 +1,92 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/vision-components,mipi-module-controller.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Vision Components MIPI Camera Module Controller
> > +
> > +maintainers:
> > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > +  - Richard Leitner <richard.leitner@linux.dev>
> > +
> > +description: |-
> > +  The MIPI camera module controller is an FPGA-based, I2C-accessible system
> > +  controller found on MIPI camera modules from Vision Components. It abstracts
> > +  the camera sensor behind a unified register interface, and controls and
> > +  sequences the on-board power supplies and clocks.
> > +
> > +  The camera sensor abstraction is optional. The controller exposes a tunneled
> > +  downstream I2C bus used by the attached image sensor. The controller node
> > +  acts as the upstream device on the host bus, while child node below the
> > +  controller represent the sensor device reachable through the tunnel.
> > +
> > +properties:
> > +  compatible:
> > +    const: vision-components,mipi-module-controller
> 
> There is no model name, no version, nothing identifying it better?
> Compatible must be specific to the device (see also writing bindings).

There is one FPGA implemenation for all Vision Components MIPI camera
modules AFAICT. So I have no idea how it could be more specific, TBH...

Adding the FPGA hardware model/type is wrong IMHO as it depends on its
configuration/firmware, not the hw.

There are different versions of the configuration/firmware. But I think
this is also not the correct way to distinguish those, or is it?

According to their homepage the vendor calls those modules simply
"VC MIPI" modules, which implies this FPGA controller is available on it.
So maybe "vision-components,vc-mipi-controller" would be a better fit?

Do you have any ideas/feedback on how improve this name?

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#clock-cells':
> > +    const: 0
> > +
> > +  clock-frequency:
> > +    description: Frequency of the sensor clock provided by the module
> 
> Drop, implied by the compatible
> 

Do you mean dropping the whole property, or just the "description"?

> > +
> > +  vcc-supply:
> > +    description: Power supply of the module (3.3V)
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> 
> No children allowed, so why these two?

Based on your and the bot feedback I would suggest for v2 to change this
"generic i2c bus" to a simple "i2c-tunnel" property which has
"$ref: /schemas/i2c/i2c-controller.yaml".

This would better reflect the actual hardware, as there is only this one,
in firmware hard-coded I2C downstream bus.

Would this be a sane approach?

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#clock-cells'
> > +  - clock-frequency
> > +  - vcc-supply
> > +  - '#address-cells'
> > +  - '#size-cells'
> > +
> > +unevaluatedProperties: false
> 
> additionalProperties instead, see writing bindings or writing schema.
> Unless you miss here some other schema $ref.
> 
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        vc_mipi_ctrl: controller@10 {
> > +            compatible = "vision-components,mipi-module-controller";
> > +            reg = <0x10>;
> > +            #clock-cells = <0>;
> > +            clock-frequency = <37125000>;
> > +            vcc-supply = <&cam_3v3>;
> > +
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            i2c@0 {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                vc_mipi_sensor: camera@60 {
> > +                    compatible = "ovti,ov9281";
> > +                    reg = <0x60>;
> 
> Why having the child abstraction if it is completely abstracted? I don't
> fully get the explanation from description. Completely optional means no
> benefits, no point in it, no?

I will try to improve the description. The idea behind this device
driver/devicetree node is to not rely on the sensor abstraction from
vision components, but to use the upstream sensor specific driver.

As the tunneled downstream i2c bus needs quirks applied this is modeled
as a separate bus.

Previous discussions on this approach are available at:
	https://lore.kernel.org/lkml/20260728092856.GB1494774@killaraus.ideasonboard.com/

Thanks again for your review Krzysztof!

regards;rl

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller

[Laurent Pinchart]

On Wed, Sep 16, 2026 at 09:33:36AM +0200, Richard Leitner wrote:
> On Wed, Sep 16, 2026 at 09:00:46AM +0200, Krzysztof Kozlowski wrote:
> > On Tue, Sep 15, 2026 at 10:20:24PM +0200, Richard Leitner wrote:
> > > Add bindings for the Vision Components MIPI Camera Module Controller.
> > > 
> > > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > > ---
> > >  .../vision-components,mipi-module-controller.yaml  | 92 ++++++++++++++++++++++
> > >  MAINTAINERS                                        |  7 ++
> > >  2 files changed, 99 insertions(+)
> > 
> > This fails tests, so a very brief review / a few comments:
> 
> Mea culpa, I simply failed to run the dtb check before submitting. Sorry.
> Will not happen again.
> 
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
> > > new file mode 100644
> > > index 0000000000000..2a031aea68457
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
> > > @@ -0,0 +1,92 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/vision-components,mipi-module-controller.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Vision Components MIPI Camera Module Controller
> > > +
> > > +maintainers:
> > > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > +  - Richard Leitner <richard.leitner@linux.dev>
> > > +
> > > +description: |-
> > > +  The MIPI camera module controller is an FPGA-based, I2C-accessible system
> > > +  controller found on MIPI camera modules from Vision Components. It abstracts
> > > +  the camera sensor behind a unified register interface, and controls and
> > > +  sequences the on-board power supplies and clocks.
> > > +
> > > +  The camera sensor abstraction is optional. The controller exposes a tunneled
> > > +  downstream I2C bus used by the attached image sensor. The controller node
> > > +  acts as the upstream device on the host bus, while child node below the
> > > +  controller represent the sensor device reachable through the tunnel.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: vision-components,mipi-module-controller
> > 
> > There is no model name, no version, nothing identifying it better?
> > Compatible must be specific to the device (see also writing bindings).
> 
> There is one FPGA implemenation for all Vision Components MIPI camera
> modules AFAICT. So I have no idea how it could be more specific, TBH...
> 
> Adding the FPGA hardware model/type is wrong IMHO as it depends on its
> configuration/firmware, not the hw.
> 
> There are different versions of the configuration/firmware. But I think
> this is also not the correct way to distinguish those, or is it?
> 
> According to their homepage the vendor calls those modules simply
> "VC MIPI" modules, which implies this FPGA controller is available on it.
> So maybe "vision-components,vc-mipi-controller" would be a better fit?
> 
> Do you have any ideas/feedback on how improve this name?
> 
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  '#clock-cells':
> > > +    const: 0
> > > +
> > > +  clock-frequency:
> > > +    description: Frequency of the sensor clock provided by the module
> > 
> > Drop, implied by the compatible
> 
> Do you mean dropping the whole property, or just the "description"?

Different modules have different clock frequencies, don't they ? The
property seems needed.

> > > +
> > > +  vcc-supply:
> > > +    description: Power supply of the module (3.3V)
> > > +
> > > +  '#address-cells':
> > > +    const: 1
> > > +
> > > +  '#size-cells':
> > > +    const: 0
> > 
> > No children allowed, so why these two?
> 
> Based on your and the bot feedback I would suggest for v2 to change this
> "generic i2c bus" to a simple "i2c-tunnel" property which has
> "$ref: /schemas/i2c/i2c-controller.yaml".
> 
> This would better reflect the actual hardware, as there is only this one,
> in firmware hard-coded I2C downstream bus.
> 
> Would this be a sane approach?
> 
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - '#clock-cells'
> > > +  - clock-frequency
> > > +  - vcc-supply
> > > +  - '#address-cells'
> > > +  - '#size-cells'
> > > +
> > > +unevaluatedProperties: false
> > 
> > additionalProperties instead, see writing bindings or writing schema.
> > Unless you miss here some other schema $ref.
> > 
> > > +
> > > +examples:
> > > +  - |
> > > +    i2c {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        vc_mipi_ctrl: controller@10 {
> > > +            compatible = "vision-components,mipi-module-controller";
> > > +            reg = <0x10>;
> > > +            #clock-cells = <0>;
> > > +            clock-frequency = <37125000>;
> > > +            vcc-supply = <&cam_3v3>;
> > > +
> > > +            #address-cells = <1>;
> > > +            #size-cells = <0>;
> > > +
> > > +            i2c@0 {
> > > +                #address-cells = <1>;
> > > +                #size-cells = <0>;
> > > +
> > > +                vc_mipi_sensor: camera@60 {
> > > +                    compatible = "ovti,ov9281";
> > > +                    reg = <0x60>;
> > 
> > Why having the child abstraction if it is completely abstracted? I don't
> > fully get the explanation from description. Completely optional means no
> > benefits, no point in it, no?
> 
> I will try to improve the description. The idea behind this device
> driver/devicetree node is to not rely on the sensor abstraction from
> vision components, but to use the upstream sensor specific driver.
> 
> As the tunneled downstream i2c bus needs quirks applied this is modeled
> as a separate bus.
> 
> Previous discussions on this approach are available at:
> 	https://lore.kernel.org/lkml/20260728092856.GB1494774@killaraus.ideasonboard.com/
> 
> Thanks again for your review Krzysztof!

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller

[Krzysztof Kozlowski]

On 16/09/2026 09:45, Laurent Pinchart wrote:
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  '#clock-cells':
>>>> +    const: 0
>>>> +
>>>> +  clock-frequency:
>>>> +    description: Frequency of the sensor clock provided by the module
>>>
>>> Drop, implied by the compatible
>>
>> Do you mean dropping the whole property, or just the "description"?
> 
> Different modules have different clock frequencies, don't they ? The
> property seems needed.

Yes, that would be fine with assumption there is no actual sensor
described. If there is a sensor in DT, that one will have a proper
driver binding and asking for frequency?

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller

[Krzysztof Kozlowski]

On 16/09/2026 09:33, Richard Leitner wrote:
> Hi Krzysztof,
> 
> thanks for the review!
> 
> On Wed, Sep 16, 2026 at 09:00:46AM +0200, Krzysztof Kozlowski wrote:
>> On Tue, Sep 15, 2026 at 10:20:24PM +0200, Richard Leitner wrote:
>>> Add bindings for the Vision Components MIPI Camera Module Controller.
>>>
>>> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
>>> ---
>>>  .../vision-components,mipi-module-controller.yaml  | 92 ++++++++++++++++++++++
>>>  MAINTAINERS                                        |  7 ++
>>>  2 files changed, 99 insertions(+)
>>
>> This fails tests, so a very brief review / a few comments:
> 
> Mea culpa, I simply failed to run the dtb check before submitting. Sorry.
> Will not happen again.
> 
>>
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
>>> new file mode 100644
>>> index 0000000000000..2a031aea68457
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
>>> @@ -0,0 +1,92 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/media/i2c/vision-components,mipi-module-controller.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Vision Components MIPI Camera Module Controller
>>> +
>>> +maintainers:
>>> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> +  - Richard Leitner <richard.leitner@linux.dev>
>>> +
>>> +description: |-
>>> +  The MIPI camera module controller is an FPGA-based, I2C-accessible system
>>> +  controller found on MIPI camera modules from Vision Components. It abstracts
>>> +  the camera sensor behind a unified register interface, and controls and
>>> +  sequences the on-board power supplies and clocks.
>>> +
>>> +  The camera sensor abstraction is optional. The controller exposes a tunneled
>>> +  downstream I2C bus used by the attached image sensor. The controller node
>>> +  acts as the upstream device on the host bus, while child node below the
>>> +  controller represent the sensor device reachable through the tunnel.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: vision-components,mipi-module-controller
>>
>> There is no model name, no version, nothing identifying it better?
>> Compatible must be specific to the device (see also writing bindings).
> 
> There is one FPGA implemenation for all Vision Components MIPI camera
> modules AFAICT. So I have no idea how it could be more specific, TBH...
> 
> Adding the FPGA hardware model/type is wrong IMHO as it depends on its
> configuration/firmware, not the hw.
> 
> There are different versions of the configuration/firmware. But I think
> this is also not the correct way to distinguish those, or is it?
> 
> According to their homepage the vendor calls those modules simply
> "VC MIPI" modules, which implies this FPGA controller is available on it.
> So maybe "vision-components,vc-mipi-controller" would be a better fit?
> 
> Do you have any ideas/feedback on how improve this name?

So there are different modules? I see several different names on:
https://www.mipi-modules.com/en/mipi-camera-modules/


> 
>>
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  '#clock-cells':
>>> +    const: 0
>>> +
>>> +  clock-frequency:
>>> +    description: Frequency of the sensor clock provided by the module
>>
>> Drop, implied by the compatible
>>
> 
> Do you mean dropping the whole property, or just the "description"?

I meant entire property, but we keep discussing in Laurent's reply.

> 
>>> +
>>> +  vcc-supply:
>>> +    description: Power supply of the module (3.3V)
>>> +
>>> +  '#address-cells':
>>> +    const: 1
>>> +
>>> +  '#size-cells':
>>> +    const: 0
>>
>> No children allowed, so why these two?
> 
> Based on your and the bot feedback I would suggest for v2 to change this
> "generic i2c bus" to a simple "i2c-tunnel" property which has
> "$ref: /schemas/i2c/i2c-controller.yaml".
> 
> This would better reflect the actual hardware, as there is only this one,
> in firmware hard-coded I2C downstream bus.
> 
> Would this be a sane approach?

If the underlying I2C bus and sensor are important, then yes. But I have
doubts that you need to describe the sensor if it is truly
unadressable/invisible to the OS.

> 
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - '#clock-cells'
>>> +  - clock-frequency
>>> +  - vcc-supply
>>> +  - '#address-cells'
>>> +  - '#size-cells'
>>> +
>>> +unevaluatedProperties: false
>>
>> additionalProperties instead, see writing bindings or writing schema.
>> Unless you miss here some other schema $ref.
>>
>>> +
>>> +examples:
>>> +  - |
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        vc_mipi_ctrl: controller@10 {
>>> +            compatible = "vision-components,mipi-module-controller";
>>> +            reg = <0x10>;
>>> +            #clock-cells = <0>;
>>> +            clock-frequency = <37125000>;
>>> +            vcc-supply = <&cam_3v3>;
>>> +
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +
>>> +            i2c@0 {
>>> +                #address-cells = <1>;
>>> +                #size-cells = <0>;
>>> +
>>> +                vc_mipi_sensor: camera@60 {
>>> +                    compatible = "ovti,ov9281";
>>> +                    reg = <0x60>;
>>
>> Why having the child abstraction if it is completely abstracted? I don't
>> fully get the explanation from description. Completely optional means no
>> benefits, no point in it, no?
> 
> I will try to improve the description. The idea behind this device
> driver/devicetree node is to not rely on the sensor abstraction from
> vision components, but to use the upstream sensor specific driver.

You can use driver even without these nodes... but fine, let's assume
you have them, so driver will talk with OV9281 sensor for example?

> 
> As the tunneled downstream i2c bus needs quirks applied this is modeled
> as a separate bus.
> 
> Previous discussions on this approach are available at:
> 	https://lore.kernel.org/lkml/20260728092856.GB1494774@killaraus.ideasonboard.com/
> 
Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller

[Richard Leitner]

On Wed, Sep 16, 2026 at 10:35:22AM +0200, Krzysztof Kozlowski wrote:
> On 16/09/2026 09:33, Richard Leitner wrote:
> > Hi Krzysztof,
> > 
> > thanks for the review!
> > 
> > On Wed, Sep 16, 2026 at 09:00:46AM +0200, Krzysztof Kozlowski wrote:
> >> On Tue, Sep 15, 2026 at 10:20:24PM +0200, Richard Leitner wrote:
> >>> Add bindings for the Vision Components MIPI Camera Module Controller.
> >>>
> >>> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> >>> ---
> >>>  .../vision-components,mipi-module-controller.yaml  | 92 ++++++++++++++++++++++
> >>>  MAINTAINERS                                        |  7 ++
> >>>  2 files changed, 99 insertions(+)
> >>
> >> This fails tests, so a very brief review / a few comments:
> > 
> > Mea culpa, I simply failed to run the dtb check before submitting. Sorry.
> > Will not happen again.
> > 
> >>
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
> >>> new file mode 100644
> >>> index 0000000000000..2a031aea68457
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/media/i2c/vision-components,mipi-module-controller.yaml
> >>> @@ -0,0 +1,92 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/media/i2c/vision-components,mipi-module-controller.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Vision Components MIPI Camera Module Controller
> >>> +
> >>> +maintainers:
> >>> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>> +  - Richard Leitner <richard.leitner@linux.dev>
> >>> +
> >>> +description: |-
> >>> +  The MIPI camera module controller is an FPGA-based, I2C-accessible system
> >>> +  controller found on MIPI camera modules from Vision Components. It abstracts
> >>> +  the camera sensor behind a unified register interface, and controls and
> >>> +  sequences the on-board power supplies and clocks.
> >>> +
> >>> +  The camera sensor abstraction is optional. The controller exposes a tunneled
> >>> +  downstream I2C bus used by the attached image sensor. The controller node
> >>> +  acts as the upstream device on the host bus, while child node below the
> >>> +  controller represent the sensor device reachable through the tunnel.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    const: vision-components,mipi-module-controller
> >>
> >> There is no model name, no version, nothing identifying it better?
> >> Compatible must be specific to the device (see also writing bindings).
> > 
> > There is one FPGA implemenation for all Vision Components MIPI camera
> > modules AFAICT. So I have no idea how it could be more specific, TBH...
> > 
> > Adding the FPGA hardware model/type is wrong IMHO as it depends on its
> > configuration/firmware, not the hw.
> > 
> > There are different versions of the configuration/firmware. But I think
> > this is also not the correct way to distinguish those, or is it?
> > 
> > According to their homepage the vendor calls those modules simply
> > "VC MIPI" modules, which implies this FPGA controller is available on it.
> > So maybe "vision-components,vc-mipi-controller" would be a better fit?
> > 
> > Do you have any ideas/feedback on how improve this name?
> 
> So there are different modules? I see several different names on:
> https://www.mipi-modules.com/en/mipi-camera-modules/

Yes, there are different modules, but all feature the same controller.
Which this is basically the device driver binding for. So the idea is to
describe the controller independently from the sensor which is "behind"
it.

This works because the controller "soft-core" should be the same on all
modules. A downstream implemenation (which I haven't studied in detail)
is available at https://github.com/VC-MIPI-modules/vc_mipi_core if that
helps?

> 
> 
> > 
> >>
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  '#clock-cells':
> >>> +    const: 0
> >>> +
> >>> +  clock-frequency:
> >>> +    description: Frequency of the sensor clock provided by the module
> >>
> >> Drop, implied by the compatible
> >>
> > 
> > Do you mean dropping the whole property, or just the "description"?
> 
> I meant entire property, but we keep discussing in Laurent's reply.
> 
> > 
> >>> +
> >>> +  vcc-supply:
> >>> +    description: Power supply of the module (3.3V)
> >>> +
> >>> +  '#address-cells':
> >>> +    const: 1
> >>> +
> >>> +  '#size-cells':
> >>> +    const: 0
> >>
> >> No children allowed, so why these two?
> > 
> > Based on your and the bot feedback I would suggest for v2 to change this
> > "generic i2c bus" to a simple "i2c-tunnel" property which has
> > "$ref: /schemas/i2c/i2c-controller.yaml".
> > 
> > This would better reflect the actual hardware, as there is only this one,
> > in firmware hard-coded I2C downstream bus.
> > 
> > Would this be a sane approach?
> 
> If the underlying I2C bus and sensor are important, then yes. But I have
> doubts that you need to describe the sensor if it is truly
> unadressable/invisible to the OS.

Yes, the I2C bus and sensor is important. The device driver of the sensor
talks (via the tunneled I2C interface) directly to the sensor.

The separate I2C controller/bus description is necessary as the tunneled
I2C bus has some quirks unfortunately. Those need to be addressed as 
otherwise the sensor drivers do not work.

So the idea is to not have a vc-mipi module binding per "sensor variant"
of the camera modules, but provide a common controller driver which
provides the I2C bus for the sensor driver.

> 
> > 
> >>
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +  - '#clock-cells'
> >>> +  - clock-frequency
> >>> +  - vcc-supply
> >>> +  - '#address-cells'
> >>> +  - '#size-cells'
> >>> +
> >>> +unevaluatedProperties: false
> >>
> >> additionalProperties instead, see writing bindings or writing schema.
> >> Unless you miss here some other schema $ref.
> >>
> >>> +
> >>> +examples:
> >>> +  - |
> >>> +    i2c {
> >>> +        #address-cells = <1>;
> >>> +        #size-cells = <0>;
> >>> +
> >>> +        vc_mipi_ctrl: controller@10 {
> >>> +            compatible = "vision-components,mipi-module-controller";
> >>> +            reg = <0x10>;
> >>> +            #clock-cells = <0>;
> >>> +            clock-frequency = <37125000>;
> >>> +            vcc-supply = <&cam_3v3>;
> >>> +
> >>> +            #address-cells = <1>;
> >>> +            #size-cells = <0>;
> >>> +
> >>> +            i2c@0 {
> >>> +                #address-cells = <1>;
> >>> +                #size-cells = <0>;
> >>> +
> >>> +                vc_mipi_sensor: camera@60 {
> >>> +                    compatible = "ovti,ov9281";
> >>> +                    reg = <0x60>;
> >>
> >> Why having the child abstraction if it is completely abstracted? I don't
> >> fully get the explanation from description. Completely optional means no
> >> benefits, no point in it, no?
> > 
> > I will try to improve the description. The idea behind this device
> > driver/devicetree node is to not rely on the sensor abstraction from
> > vision components, but to use the upstream sensor specific driver.
> 
> You can use driver even without these nodes... but fine, let's assume
> you have them, so driver will talk with OV9281 sensor for example?

The vc-mipi driver does not talk to sensor at all. This is done by the
dedicated sensor driver. The vc-mipi driver is only controlling the
regulator, clock, etc. as described and sets up the "quirk aware"
tunneled i2c interface.

> 
> > 
> > As the tunneled downstream i2c bus needs quirks applied this is modeled
> > as a separate bus.
> > 
> > Previous discussions on this approach are available at:
> > 	https://lore.kernel.org/lkml/20260728092856.GB1494774@killaraus.ideasonboard.com/
> > 
> Best regards,
> Krzysztof

If you have any other questions or something is still unclear to you,
please feel free to reach out!

regards;rl