* [PATCH 0/3] Add Vision Components MIPI Camera Module support
@ 2026-09-15 20:20 Richard Leitner
2026-09-15 20:20 ` [PATCH 1/3] dt-bindings: vendor-prefixes: add Vision Components GmbH Richard Leitner
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Richard Leitner @ 2026-09-15 20:20 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart
Cc: Alexander Stein, devicetree, linux-kernel, linux-media, 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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] dt-bindings: vendor-prefixes: add Vision Components GmbH
2026-09-15 20:20 [PATCH 0/3] Add Vision Components MIPI Camera Module support Richard Leitner
@ 2026-09-15 20:20 ` Richard Leitner
2026-09-15 20:20 ` [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller Richard Leitner
2026-09-15 20:20 ` [PATCH 3/3] media: i2c: vc-mipi: add Vision Components MIPI Module Controller Richard Leitner
2 siblings, 0 replies; 12+ messages in thread
From: Richard Leitner @ 2026-09-15 20:20 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart
Cc: Alexander Stein, devicetree, linux-kernel, linux-media, 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-15 20:20 [PATCH 0/3] Add Vision Components MIPI Camera Module support Richard Leitner
2026-09-15 20:20 ` [PATCH 1/3] dt-bindings: vendor-prefixes: add Vision Components GmbH Richard Leitner
@ 2026-09-15 20:20 ` Richard Leitner
2026-09-15 21:57 ` Rob Herring (Arm)
2026-09-16 7:00 ` Krzysztof Kozlowski
2026-09-15 20:20 ` [PATCH 3/3] media: i2c: vc-mipi: add Vision Components MIPI Module Controller Richard Leitner
2 siblings, 2 replies; 12+ messages in thread
From: Richard Leitner @ 2026-09-15 20:20 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart
Cc: Alexander Stein, devicetree, linux-kernel, linux-media, 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] media: i2c: vc-mipi: add Vision Components MIPI Module Controller
2026-09-15 20:20 [PATCH 0/3] Add Vision Components MIPI Camera Module support Richard Leitner
2026-09-15 20:20 ` [PATCH 1/3] dt-bindings: vendor-prefixes: add Vision Components GmbH Richard Leitner
2026-09-15 20:20 ` [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller Richard Leitner
@ 2026-09-15 20:20 ` Richard Leitner
2 siblings, 0 replies; 12+ messages in thread
From: Richard Leitner @ 2026-09-15 20:20 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart
Cc: Alexander Stein, devicetree, linux-kernel, linux-media, 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-15 20:20 ` [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller Richard Leitner
@ 2026-09-15 21:57 ` Rob Herring (Arm)
2026-09-16 7:00 ` Krzysztof Kozlowski
1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2026-09-15 21:57 UTC (permalink / raw)
To: Richard Leitner
Cc: Mauro Carvalho Chehab, Alexander Stein, Krzysztof Kozlowski,
devicetree, Laurent Pinchart, linux-media, linux-kernel,
Conor Dooley
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-15 20:20 ` [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller Richard Leitner
2026-09-15 21:57 ` Rob Herring (Arm)
@ 2026-09-16 7:00 ` Krzysztof Kozlowski
2026-09-16 7:33 ` Richard Leitner
1 sibling, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-16 7:00 UTC (permalink / raw)
To: Richard Leitner
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart, Alexander Stein,
devicetree, linux-kernel, linux-media
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-16 7:00 ` Krzysztof Kozlowski
@ 2026-09-16 7:33 ` Richard Leitner
2026-09-16 7:45 ` Laurent Pinchart
2026-09-16 8:35 ` Krzysztof Kozlowski
0 siblings, 2 replies; 12+ messages in thread
From: Richard Leitner @ 2026-09-16 7:33 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart, Alexander Stein,
devicetree, linux-kernel, linux-media
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-16 7:33 ` Richard Leitner
@ 2026-09-16 7:45 ` Laurent Pinchart
2026-09-16 8:30 ` Krzysztof Kozlowski
2026-09-16 8:35 ` Krzysztof Kozlowski
1 sibling, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2026-09-16 7:45 UTC (permalink / raw)
To: Richard Leitner
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, Alexander Stein, devicetree,
linux-kernel, linux-media
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-16 7:45 ` Laurent Pinchart
@ 2026-09-16 8:30 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-16 8:30 UTC (permalink / raw)
To: Laurent Pinchart, Richard Leitner
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Alexander Stein, devicetree, linux-kernel,
linux-media
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-16 7:33 ` Richard Leitner
2026-09-16 7:45 ` Laurent Pinchart
@ 2026-09-16 8:35 ` Krzysztof Kozlowski
2026-09-16 9:01 ` Richard Leitner
1 sibling, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-16 8:35 UTC (permalink / raw)
To: Richard Leitner
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart, Alexander Stein,
devicetree, linux-kernel, linux-media
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-16 8:35 ` Krzysztof Kozlowski
@ 2026-09-16 9:01 ` Richard Leitner
2026-09-16 13:41 ` Krzysztof Kozlowski
0 siblings, 1 reply; 12+ messages in thread
From: Richard Leitner @ 2026-09-16 9:01 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart, Alexander Stein,
devicetree, linux-kernel, linux-media
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller
2026-09-16 9:01 ` Richard Leitner
@ 2026-09-16 13:41 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-16 13:41 UTC (permalink / raw)
To: Richard Leitner
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Laurent Pinchart, Alexander Stein,
devicetree, linux-kernel, linux-media
On 16/09/2026 11:01, Richard Leitner wrote:
> 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.
Bindings must accurately describe the device and so far - based on the
website - there is no device as mipi-module-controller alone.
I don't get why you assume that all of the variants are exactly
identical, thus sensor variant is not applicable.
If they are identical in all aspects, then why clock-frequency property?
That's obviously rhetorical question, because they are not identical in
all aspects and must produce different clock at least.
>
>>
>>>
>>>>
>>>>> +
>>>>> +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.
I meant, driver for the sensor. So who controls sensor supplies? Not the
sensor driver? It tells something how the hardware is managed, no?
>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-09-16 13:41 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15 20:20 [PATCH 0/3] Add Vision Components MIPI Camera Module support Richard Leitner
2026-09-15 20:20 ` [PATCH 1/3] dt-bindings: vendor-prefixes: add Vision Components GmbH Richard Leitner
2026-09-15 20:20 ` [PATCH 2/3] dt-bindings: media: i2c: Add vision-components,mipi-module-controller Richard Leitner
2026-09-15 21:57 ` Rob Herring (Arm)
2026-09-16 7:00 ` Krzysztof Kozlowski
2026-09-16 7:33 ` Richard Leitner
2026-09-16 7:45 ` Laurent Pinchart
2026-09-16 8:30 ` Krzysztof Kozlowski
2026-09-16 8:35 ` Krzysztof Kozlowski
2026-09-16 9:01 ` Richard Leitner
2026-09-16 13:41 ` Krzysztof Kozlowski
2026-09-15 20:20 ` [PATCH 3/3] media: i2c: vc-mipi: add Vision Components MIPI Module Controller Richard Leitner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®