mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support
@ 2026-09-18 22:16 Peter Marshall
  2026-09-18 22:16 ` [PATCH 01/11] dt-bindings: media: i2c: st,vd55g1: Move allOf: after required: Peter Marshall
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:16 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Dan Scally, Hans de Goede, Ilpo Järvinen

This patch series refactors the STMicroelectronics VD55G1 and VD65G4 image
sensor driver (vd55g1) to abstract sensor differences and adds support for
the VD55G0 monochrome sensor.

The VD55G0 is used as the IR face ID sensor in devices such as the
Microsoft Surface Pro 9, where it is found under ACPI HID `SMO55F0`. It
features a native resolution of 644 x 604 pixels and differs from later
variants in the family in several ways:

* It uses a different native resolution and register addresses mapping,
  requiring specification of generational feature differences.
* It lacks support for spatial and temporal HDR.
* It requires an exposure duty cycle limit to keep the illuminator current
  within safe levels.

Tested on a Microsoft Surface Pro 9 (an Intel IPU6 platform with ACPI
HID "SMO55F0") running kernel 7.2.0 (changes cherry-picked) and libcamera
0.7.0. Builds successfully on linux-media next.

Key changes from the previous series:
* The vd55g1 module is grown to support the extra hardware instead of
  replacing it with a completely new driver.
* The driver now includes a builtin patch array. External firmware loading
  is no longer required.
* Lower-level error messages are demoted to the debug level to keep kernel
  logs clean and actionable.
* Hardware registers are now fully abstracted, so operational logic never
  needs to touch a raw CCI address.
* Changes are broken into logical steps to improve clarity and make
  bisection possible.
* Rebased against linux-media next.

Peter Marshall (11):
  dt-bindings: media: i2c: st,vd55g1: Move allOf: after required:
  media: dt-bindings: i2c: vd55g1: Add vd55g0 compatible
  media: ipu-bridge: Add VD55G0 to the list of supported sensors
  platform/x86: int3472: Add VD55G0 supply GPIO mapping
  media: i2c: st-vd55g1: Default to illuminator on GPIO 1
  media: i2c: st,vd55g1: Handle virtual firmware graph endpoints
  media: i2c: st-vd55g1: Clean up module error reporting
  media: i2c: st-vd55g1: Unify frame timing calculations
  media: i2c: st-vd55g1: Abstract sensor models, revisions, and features
  media: i2c: st-vd55g1: Use indirect hardware register addressing
  media: i2c: st-vd55g1: Support VD55G0 global-shutter image sensor

 .../bindings/media/i2c/st,vd55g1.yaml         |   34 +-
 MAINTAINERS                                   |    1 +
 drivers/media/i2c/vd55g1.c                    | 1667 +++++++++++++----
 drivers/media/pci/intel/ipu-bridge.c          |    2 +
 drivers/platform/x86/intel/int3472/discrete.c |   18 +
 5 files changed, 1394 insertions(+), 328 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 01/11] dt-bindings: media: i2c: st,vd55g1: Move allOf: after required:
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
@ 2026-09-18 22:16 ` Peter Marshall
  2026-09-18 22:16 ` [PATCH 02/11] media: dt-bindings: i2c: vd55g1: Add vd55g0 compatible Peter Marshall
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:16 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall, Rob Herring, Krzysztof Kozlowski, Conor Dooley

Update this binding to follow the ordering convention of
example-schema.yaml which places allOf: after required:. No functional
change.

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml b/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
index dfd87beba065..ea0ae1ea12b9 100644
--- a/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
@@ -20,9 +20,6 @@ description: |-
  lane. The sensor provides 4 GPIOS that can be used for external LED signal
  (synchronized with sensor integration periods).
 
-allOf:
-  - $ref: /schemas/media/video-interface-devices.yaml#
-
 properties:
   compatible:
     enum:
@@ -101,6 +98,9 @@ required:
   - reset-gpios
   - port
 
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
 unevaluatedProperties: false
 
 examples:
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 02/11] media: dt-bindings: i2c: vd55g1: Add vd55g0 compatible
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
  2026-09-18 22:16 ` [PATCH 01/11] dt-bindings: media: i2c: st,vd55g1: Move allOf: after required: Peter Marshall
@ 2026-09-18 22:16 ` Peter Marshall
  2026-09-18 22:16 ` [PATCH 03/11] media: ipu-bridge: Add VD55G0 to the list of supported sensors Peter Marshall
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:16 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall, Rob Herring, Krzysztof Kozlowski, Conor Dooley

Add the vd55g0 to the compatible enum and update messages to reflect
its support.

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 .../bindings/media/i2c/st,vd55g1.yaml         | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml b/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
index ea0ae1ea12b9..195103ec0912 100644
--- a/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
@@ -5,16 +5,18 @@
 $id: http://devicetree.org/schemas/media/i2c/st,vd55g1.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: STMicroelectronics VD55G1 Global Shutter Image Sensor
+title: STMicroelectronics VD55G1 Family Global Shutter Image Sensors
 
 maintainers:
   - Benjamin Mugnier <benjamin.mugnier@foss.st.com>
   - Sylvain Petinot <sylvain.petinot@foss.st.com>
+  - Peter Marshall <pm@petermarshall.ca>
 
 description: |-
- The STMicroelectronics VD55G1 is a global shutter image sensor with an active
- array size of 804H x 704V. It is programmable through I2C interface. The I2C
- address is fixed to 0x10.
+ The STMicroelectronics VD55G1 family (VD55G0, VD55G1, and VD65G4) consists of
+ global shutter image sensors. The VD55G0 has an array size of 644H x 604V.
+ The VD55G1 and VD65G4 have an array size of 804H x 704V. They are programmable
+ through I2C interface. The I2C address is fixed to 0x10.
 
  Image data is sent through MIPI CSI-2, which is configured as only 1 data
  lane. The sensor provides 4 GPIOS that can be used for external LED signal
@@ -23,11 +25,12 @@ description: |-
 properties:
   compatible:
     enum:
+      - st,vd55g0
       - st,vd55g1
       - st,vd55g4
       - st,vd65g4
     description:
-      VD55G1 and VD55G4 are monochrome variants, while VD65G4 is a color one.
+      VD55G0 and VD55G1 are monochrome variants, while VD65G4 is a color variant.
 
   reg:
     maxItems: 1
@@ -101,6 +104,21 @@ required:
 allOf:
   - $ref: /schemas/media/video-interface-devices.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: st,vd55g0
+    then:
+      properties:
+        port:
+          properties:
+            endpoint:
+              properties:
+                link-frequencies:
+                  items:
+                    minimum: 375000000
+
 unevaluatedProperties: false
 
 examples:
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 03/11] media: ipu-bridge: Add VD55G0 to the list of supported sensors
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
  2026-09-18 22:16 ` [PATCH 01/11] dt-bindings: media: i2c: st,vd55g1: Move allOf: after required: Peter Marshall
  2026-09-18 22:16 ` [PATCH 02/11] media: dt-bindings: i2c: vd55g1: Add vd55g0 compatible Peter Marshall
@ 2026-09-18 22:16 ` Peter Marshall
  2026-09-18 22:16 ` [PATCH 04/11] platform/x86: int3472: Add VD55G0 supply GPIO mapping Peter Marshall
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:16 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall, Dan Scally, Hans de Goede

Add SMO55F0 ACPI HID for the ST VD55G0 sensor to the list of supported
sensors. The sensor is found with this HID on a Microsoft Surface Pro
9 tablet. Use the lowest supported link frequency.

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 drivers/media/pci/intel/ipu-bridge.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 77257311d1d8..a36f53965437 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -116,6 +116,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = {
 	IPU_SENSOR_CONFIG("OVTI5693", 1, 419200000),
 	/* Omnivision OV8856 */
 	IPU_SENSOR_CONFIG("OVTI8856", 3, 180000000, 360000000, 720000000),
+	/* STMicroelectronics VD55G0 */
+	IPU_SENSOR_CONFIG("SMO55F0", 1,  380000000),
 	/* Sony IMX471 */
 	IPU_SENSOR_CONFIG("SONY471A", 1, 200000000),
 	/* Sony IMX471 (found on Lenovo X1 Carbon G14) */
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 04/11] platform/x86: int3472: Add VD55G0 supply GPIO mapping
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
                   ` (2 preceding siblings ...)
  2026-09-18 22:16 ` [PATCH 03/11] media: ipu-bridge: Add VD55G0 to the list of supported sensors Peter Marshall
@ 2026-09-18 22:16 ` Peter Marshall
  2026-09-18 22:16 ` [PATCH 05/11] media: i2c: st-vd55g1: Default to illuminator on GPIO 1 Peter Marshall
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:16 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall, Daniel Scally, Hans de Goede, Ilpo Järvinen

The vd55g sensor drivers / DT bindings expect non-standard supply names.
Add a mapping to enable them.

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 drivers/platform/x86/intel/int3472/discrete.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index b48cf1b7fd5f..bf145ee0869b 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -198,6 +198,24 @@ static const struct int3472_gpio_map int3472_gpio_map[] = {
 		.con_id = "vana",
 		.enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
 	},
+	{
+		.hid = "SMO55F0",
+		.type_from = INT3472_GPIO_TYPE_HANDSHAKE,
+		.type_to = INT3472_GPIO_TYPE_HANDSHAKE,
+		.con_id = "vcore",
+	},
+	{
+		.hid = "SMO55F0",
+		.type_from = INT3472_GPIO_TYPE_DOVDD,
+		.type_to = INT3472_GPIO_TYPE_DOVDD,
+		.con_id = "vddio",
+	},
+	{
+		.hid = "SMO55F0",
+		.type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
+		.type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
+		.con_id = "vana",
+	},
 };
 
 static bool int3472_gpio_map_hids_match(struct acpi_device *adev,
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 05/11] media: i2c: st-vd55g1: Default to illuminator on GPIO 1
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
                   ` (3 preceding siblings ...)
  2026-09-18 22:16 ` [PATCH 04/11] platform/x86: int3472: Add VD55G0 supply GPIO mapping Peter Marshall
@ 2026-09-18 22:16 ` Peter Marshall
  2026-09-18 22:17 ` [PATCH 06/11] media: i2c: st,vd55g1: Handle virtual firmware graph endpoints Peter Marshall
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:16 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall

Sensors enumerated through a bus at runtime instead of a device tree may
not have any GPIO pins detected. Assume the illuminator is connected to
GPIO 1.

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 drivers/media/i2c/vd55g1.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 22b1497e8851..032b76a84638 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -1895,6 +1895,11 @@ static int vd55g1_parse_dt_gpios(struct vd55g1 *sensor)
 		set_bit(led_gpios[i], &sensor->ext_leds_mask);
 	}
 
+	if (nb_gpios_leds == 0) {
+		sensor->gpios[1] = VD55G1_GPIO_MODE_STROBE;
+		set_bit(1, &sensor->ext_leds_mask);
+	}
+
 	return 0;
 }
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 06/11] media: i2c: st,vd55g1: Handle virtual firmware graph endpoints
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
                   ` (4 preceding siblings ...)
  2026-09-18 22:16 ` [PATCH 05/11] media: i2c: st-vd55g1: Default to illuminator on GPIO 1 Peter Marshall
@ 2026-09-18 22:17 ` Peter Marshall
  2026-09-18 22:17 ` [PATCH 07/11] media: i2c: st-vd55g1: Clean up module error reporting Peter Marshall
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:17 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall

The firmware graph endpoint may be conveyed by a software node from
a module that has not been loaded yet, causing the module to bail out.

v4l2_fwnode_endpoint_alloc_parse() handles a NULL/missing endpoint by
returning -EPROBE_DEFER. Skip our own validation in the device tree
parsing logic and rely on its native error handling.

Clean up the fwnode allocation by handling it inside check_csi_conf().

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 drivers/media/i2c/vd55g1.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 032b76a84638..3e2261a95f2d 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -1792,16 +1792,19 @@ static int vd55g1_power_off(struct device *dev)
 	return 0;
 }
 
-static int vd55g1_check_csi_conf(struct vd55g1 *sensor,
-				 struct fwnode_handle *endpoint)
+static int vd55g1_check_csi_conf(struct vd55g1 *sensor)
 {
 	struct v4l2_fwnode_endpoint ep = { .bus_type = V4L2_MBUS_CSI2_DPHY };
+	struct fwnode_handle *fwnode;
 	u8 n_lanes;
 	int ret;
 
-	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep);
+	fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(sensor->dev),
+						 0, 0, 0);
+
+	ret = v4l2_fwnode_endpoint_alloc_parse(fwnode, &ep);
 	if (ret)
-		return -EINVAL;
+		goto fwnode;
 
 	/* Check lanes number */
 	n_lanes = ep.bus.mipi_csi2.num_data_lanes;
@@ -1839,6 +1842,9 @@ static int vd55g1_check_csi_conf(struct vd55g1 *sensor,
 done:
 	v4l2_fwnode_endpoint_free(&ep);
 
+fwnode:
+	fwnode_handle_put(fwnode);
+
 	return ret;
 }
 
@@ -1905,18 +1911,9 @@ static int vd55g1_parse_dt_gpios(struct vd55g1 *sensor)
 
 static int vd55g1_parse_dt(struct vd55g1 *sensor)
 {
-	struct fwnode_handle *endpoint;
 	int ret;
 
-	endpoint = fwnode_graph_get_endpoint_by_id(dev_fwnode(sensor->dev),
-						   0, 0, 0);
-	if (!endpoint) {
-		dev_err(sensor->dev, "Endpoint node not found\n");
-		return -EINVAL;
-	}
-
-	ret = vd55g1_check_csi_conf(sensor, endpoint);
-	fwnode_handle_put(endpoint);
+	ret = vd55g1_check_csi_conf(sensor);
 	if (ret)
 		return ret;
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 07/11] media: i2c: st-vd55g1: Clean up module error reporting
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
                   ` (5 preceding siblings ...)
  2026-09-18 22:17 ` [PATCH 06/11] media: i2c: st,vd55g1: Handle virtual firmware graph endpoints Peter Marshall
@ 2026-09-18 22:17 ` Peter Marshall
  2026-09-18 22:17 ` [PATCH 08/11] media: i2c: st-vd55g1: Unify frame timing calculations Peter Marshall
  2026-09-18 22:17 ` [PATCH 09/11] media: i2c: st-vd55g1: Abstract sensor models, revisions, and features Peter Marshall
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:17 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall

Downgrade helper error messages from dev_err to dev_dbg prints to keep
logs clean and actionable while preserving detailed information for
debugging.

Delegate module data management and error reporting to the callers of
power_on/power_off. Introduce dedicated PM handlers to dispatch them
instead of using them directly as module entry points.

Handle failure of a previously unchecked write in power_on.

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 drivers/media/i2c/vd55g1.c | 112 +++++++++++++++++++++++--------------
 1 file changed, 69 insertions(+), 43 deletions(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 3e2261a95f2d..f54f8ba71284 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -789,7 +789,7 @@ static int vd55g1_prepare_clock_tree(struct vd55g1 *sensor)
 
 	if (sensor->xclk_freq < VD55G1_XCLK_FREQ_MIN ||
 	    sensor->xclk_freq > VD55G1_XCLK_FREQ_MAX) {
-		dev_err(sensor->dev,
+		dev_dbg(sensor->dev,
 			"Only %luMhz-%luMhz clock range supported. Provided %lu MHz\n",
 			VD55G1_XCLK_FREQ_MIN / HZ_PER_MHZ,
 			VD55G1_XCLK_FREQ_MAX / HZ_PER_MHZ,
@@ -802,7 +802,7 @@ static int vd55g1_prepare_clock_tree(struct vd55g1 *sensor)
 
 	if (sensor->mipi_rate < VD55G1_MIPI_RATE_MIN ||
 	    sensor->mipi_rate > VD55G1_MIPI_RATE_MAX) {
-		dev_err(sensor->dev,
+		dev_dbg(sensor->dev,
 			"Only %luMbps-%luMbps data rate range supported. Provided %lu Mbps\n",
 			VD55G1_MIPI_RATE_MIN / MEGA,
 			VD55G1_MIPI_RATE_MAX / MEGA,
@@ -1220,14 +1220,14 @@ static int vd55g1_patch(struct vd55g1 *sensor)
 			     VD55G1_BOOT_PATCH_AND_BOOT, &ret);
 		vd55g1_poll_reg(sensor, VD55G1_REG_BOOT, 0, &ret);
 		if (ret) {
-			dev_err(sensor->dev, "Failed to apply patch\n");
+			dev_dbg(sensor->dev, "Failed to apply patch\n");
 			return ret;
 		}
 
 		vd55g1_read(sensor, VD55G1_REG_FWPATCH_REVISION, &patch, &ret);
 		if (patch != (VD55G1_FWPATCH_REVISION_MAJOR << 8) +
 		    VD55G1_FWPATCH_REVISION_MINOR) {
-			dev_err(sensor->dev, "Bad patch version expected %d.%d got %d.%d\n",
+			dev_dbg(sensor->dev, "Bad patch version expected %d.%d got %d.%d\n",
 				VD55G1_FWPATCH_REVISION_MAJOR,
 				VD55G1_FWPATCH_REVISION_MINOR,
 				(u8)(patch >> 8), (u8)(patch & 0xff));
@@ -1240,14 +1240,14 @@ static int vd55g1_patch(struct vd55g1 *sensor)
 		vd55g1_write(sensor, VD55G1_REG_BOOT, VD55G1_BOOT_BOOT, &ret);
 		vd55g1_poll_reg(sensor, VD55G1_REG_BOOT, 0, &ret);
 		if (ret) {
-			dev_err(sensor->dev, "Failed to boot\n");
+			dev_dbg(sensor->dev, "Failed to boot\n");
 			return ret;
 		}
 	}
 
 	ret = vd55g1_wait_state(sensor, VD55G1_SYSTEM_FSM_SW_STBY, NULL);
 	if (ret) {
-		dev_err(sensor->dev, "Sensor waiting after boot failed\n");
+		dev_dbg(sensor->dev, "Sensor waiting after boot failed\n");
 		return ret;
 	}
 
@@ -1705,18 +1705,21 @@ static int vd55g1_detect(struct vd55g1 *sensor)
 
 	vd55g1_read(sensor, VD55G1_REG_MODEL_ID, &id, &ret);
 	vd55g1_read(sensor, VD55G1_REG_COLOR_VERSION, &color, &ret);
-	if (ret)
+	if (ret) {
+		dev_dbg(sensor->dev,
+			"Failed to read sensor model: %d\n", ret);
 		return ret;
+	}
 
 	version = vd55g1_get_version(id, color);
 	if (!version) {
-		dev_warn(sensor->dev, "Unsupported sensor version, expected %s\n",
-			 dt_version->name);
+		dev_dbg(sensor->dev, "Unsupported sensor version, expected %s\n",
+			dt_version->name);
 		return -ENODEV;
 	}
 	if (version->id != dt_version->id ||
 	    version->color != dt_version->color) {
-		dev_err(sensor->dev, "Probed sensor version %s and device tree definition %s mismatch",
+		dev_dbg(sensor->dev, "Probed sensor version %s and device tree definition %s mismatch",
 			version->name, dt_version->name);
 		return -ENODEV;
 	}
@@ -1726,22 +1729,20 @@ static int vd55g1_detect(struct vd55g1 *sensor)
 	return 0;
 }
 
-static int vd55g1_power_on(struct device *dev)
+static int vd55g1_power_on(struct vd55g1 *sensor)
 {
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct vd55g1 *sensor = to_vd55g1(sd);
 	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(vd55g1_supply_name),
 				    sensor->supplies);
 	if (ret) {
-		dev_err(dev, "Failed to enable regulators %d\n", ret);
+		dev_dbg(sensor->dev, "Failed to enable regulators: %d\n", ret);
 		return ret;
 	}
 
 	ret = clk_prepare_enable(sensor->xclk);
 	if (ret) {
-		dev_err(dev, "Failed to enable clock %d\n", ret);
+		dev_dbg(sensor->dev, "Failed to enable clock: %d\n", ret);
 		goto disable_bulk;
 	}
 
@@ -1749,25 +1750,27 @@ static int vd55g1_power_on(struct device *dev)
 	usleep_range(5000, 10000);
 	ret = vd55g1_wait_state(sensor, VD55G1_SYSTEM_FSM_READY_TO_BOOT, NULL);
 	if (ret) {
-		dev_err(dev, "Sensor reset failed %d\n", ret);
+		dev_dbg(sensor->dev, "Sensor reset failed: %d\n", ret);
 		goto disable_clock;
 	}
 
 	ret = vd55g1_detect(sensor);
-	if (ret) {
-		dev_err(dev, "Sensor detect failed %d\n", ret);
+	if (ret)
 		goto disable_clock;
-	}
 
 	/* Setup clock now to advance through system FSM states */
 	vd55g1_write(sensor, VD55G1_REG_EXT_CLOCK, sensor->xclk_freq, &ret);
-
-	ret = vd55g1_patch(sensor);
 	if (ret) {
-		dev_err(dev, "Sensor patch failed %d\n", ret);
+		dev_dbg(sensor->dev,
+			"Failed to write external clock frequency: %d\n",
+			ret);
 		goto disable_clock;
 	}
 
+	ret = vd55g1_patch(sensor);
+	if (ret)
+		goto disable_clock;
+
 	return 0;
 
 disable_clock:
@@ -1780,11 +1783,8 @@ static int vd55g1_power_on(struct device *dev)
 	return ret;
 }
 
-static int vd55g1_power_off(struct device *dev)
+static int vd55g1_power_off(struct vd55g1 *sensor)
 {
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct vd55g1 *sensor = to_vd55g1(sd);
-
 	gpiod_set_value_cansleep(sensor->reset_gpio, 1);
 	clk_disable_unprepare(sensor->xclk);
 	regulator_bulk_disable(ARRAY_SIZE(sensor->supplies), sensor->supplies);
@@ -1792,6 +1792,32 @@ static int vd55g1_power_off(struct device *dev)
 	return 0;
 }
 
+static int vd55g1_pm_resume(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct vd55g1 *sensor = to_vd55g1(sd);
+	int ret;
+
+	ret = vd55g1_power_on(sensor);
+	if (ret)
+		dev_err(dev, "Failed to power on during PM resume: %d\n", ret);
+
+	return ret;
+}
+
+static int vd55g1_pm_suspend(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct vd55g1 *sensor = to_vd55g1(sd);
+	int ret;
+
+	ret = vd55g1_power_off(sensor);
+	if (ret)
+		dev_err(dev, "Failed to power off during PM suspend: %d\n", ret);
+
+	return ret;
+}
+
 static int vd55g1_check_csi_conf(struct vd55g1 *sensor)
 {
 	struct v4l2_fwnode_endpoint ep = { .bus_type = V4L2_MBUS_CSI2_DPHY };
@@ -1809,7 +1835,7 @@ static int vd55g1_check_csi_conf(struct vd55g1 *sensor)
 	/* Check lanes number */
 	n_lanes = ep.bus.mipi_csi2.num_data_lanes;
 	if (n_lanes != 1) {
-		dev_err(sensor->dev, "Sensor only supports 1 lane, found %d\n",
+		dev_dbg(sensor->dev, "Sensor only supports 1 lane, found %d\n",
 			n_lanes);
 		ret = -EINVAL;
 		goto done;
@@ -1817,7 +1843,7 @@ static int vd55g1_check_csi_conf(struct vd55g1 *sensor)
 
 	/* Clock lane must be first */
 	if (ep.bus.mipi_csi2.clock_lane != 0) {
-		dev_err(sensor->dev, "Clock lane must be mapped to lane 0\n");
+		dev_dbg(sensor->dev, "Clock lane must be mapped to lane 0\n");
 		ret = -EINVAL;
 		goto done;
 	}
@@ -1828,12 +1854,12 @@ static int vd55g1_check_csi_conf(struct vd55g1 *sensor)
 
 	/* Check the link frequency set in device tree */
 	if (!ep.nr_of_link_frequencies) {
-		dev_err(sensor->dev, "link-frequency property not found in DT\n");
+		dev_dbg(sensor->dev, "link-frequency property not found in DT\n");
 		ret = -EINVAL;
 		goto done;
 	}
 	if (ep.nr_of_link_frequencies != 1) {
-		dev_err(sensor->dev, "Multiple link frequencies not supported\n");
+		dev_dbg(sensor->dev, "Multiple link frequencies not supported\n");
 		ret = -EINVAL;
 		goto done;
 	}
@@ -1864,12 +1890,12 @@ static int vd55g1_parse_dt_gpios_array(struct vd55g1 *sensor,
 	ret = device_property_read_u32_array(sensor->dev,
 					     prop_name, array, *nb);
 	if (ret) {
-		dev_err(sensor->dev, "Failed to read %s prop\n", prop_name);
+		dev_dbg(sensor->dev, "Failed to read %s prop\n", prop_name);
 		return ret;
 	}
 	for (i = 0; i < *nb;  i++) {
 		if (array[i] >= VD55G1_NB_GPIOS) {
-			dev_err(sensor->dev, "Invalid GPIO number %d\n",
+			dev_dbg(sensor->dev, "Invalid GPIO number %d\n",
 				array[i]);
 			return -EINVAL;
 		}
@@ -1933,14 +1959,14 @@ static int vd55g1_subdev_init(struct vd55g1 *sensor)
 	sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 	ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad);
 	if (ret) {
-		dev_err(sensor->dev, "Failed to init media entity: %d\n", ret);
+		dev_dbg(sensor->dev, "Failed to init media entity: %d\n", ret);
 		return ret;
 	}
 
 	sensor->sd.state_lock = sensor->ctrl_handler.lock;
 	ret = v4l2_subdev_init_finalize(&sensor->sd);
 	if (ret) {
-		dev_err(sensor->dev, "Subdev init error: %d\n", ret);
+		dev_dbg(sensor->dev, "Subdev init error: %d\n", ret);
 		goto err_ctrls;
 	}
 
@@ -1950,7 +1976,7 @@ static int vd55g1_subdev_init(struct vd55g1 *sensor)
 	 */
 	ret = vd55g1_init_ctrls(sensor);
 	if (ret) {
-		dev_err(sensor->dev, "Controls initialization failed %d\n",
+		dev_dbg(sensor->dev, "Controls initialization failed %d\n",
 			ret);
 		goto err_media;
 	}
@@ -2015,7 +2041,7 @@ static int vd55g1_probe(struct i2c_client *client)
 	sensor->xclk_freq = clk_get_rate(sensor->xclk);
 	ret = vd55g1_prepare_clock_tree(sensor);
 	if (ret)
-		return ret;
+		return dev_err_probe(dev, ret, "Unsupported clock configuration\n");
 
 	sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
 						     GPIOD_OUT_HIGH);
@@ -2029,9 +2055,9 @@ static int vd55g1_probe(struct i2c_client *client)
 				     "Failed to init regmap\n");
 
 	/* Detect if sensor is present and if its revision is supported */
-	ret = vd55g1_power_on(dev);
+	ret = vd55g1_power_on(sensor);
 	if (ret)
-		return ret;
+		return dev_err_probe(dev, ret, "Failed to power on during probe\n");
 
 	/* Enable pm_runtime and power off the sensor */
 	pm_runtime_set_active(dev);
@@ -2043,13 +2069,13 @@ static int vd55g1_probe(struct i2c_client *client)
 
 	ret = vd55g1_subdev_init(sensor);
 	if (ret) {
-		dev_err(dev, "V4l2 init failed: %d\n", ret);
+		dev_err_probe(dev, ret, "V4l2 subdev init failed\n");
 		goto err_power_off;
 	}
 
 	ret = v4l2_async_register_subdev(&sensor->sd);
 	if (ret) {
-		dev_err(dev, "async subdev register failed %d\n", ret);
+		dev_err_probe(dev, ret, "async subdev register failed\n");
 		goto err_subdev;
 	}
 
@@ -2061,7 +2087,7 @@ static int vd55g1_probe(struct i2c_client *client)
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);
 	pm_runtime_dont_use_autosuspend(dev);
-	vd55g1_power_off(dev);
+	vd55g1_power_off(sensor);
 
 	return ret;
 }
@@ -2075,7 +2101,7 @@ static void vd55g1_remove(struct i2c_client *client)
 
 	pm_runtime_disable(&client->dev);
 	if (!pm_runtime_status_suspended(&client->dev))
-		vd55g1_power_off(&client->dev);
+		vd55g1_power_off(sensor);
 	pm_runtime_set_suspended(&client->dev);
 	pm_runtime_dont_use_autosuspend(&client->dev);
 }
@@ -2089,7 +2115,7 @@ static const struct of_device_id vd55g1_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, vd55g1_dt_ids);
 
 static const struct dev_pm_ops vd55g1_pm_ops = {
-	SET_RUNTIME_PM_OPS(vd55g1_power_off, vd55g1_power_on, NULL)
+	SET_RUNTIME_PM_OPS(vd55g1_pm_suspend, vd55g1_pm_resume, NULL)
 };
 
 static struct i2c_driver vd55g1_i2c_driver = {
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 08/11] media: i2c: st-vd55g1: Unify frame timing calculations
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
                   ` (6 preceding siblings ...)
  2026-09-18 22:17 ` [PATCH 07/11] media: i2c: st-vd55g1: Clean up module error reporting Peter Marshall
@ 2026-09-18 22:17 ` Peter Marshall
  2026-09-18 22:17 ` [PATCH 09/11] media: i2c: st-vd55g1: Abstract sensor models, revisions, and features Peter Marshall
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:17 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall

The calculation of frame length and maximum exposure lines is currently
duplicated both when changing to a new format and when adjusting vblank
time. Introduce a reusable function writing calculated frame time and
exposure limit to an info struct.

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 drivers/media/i2c/vd55g1.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index f54f8ba71284..1776f5ec6a7d 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -559,6 +559,11 @@ struct vd55g1_vblank_limits {
 	u16 max;
 };
 
+struct vd55g1_frame_timings {
+	u16 frame_length;
+	u16 expo_max;
+};
+
 struct vd55g1 {
 	struct device *dev;
 	const struct vd55g1_version *version;
@@ -722,6 +727,15 @@ static void vd55g1_get_vblank_limits(struct vd55g1 *sensor,
 	limits->max = VD55G1_VBLANK_MAX - crop->height;
 }
 
+static void vd55g1_get_frame_timings(struct vd55g1 *sensor,
+				     struct v4l2_rect *crop,
+				     struct vd55g1_frame_timings *timings)
+{
+	timings->frame_length = crop->height + sensor->vblank_ctrl->val;
+
+	timings->expo_max = timings->frame_length - VD55G1_EXPO_MAX_TERM;
+}
+
 #define vd55g1_read(sensor, reg, val, err) \
 	cci_read((sensor)->regmap, reg, val, err)
 
@@ -1304,9 +1318,8 @@ static int vd55g1_new_format_change_controls(struct vd55g1 *sensor,
 					     struct v4l2_rect *crop)
 {
 	struct vd55g1_vblank_limits vblank;
+	struct vd55g1_frame_timings timings;
 	unsigned int hblank;
-	unsigned int frame_length = 0;
-	unsigned int expo_max;
 	int ret;
 
 	/* Reset vblank and frame length to default */
@@ -1317,10 +1330,9 @@ static int vd55g1_new_format_change_controls(struct vd55g1 *sensor,
 		return ret;
 
 	/* Max exposure changes with vblank */
-	frame_length = crop->height + sensor->vblank_ctrl->val;
-	expo_max = frame_length - VD55G1_EXPO_MAX_TERM;
-	ret = __v4l2_ctrl_modify_range(sensor->expo_ctrl, 0, expo_max, 1,
-				       VD55G1_EXPO_DEF);
+	vd55g1_get_frame_timings(sensor, crop, &timings);
+	ret = __v4l2_ctrl_modify_range(sensor->expo_ctrl, 0, timings.expo_max,
+				       1, VD55G1_EXPO_DEF);
 	if (ret)
 		return ret;
 
@@ -1478,8 +1490,6 @@ static int vd55g1_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 static int vd55g1_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct vd55g1 *sensor = ctrl_to_vd55g1(ctrl);
-	unsigned int frame_length = 0;
-	unsigned int expo_max;
 	struct v4l2_subdev_state *state =
 		v4l2_subdev_get_locked_active_state(&sensor->sd);
 	struct v4l2_rect *crop =
@@ -1487,6 +1497,7 @@ static int vd55g1_s_ctrl(struct v4l2_ctrl *ctrl)
 	struct v4l2_mbus_framefmt *format =
 		v4l2_subdev_state_get_format(state, 0);
 	unsigned int hblank = vd55g1_get_hblank_min(sensor, format, crop);
+	struct vd55g1_frame_timings timings;
 	bool is_auto = false;
 	int ret = 0;
 
@@ -1496,10 +1507,10 @@ static int vd55g1_s_ctrl(struct v4l2_ctrl *ctrl)
 	/* Update controls state, range, etc. whatever the state of the HW */
 	switch (ctrl->id) {
 	case V4L2_CID_VBLANK:
-		frame_length = crop->height + ctrl->val;
-		expo_max = frame_length - VD55G1_EXPO_MAX_TERM;
-		ret = __v4l2_ctrl_modify_range(sensor->expo_ctrl, 0, expo_max,
-					       1, VD55G1_EXPO_DEF);
+		vd55g1_get_frame_timings(sensor, crop, &timings);
+		ret = __v4l2_ctrl_modify_range(sensor->expo_ctrl, 0,
+					       timings.expo_max, 1,
+					       VD55G1_EXPO_DEF);
 		break;
 	case V4L2_CID_EXPOSURE_AUTO:
 		is_auto = (ctrl->val == V4L2_EXPOSURE_AUTO);
@@ -1551,7 +1562,7 @@ static int vd55g1_s_ctrl(struct v4l2_ctrl *ctrl)
 		ret = vd55g1_update_exposure_target(sensor, ctrl->val);
 		break;
 	case V4L2_CID_VBLANK:
-		ret = vd55g1_update_frame_length(sensor, frame_length);
+		ret = vd55g1_update_frame_length(sensor, timings.frame_length);
 		break;
 	case V4L2_CID_FLASH_LED_MODE:
 		ret = vd55g1_update_gpios(sensor, sensor->ext_leds_mask);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 09/11] media: i2c: st-vd55g1: Abstract sensor models, revisions, and features
  2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
                   ` (7 preceding siblings ...)
  2026-09-18 22:17 ` [PATCH 08/11] media: i2c: st-vd55g1: Unify frame timing calculations Peter Marshall
@ 2026-09-18 22:17 ` Peter Marshall
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Marshall @ 2026-09-18 22:17 UTC (permalink / raw)
  To: Sakari Ailus, Mauro Carvalho Chehab, Benjamin Mugnier, Sylvain Petinot
  Cc: linux-kernel, devicetree, linux-media, platform-driver-x86,
	Peter Marshall

Store a reference to the requested model struct in the client data
early during probe. This lets init steps know what sensor model we're
expecting even before it gets verified in detect().

Reintroduce the chip revision check that was lost during a restructure
in commit 7a1e5239ae63 ("media: i2c: vd55g1: Add support for vd55g4")
to prevent a firmware patch mismatch. Pass a list of supported revisions
and their required patch in the model struct. Skip check for vd55g4
where the revisions are unknown.

Break up the checks in detect() to report exactly which identifier
field had an unexpected value. Bail out on the first mismatch instead of
trying to detect what other supported chip it may represent.

Replace feature gating based on the color support code with a boolean
flag.

Signed-off-by: Peter Marshall <pm@petermarshall.ca>
---
 drivers/media/i2c/vd55g1.c | 183 ++++++++++++++++++++++++-------------
 1 file changed, 120 insertions(+), 63 deletions(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 1776f5ec6a7d..24bc4a4dc0ac 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -29,6 +29,7 @@
 
 /* Register Map */
 #define VD55G1_REG_MODEL_ID				CCI_REG32_LE(0x0000)
+#define VD55G1_REG_REVISION				CCI_REG16_LE(0x0004)
 #define VD55G1_REG_COLOR_VERSION			CCI_REG32_LE(0x0670)
 #define VD55G1_REG_FWPATCH_REVISION			CCI_REG16_LE(0x0012)
 #define VD55G1_REG_FWPATCH_START_ADDR			CCI_REG8(0x2000)
@@ -126,8 +127,6 @@
 #define VD55G1_FRAME_LENGTH_DEF				1860 /* 60 fps */
 #define VD55G1_MIPI_MARGIN				900
 #define VD55G1_CTX_OFFSET				0x50
-#define VD55G1_FWPATCH_REVISION_MAJOR			2
-#define VD55G1_FWPATCH_REVISION_MINOR			9
 #define VD55G1_XCLK_FREQ_MIN				(6 * HZ_PER_MHZ)
 #define VD55G1_XCLK_FREQ_MAX				(27 * HZ_PER_MHZ)
 #define VD55G1_MIPI_RATE_MIN				(250 * MEGA)
@@ -143,30 +142,6 @@ enum vd55g1_color_version {
 	VD55G1_COLOR_VERSION_BAYER,
 };
 
-struct vd55g1_version {
-	char *name;
-	enum vd55g1_model_id id;
-	enum vd55g1_color_version color;
-};
-
-static const struct vd55g1_version vd55g1_versions[] = {
-	{
-		.name  = "vd55g1",
-		.id    = VD55G1_MODEL_ID_2,
-		.color = VD55G1_COLOR_VERSION_MONO,
-	},
-	{
-		.name  = "vd55g4",
-		.id    = VD55G1_MODEL_ID_3,
-		.color = VD55G1_COLOR_VERSION_MONO,
-	},
-	{
-		.name  = "vd65g4",
-		.id    = VD55G1_MODEL_ID_3,
-		.color = VD55G1_COLOR_VERSION_BAYER,
-	},
-};
-
 static const u8 vd55g1_patch_array[] = {
 	0x44, 0x03, 0x09, 0x02, 0xe6, 0x01, 0x42, 0x00, 0xea, 0x01, 0x42, 0x00,
 	0xf0, 0x01, 0x42, 0x00, 0xe6, 0x01, 0x42, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -564,15 +539,90 @@ struct vd55g1_frame_timings {
 	u16 expo_max;
 };
 
+#define VD55G1_REVISION_ANY 0
+
+struct vd55g1_revision {
+	u64 id;
+	bool needs_patch;
+	const struct vd55g1_firmware *builtin_fw;
+};
+
+struct vd55g1_model {
+	const char *name;
+	unsigned int id;
+	enum vd55g1_color_version color;
+	const struct vd55g1_revision *revisions;
+	u16 num_revisions;
+	bool bayer;
+};
+
+struct vd55g1_firmware {
+	size_t size;
+	const u8 *data;
+	u8 v_major;
+	u8 v_minor;
+};
+
+static const struct vd55g1_firmware vd55g1_builtin_fw = {
+	.data = vd55g1_patch_array,
+	.size = ARRAY_SIZE(vd55g1_patch_array),
+	.v_major = 2,
+	.v_minor = 9,
+};
+
+static const struct vd55g1_revision vd55g1_revisions[] = {
+	{
+		.id = 0x2020,
+		.needs_patch = true,
+		.builtin_fw = &vd55g1_builtin_fw
+	},
+};
+
+static const struct vd55g1_revision vd55g4_revisions[] = {
+	{ .id = VD55G1_REVISION_ANY, .needs_patch = false },
+};
+
+static const struct vd55g1_revision vd65g4_revisions[] = {
+	{ .id = 0x3030, .needs_patch = false },
+};
+
+static const struct vd55g1_model vd55g1_model = {
+	.name = "vd55g1",
+	.id = VD55G1_MODEL_ID_2,
+	.color = VD55G1_COLOR_VERSION_MONO,
+	.revisions = vd55g1_revisions,
+	.num_revisions = ARRAY_SIZE(vd55g1_revisions),
+	.bayer = false,
+};
+
+static const struct vd55g1_model vd55g4_model = {
+	.name  = "vd55g4",
+	.id = VD55G1_MODEL_ID_3,
+	.color = VD55G1_COLOR_VERSION_MONO,
+	.revisions = vd55g4_revisions,
+	.num_revisions = ARRAY_SIZE(vd55g4_revisions),
+	.bayer = false,
+};
+
+static const struct vd55g1_model vd65g4_model = {
+	.name = "vd65g4",
+	.id = VD55G1_MODEL_ID_3,
+	.color = VD55G1_COLOR_VERSION_BAYER,
+	.revisions = vd65g4_revisions,
+	.num_revisions = ARRAY_SIZE(vd65g4_revisions),
+	.bayer = true,
+};
+
 struct vd55g1 {
 	struct device *dev;
-	const struct vd55g1_version *version;
 	struct v4l2_subdev sd;
 	struct media_pad pad;
 	struct regulator_bulk_data supplies[ARRAY_SIZE(vd55g1_supply_name)];
 	struct gpio_desc *reset_gpio;
 	struct clk *xclk;
 	struct regmap *regmap;
+	const struct vd55g1_model *model;
+	const struct vd55g1_revision *revision;
 	u32 xclk_freq;
 	u16 oif_ctrl;
 	u8 gpios[VD55G1_NB_GPIOS];
@@ -660,7 +710,7 @@ static u32 vd55g1_get_fmt_code(struct vd55g1 *sensor, u32 code)
 	u32 fallback_code;
 	unsigned int i, j;
 
-	if (sensor->version->color == VD55G1_COLOR_VERSION_MONO) {
+	if (!sensor->model->bayer) {
 		fallback_code = vd55g1_mbus_formats_mono[0];
 		for (i = 0; i < ARRAY_SIZE(vd55g1_mbus_formats_mono); i++)
 			if (vd55g1_mbus_formats_mono[i] == code)
@@ -1222,28 +1272,25 @@ static int vd55g1_disable_streams(struct v4l2_subdev *sd,
 
 static int vd55g1_patch(struct vd55g1 *sensor)
 {
+	const struct vd55g1_firmware *fw = sensor->revision->builtin_fw;
 	u64 patch;
 	int ret = 0;
 
-	/* Version 2 needs a patch while version 3 does not */
-	if (sensor->version->id == VD55G1_MODEL_ID_2) {
+	if (sensor->revision->needs_patch) {
 		vd55g1_write_array(sensor, VD55G1_REG_FWPATCH_START_ADDR,
-				   sizeof(vd55g1_patch_array),
-				   vd55g1_patch_array, &ret);
+				   fw->size, fw->data, &ret);
 		vd55g1_write(sensor, VD55G1_REG_BOOT,
 			     VD55G1_BOOT_PATCH_AND_BOOT, &ret);
 		vd55g1_poll_reg(sensor, VD55G1_REG_BOOT, 0, &ret);
+		vd55g1_read(sensor, VD55G1_REG_FWPATCH_REVISION, &patch, &ret);
 		if (ret) {
 			dev_dbg(sensor->dev, "Failed to apply patch\n");
 			return ret;
 		}
 
-		vd55g1_read(sensor, VD55G1_REG_FWPATCH_REVISION, &patch, &ret);
-		if (patch != (VD55G1_FWPATCH_REVISION_MAJOR << 8) +
-		    VD55G1_FWPATCH_REVISION_MINOR) {
+		if (patch != (fw->v_major << 8) + fw->v_minor) {
 			dev_dbg(sensor->dev, "Bad patch version expected %d.%d got %d.%d\n",
-				VD55G1_FWPATCH_REVISION_MAJOR,
-				VD55G1_FWPATCH_REVISION_MINOR,
+				fw->v_major, fw->v_minor,
 				(u8)(patch >> 8), (u8)(patch & 0xff));
 			return -ENODEV;
 		}
@@ -1299,7 +1346,7 @@ static int vd55g1_enum_mbus_code(struct v4l2_subdev *sd,
 	struct vd55g1 *sensor = to_vd55g1(sd);
 	u32 base_code;
 
-	if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER) {
+	if (!sensor->model->bayer) {
 		if (code->index >= ARRAY_SIZE(vd55g1_mbus_formats_mono))
 			return -EINVAL;
 		base_code = vd55g1_mbus_formats_mono[code->index];
@@ -1408,7 +1455,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
 	if (ret)
 		return ret;
 
-	if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
+	if (!sensor->model->bayer)
 		code = vd55g1_mbus_formats_mono[0];
 	else
 		code = vd55g1_mbus_formats_bayer[0][0];
@@ -1693,28 +1740,30 @@ static int vd55g1_init_ctrls(struct vd55g1 *sensor)
 	return ret;
 }
 
-static const struct vd55g1_version *
-vd55g1_get_version(enum vd55g1_model_id id,
-		   enum vd55g1_color_version color)
+static int vd55g1_match_revision(struct vd55g1 *sensor, u64 rev)
 {
-	for (unsigned int i = 0; i < ARRAY_SIZE(vd55g1_versions); i++) {
-		if (vd55g1_versions[i].id == id &&
-		    vd55g1_versions[i].color == color)
-			return &vd55g1_versions[i];
+	const struct vd55g1_revision *revision;
+
+	for (int i = 0; i < sensor->model->num_revisions; i++) {
+		revision = &sensor->model->revisions[i];
+
+		if (revision->id == rev ||
+		    revision->id == VD55G1_REVISION_ANY) {
+			sensor->revision = revision;
+			return 0;
+		}
 	}
 
-	return NULL;
+	return -ENOENT;
 }
 
 static int vd55g1_detect(struct vd55g1 *sensor)
 {
-	const struct vd55g1_version *dt_version =
-		device_get_match_data(sensor->dev);
-	const struct vd55g1_version *version;
-	u64 color, id;
+	u64 color, id, rev;
 	int ret = 0;
 
 	vd55g1_read(sensor, VD55G1_REG_MODEL_ID, &id, &ret);
+	vd55g1_read(sensor, VD55G1_REG_REVISION, &rev, &ret);
 	vd55g1_read(sensor, VD55G1_REG_COLOR_VERSION, &color, &ret);
 	if (ret) {
 		dev_dbg(sensor->dev,
@@ -1722,20 +1771,26 @@ static int vd55g1_detect(struct vd55g1 *sensor)
 		return ret;
 	}
 
-	version = vd55g1_get_version(id, color);
-	if (!version) {
-		dev_dbg(sensor->dev, "Unsupported sensor version, expected %s\n",
-			dt_version->name);
+	if (id != sensor->model->id) {
+		dev_dbg(sensor->dev,
+			"Expected %s (0x%x), but detected mismatched sensor id 0x%x\n",
+			sensor->model->name, (u32)sensor->model->id, (u32)id);
 		return -ENODEV;
 	}
-	if (version->id != dt_version->id ||
-	    version->color != dt_version->color) {
-		dev_dbg(sensor->dev, "Probed sensor version %s and device tree definition %s mismatch",
-			version->name, dt_version->name);
+	if (color != sensor->model->color) {
+		dev_dbg(sensor->dev,
+			"Expected %s (0x%x), but detected mismatched color version 0x%x\n",
+			sensor->model->name, (u32)sensor->model->color, (u32)color);
 		return -ENODEV;
 	}
 
-	sensor->version = version;
+	ret = vd55g1_match_revision(sensor, rev);
+	if (ret == -ENOENT) {
+		dev_dbg(sensor->dev,
+			"Unsupported sensor revision 0x%x, expected %s\n",
+			(u16)rev, sensor->model->name);
+		return -ENODEV;
+	}
 
 	return 0;
 }
@@ -2033,6 +2088,8 @@ static int vd55g1_probe(struct i2c_client *client)
 		return -ENOMEM;
 	sensor->dev = &client->dev;
 
+	sensor->model = device_get_match_data(dev);
+
 	v4l2_i2c_subdev_init(&sensor->sd, client, &vd55g1_subdev_ops);
 
 	ret = vd55g1_parse_dt(sensor);
@@ -2118,9 +2175,9 @@ static void vd55g1_remove(struct i2c_client *client)
 }
 
 static const struct of_device_id vd55g1_dt_ids[] = {
-	{ .compatible = "st,vd55g1", .data = (void *)&vd55g1_versions[0] },
-	{ .compatible = "st,vd55g4", .data = (void *)&vd55g1_versions[1] },
-	{ .compatible = "st,vd65g4", .data = (void *)&vd55g1_versions[2] },
+	{ .compatible = "st,vd55g1", .data = &vd55g1_model },
+	{ .compatible = "st,vd55g4", .data = &vd55g4_model },
+	{ .compatible = "st,vd65g4", .data = &vd65g4_model },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, vd55g1_dt_ids);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-09-18 22:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-18 22:16 [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support Peter Marshall
2026-09-18 22:16 ` [PATCH 01/11] dt-bindings: media: i2c: st,vd55g1: Move allOf: after required: Peter Marshall
2026-09-18 22:16 ` [PATCH 02/11] media: dt-bindings: i2c: vd55g1: Add vd55g0 compatible Peter Marshall
2026-09-18 22:16 ` [PATCH 03/11] media: ipu-bridge: Add VD55G0 to the list of supported sensors Peter Marshall
2026-09-18 22:16 ` [PATCH 04/11] platform/x86: int3472: Add VD55G0 supply GPIO mapping Peter Marshall
2026-09-18 22:16 ` [PATCH 05/11] media: i2c: st-vd55g1: Default to illuminator on GPIO 1 Peter Marshall
2026-09-18 22:17 ` [PATCH 06/11] media: i2c: st,vd55g1: Handle virtual firmware graph endpoints Peter Marshall
2026-09-18 22:17 ` [PATCH 07/11] media: i2c: st-vd55g1: Clean up module error reporting Peter Marshall
2026-09-18 22:17 ` [PATCH 08/11] media: i2c: st-vd55g1: Unify frame timing calculations Peter Marshall
2026-09-18 22:17 ` [PATCH 09/11] media: i2c: st-vd55g1: Abstract sensor models, revisions, and features Peter Marshall

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®