[PATCH v5 0/2] media: Add Himax HM1092 mono NIR sensor driver

[PATCH v5 0/2] media: Add Himax HM1092 mono NIR sensor driver

[Ramshouriesh R]

This adds a V4L2 subdev driver and DT binding for the Himax HM1092, a
1 megapixel monochrome near-infrared image sensor. On laptops it sits
behind the IR camera used for face unlock. It speaks a single MIPI CSI-2
data lane and outputs 10-bit RAW at 560x360.

The driver exposes that one native mode, a test pattern control and the
standard fwnode properties (orientation, rotation). It has been tested on
real hardware (an ASUS Zenbook A14): the sensor probes, streams, and the
on-chip test patterns come through the full CSI-2 pipeline.

The available HM1092 documentation does not describe the test pattern
modes. Their menu names are based on output observed during hardware
testing; the register programming was recovered from the Windows driver.

The pixel rate, horizontal and vertical blanking and the orientation and
rotation controls that libcamera requires have been present since v1; v5
adds the get_selection support requested for libcamera. The one gap is that
this driver does not implement the V4L2_CID_ANALOGUE_GAIN and
V4L2_CID_EXPOSURE controls that libcamera lists as mandatory for RAW
sensors: the same lack of documentation means the gain and exposure register
layout is not known, and the sensor is driven at the fixed exposure baked
into the init sequence. Gain/exposure can follow if the register map is
discovered.

The sensor driver and its binding are SoC-neutral, so they are sent on
their own through the media tree. The board-level device tree and PHY
work that wires this camera up on the ASUS Zenbook A14 will be sent as
its own series.

Signed-off-by: Ramshouriesh R <rshouriesh@gmail.com>
---
Changes in v5:
- dt-bindings: drop the data-lanes property entirely; the fixed
  single-lane wiring is not configurable, so it does not belong in DT.
- hm1092: implement the get_selection pad op (CROP, CROP_DEFAULT,
  CROP_BOUNDS and NATIVE_SIZE) reporting the sensor's pixel-array
  geometry, as required by libcamera.
- Link to v4: https://patch.msgid.link/20260705-hm1092-driver-v4-0-0a13ec274d89@gmail.com

Changes in v4:
- dt-bindings: make data-lanes optional for the fixed single-lane sensor,
  constrain explicit values to <1>, and omit it from the example.
- hm1092: add the copyright notice.
- hm1092: replace the private register representation and write helper
  with CCI register sequences and direct CCI writes.
- hm1092: use descriptive test-pattern names based on hardware captures;
  the available documentation does not identify these modes.
- hm1092: parse fwnode properties before creating controls and set
  read-only flags after validating control creation.
- hm1092: use the generic get_fmt callback for the fixed sensor mode.
- hm1092: initialize endpoint parsing with the fixed one-lane default,
  reject other lane configurations, and remove the redundant endpoint
  presence check.
- hm1092: use fsleep() and null-safe GPIO calls, and apply the requested
  declaration, brace, return-value and error-path formatting cleanups.
- Link to v3: https://patch.msgid.link/20260702-hm1092-driver-v3-0-85faa7ff4fec@gmail.com

Changes in v3:
- dt-bindings: add the Reviewed-by tag from Conor Dooley.
- hm1092: initialize RAW colorimetry fields in the pad format helper so
  userspace values cannot leak into subdevice state.
- hm1092: return -ENXIO when the required firmware graph endpoint is
  absent instead of deferring probe indefinitely.
- Link to v2: https://patch.msgid.link/20260702-hm1092-driver-v2-0-4f9f369d6a48@gmail.com

Changes in v2:
- hm1092: use pm_ptr() instead of pm_sleep_ptr() for the dev_pm_ops
  pointer. The ops come from DEFINE_RUNTIME_DEV_PM_OPS(), so gating them
  on CONFIG_PM_SLEEP dropped runtime PM on a CONFIG_PM=y, PM_SLEEP=n
  build.
- hm1092: free the control handler on the error paths in
  hm1092_init_controls(); the fwnode-parse and ctrl_hdlr->error returns
  leaked the handler.
- Link to v1: https://patch.msgid.link/20260701-hm1092-driver-v1-0-d1bd81e233b5@gmail.com

To: Ramshouriesh R <rshouriesh@gmail.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Ramshouriesh R (2):
      media: dt-bindings: Add Himax HM1092 NIR sensor
      media: i2c: hm1092: add Himax HM1092 mono NIR sensor driver

 .../bindings/media/i2c/himax,hm1092.yaml           | 101 +++
 MAINTAINERS                                        |   7 +
 drivers/media/i2c/Kconfig                          |  11 +
 drivers/media/i2c/Makefile                         |   1 +
 drivers/media/i2c/hm1092.c                         | 767 +++++++++++++++++++++
 5 files changed, 887 insertions(+)
---
base-commit: be5c93fa674f0fc3c8f359c2143abce6bbb422e6
change-id: 20260618-hm1092-driver-a6f2aaddf201

Best regards,
--  
Ramshouriesh R <rshouriesh@gmail.com>

[PATCH v5 1/2] media: dt-bindings: Add Himax HM1092 NIR sensor

[Ramshouriesh R]

Add a dt-binding schema for the Himax HM1092, a 1 megapixel monochrome
near-infrared CMOS image sensor used as the face-authentication IR
camera on laptops. The sensor streams 10-bit RAW over a single MIPI
CSI-2 data lane.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Ramshouriesh R <rshouriesh@gmail.com>
---
 .../bindings/media/i2c/himax,hm1092.yaml           | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/himax,hm1092.yaml b/Documentation/devicetree/bindings/media/i2c/himax,hm1092.yaml
new file mode 100644
index 000000000000..87c43ef883c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/himax,hm1092.yaml
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/himax,hm1092.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Himax HM1092 Monochrome NIR Sensor
+
+maintainers:
+  - Ramshouriesh R <rshouriesh@gmail.com>
+
+description:
+  The Himax HM1092 is a 1 megapixel monochrome near-infrared CMOS image
+  sensor with a MIPI CSI-2 interface, commonly used as the IR camera for
+  face authentication on laptops. It outputs 10-bit RAW over a single
+  MIPI CSI-2 data lane.
+
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+  compatible:
+    const: himax,hm1092
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  avdd-supply:
+    description: Analogue circuit voltage supply.
+
+  dovdd-supply:
+    description: I/O circuit voltage supply.
+
+  dvdd-supply:
+    description: Digital circuit voltage supply.
+
+  reset-gpios:
+    maxItems: 1
+    description: Active low GPIO connected to the XSHUTDOWN pad.
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        required:
+          - link-frequencies
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - avdd-supply
+  - dovdd-supply
+  - dvdd-supply
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        camera@24 {
+            compatible = "himax,hm1092";
+            reg = <0x24>;
+
+            reset-gpios = <&tlmm 109 GPIO_ACTIVE_LOW>;
+            pinctrl-names = "default";
+            pinctrl-0 = <&cam_ir_default>;
+
+            clocks = <&camcc 1>;
+            assigned-clocks = <&camcc 1>;
+            assigned-clock-rates = <24000000>;
+
+            orientation = <0>;
+
+            avdd-supply = <&vreg_l7m_2p9>;
+            dvdd-supply = <&vreg_l7m_2p9>;
+            dovdd-supply = <&vreg_l4m_1p8>;
+
+            port {
+                hm1092_ep: endpoint {
+                    link-frequencies = /bits/ 64 <400000000>;
+                    remote-endpoint = <&camss_csiphy0_inep>;
+                };
+            };
+        };
+    };
+...

-- 
2.53.0

[PATCH v5 2/2] media: i2c: hm1092: add Himax HM1092 mono NIR sensor driver

[Ramshouriesh R]

Add a V4L2 subdev driver for the Himax HM1092, a 1 megapixel monochrome
near-infrared image sensor with a single-lane MIPI CSI-2 interface,
typically used as the face-authentication IR camera on laptops.

The driver supports the sensor's native 560x360 10-bit mono mode, a test
pattern control and the standard fwnode properties (orientation,
rotation).

Signed-off-by: Ramshouriesh R <rshouriesh@gmail.com>
---
 MAINTAINERS                |   7 +
 drivers/media/i2c/Kconfig  |  11 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/hm1092.c | 767 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 786 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5e11dd741d9f..4d9a4991ca2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11652,6 +11652,13 @@ L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	drivers/misc/hisi_hikey_usb.c
 
+HIMAX HM1092 SENSOR DRIVER
+M:	Ramshouriesh R <rshouriesh@gmail.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/media/i2c/himax,hm1092.yaml
+F:	drivers/media/i2c/hm1092.c
+
 HIMAX HX83112B TOUCHSCREEN SUPPORT
 M:	Job Noorman <job@noorman.info>
 L:	linux-input@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 5d173e0ecf42..f94621edbe1a 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -137,6 +137,17 @@ config VIDEO_HI847
           To compile this driver as a module, choose M here: the
           module will be called hi847.
 
+config VIDEO_HM1092
+	tristate "Himax HM1092 sensor support"
+	select V4L2_CCI_I2C
+	help
+	  This is a Video4Linux2 sensor driver for the Himax HM1092
+	  monochrome near-infrared CMOS image sensor, used as the
+	  face-authentication IR camera on laptops.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called hm1092.
+
 config VIDEO_IMX111
 	tristate "Sony IMX111 sensor support"
 	select V4L2_CCI_I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index e45359efe0e4..6507c173d0fa 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_VIDEO_GC2145) += gc2145.o
 obj-$(CONFIG_VIDEO_HI556) += hi556.o
 obj-$(CONFIG_VIDEO_HI846) += hi846.o
 obj-$(CONFIG_VIDEO_HI847) += hi847.o
+obj-$(CONFIG_VIDEO_HM1092) += hm1092.o
 obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
 obj-$(CONFIG_VIDEO_IMX111) += imx111.o
 obj-$(CONFIG_VIDEO_IMX208) += imx208.o
diff --git a/drivers/media/i2c/hm1092.c b/drivers/media/i2c/hm1092.c
new file mode 100644
index 000000000000..c7cde6928bb9
--- /dev/null
+++ b/drivers/media/i2c/hm1092.c
@@ -0,0 +1,767 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Himax HM1092 monochrome near-infrared image sensor driver.
+ *
+ * Copyright (c) 2026 Ramshouriesh R <rshouriesh@gmail.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <media/v4l2-cci.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+
+#define HM1092_LINK_FREQ_400MHZ		400000000ULL
+#define HM1092_MCLK			24000000
+#define HM1092_BITS_PER_SAMPLE		10
+
+#define HM1092_REG_STREAM		CCI_REG8(0x0100)
+#define HM1092_STREAM_STANDBY		0x00
+#define HM1092_STREAM_ON		0x01
+
+#define HM1092_REG_TEST_PATTERN		CCI_REG8(0x0601)
+#define HM1092_TEST_PATTERN_MAX		4
+
+/*
+ * Pixel array geometry, as programmed by the streaming init sequence:
+ * the readout window spans X_ADDR [0x0030..0x04ad] and Y_ADDR
+ * [0x0008..0x02d5], i.e. a 1150x718 active area whose bottom-right corner
+ * is the last addressable pixel. There is a single fixed mode that 2x bins
+ * this window down to the 560x360 output.
+ */
+#define HM1092_NATIVE_WIDTH		1198U
+#define HM1092_NATIVE_HEIGHT		726U
+#define HM1092_ACTIVE_LEFT		48U
+#define HM1092_ACTIVE_TOP		8U
+#define HM1092_ACTIVE_WIDTH		1150U
+#define HM1092_ACTIVE_HEIGHT		718U
+
+static const struct cci_reg_sequence hm1092_init_regs[] = {
+	{ CCI_REG8(0x0103), 0x00 },
+	{ CCI_REG8(0x030a), 0x05 },
+	{ CCI_REG8(0x030d), 0x0c },
+	{ CCI_REG8(0x030f), 0x5a },
+	{ CCI_REG8(0x0307), 0x00 },
+	{ CCI_REG8(0x0309), 0x01 },
+	{ CCI_REG8(0x0387), 0x01 },
+	{ CCI_REG8(0x0100), 0x02 },
+	{ CCI_REG8(0x4265), 0x02 },
+	{ CCI_REG8(0x4002), 0x2b },
+	{ CCI_REG8(0x4001), 0x00 },
+	{ CCI_REG8(0x0101), 0x03 },
+	{ CCI_REG8(0x4024), 0x40 },
+	{ CCI_REG8(0x0203), 0xbe },
+	{ CCI_REG8(0x0202), 0x00 },
+	{ CCI_REG8(0x0341), 0xee },
+	{ CCI_REG8(0x0340), 0x02 },
+	{ CCI_REG8(0x0343), 0x50 },
+	{ CCI_REG8(0x0342), 0x06 },
+	{ CCI_REG8(0x0345), 0x30 },
+	{ CCI_REG8(0x0344), 0x00 },
+	{ CCI_REG8(0x0349), 0xad },
+	{ CCI_REG8(0x0348), 0x04 },
+	{ CCI_REG8(0x0347), 0x08 },
+	{ CCI_REG8(0x0346), 0x00 },
+	{ CCI_REG8(0x034b), 0xd5 },
+	{ CCI_REG8(0x034a), 0x02 },
+	{ CCI_REG8(0x5015), 0xb3 },
+	{ CCI_REG8(0x0350), 0x53 },
+	{ CCI_REG8(0x0361), 0x30 },
+	{ CCI_REG8(0x0360), 0x00 },
+	{ CCI_REG8(0x034d), 0x30 },
+	{ CCI_REG8(0x034c), 0x02 },
+	{ CCI_REG8(0x034f), 0x68 },
+	{ CCI_REG8(0x034e), 0x01 },
+	{ CCI_REG8(0x0390), 0x03 },
+	{ CCI_REG8(0x0383), 0x00 },
+	{ CCI_REG8(0x0387), 0x10 },
+	{ CCI_REG8(0x50dd), 0x01 },
+	{ CCI_REG8(0x50cb), 0x21 },
+	{ CCI_REG8(0x5005), 0x28 },
+	{ CCI_REG8(0x5004), 0x40 },
+	{ CCI_REG8(0x5007), 0x28 },
+	{ CCI_REG8(0x5006), 0x40 },
+	{ CCI_REG8(0x5011), 0x00 },
+	{ CCI_REG8(0x501d), 0x4c },
+	{ CCI_REG8(0x5013), 0x03 },
+	{ CCI_REG8(0x4131), 0x01 },
+	{ CCI_REG8(0x5283), 0x03 },
+	{ CCI_REG8(0x5282), 0xff },
+	{ CCI_REG8(0x5010), 0x20 },
+	{ CCI_REG8(0x4132), 0x20 },
+	{ CCI_REG8(0x50d5), 0xe0 },
+	{ CCI_REG8(0x50d7), 0x12 },
+	{ CCI_REG8(0x50bb), 0x14 },
+	{ CCI_REG8(0x50b7), 0x00 },
+	{ CCI_REG8(0x50b9), 0xff },
+	{ CCI_REG8(0x50b8), 0x70 },
+	{ CCI_REG8(0x50ba), 0xff },
+	{ CCI_REG8(0x50fa), 0x02 },
+	{ CCI_REG8(0x50b4), 0x00 },
+	{ CCI_REG8(0x50a2), 0x0b },
+	{ CCI_REG8(0x50ad), 0x07 },
+	{ CCI_REG8(0x50ac), 0x24 },
+	{ CCI_REG8(0x50af), 0x40 },
+	{ CCI_REG8(0x50ae), 0x20 },
+	{ CCI_REG8(0x50ab), 0x07 },
+	{ CCI_REG8(0x50aa), 0x22 },
+	{ CCI_REG8(0x50a7), 0x00 },
+	{ CCI_REG8(0x50a6), 0x00 },
+	{ CCI_REG8(0x5099), 0x11 },
+	{ CCI_REG8(0x509b), 0x03 },
+	{ CCI_REG8(0x50b3), 0x04 },
+	{ CCI_REG8(0x50a0), 0x30 },
+	{ CCI_REG8(0x5098), 0x00 },
+	{ CCI_REG8(0x52f2), 0x53 },
+	{ CCI_REG8(0x5209), 0x0c },
+	{ CCI_REG8(0x5216), 0x02 },
+	{ CCI_REG8(0x521e), 0x01 },
+	{ CCI_REG8(0x50e8), 0x00 },
+	{ CCI_REG8(0x5200), 0x60 },
+	{ CCI_REG8(0x5202), 0x00 },
+	{ CCI_REG8(0x5201), 0x80 },
+	{ CCI_REG8(0x5203), 0x01 },
+	{ CCI_REG8(0x5208), 0x0b },
+	{ CCI_REG8(0x520d), 0x40 },
+	{ CCI_REG8(0x520c), 0x15 },
+	{ CCI_REG8(0x5215), 0x04 },
+	{ CCI_REG8(0x50ea), 0x74 },
+	{ CCI_REG8(0x5214), 0x28 },
+	{ CCI_REG8(0x5218), 0x07 },
+	{ CCI_REG8(0x5217), 0x01 },
+	{ CCI_REG8(0x0310), 0x00 },
+	{ CCI_REG8(0x4b31), 0x06 },
+	{ CCI_REG8(0x4b3b), 0x02 },
+	{ CCI_REG8(0x4b45), 0x01 },
+	{ CCI_REG8(0x4b44), 0x0c },
+	{ CCI_REG8(0x4b47), 0x00 },
+	{ CCI_REG8(0x5101), 0x13 },
+	{ CCI_REG8(0x5100), 0x03 },
+	{ CCI_REG8(0x5103), 0x33 },
+	{ CCI_REG8(0x5102), 0x23 },
+	{ CCI_REG8(0x5105), 0x42 },
+	{ CCI_REG8(0x5104), 0x43 },
+	{ CCI_REG8(0x5106), 0x40 },
+	{ CCI_REG8(0x5119), 0x00 },
+	{ CCI_REG8(0x5118), 0x00 },
+	{ CCI_REG8(0x511b), 0x00 },
+	{ CCI_REG8(0x511a), 0x00 },
+	{ CCI_REG8(0x511d), 0x00 },
+	{ CCI_REG8(0x511c), 0x00 },
+	{ CCI_REG8(0x511e), 0x00 },
+	{ CCI_REG8(0x5131), 0x23 },
+	{ CCI_REG8(0x5130), 0x13 },
+	{ CCI_REG8(0x5133), 0x43 },
+	{ CCI_REG8(0x5132), 0x33 },
+	{ CCI_REG8(0x5135), 0x40 },
+	{ CCI_REG8(0x5134), 0x42 },
+	{ CCI_REG8(0x5136), 0x40 },
+	{ CCI_REG8(0x5149), 0x01 },
+	{ CCI_REG8(0x5148), 0x01 },
+	{ CCI_REG8(0x514b), 0x01 },
+	{ CCI_REG8(0x514a), 0x01 },
+	{ CCI_REG8(0x514d), 0x01 },
+	{ CCI_REG8(0x514c), 0x01 },
+	{ CCI_REG8(0x514e), 0x01 },
+	{ CCI_REG8(0x51c0), 0x00 },
+	{ CCI_REG8(0x51c6), 0x00 },
+	{ CCI_REG8(0x51cc), 0x00 },
+	{ CCI_REG8(0x51d2), 0x00 },
+	{ CCI_REG8(0x51d8), 0x00 },
+	{ CCI_REG8(0x51c1), 0x81 },
+	{ CCI_REG8(0x51c7), 0x81 },
+	{ CCI_REG8(0x51cd), 0x81 },
+	{ CCI_REG8(0x51d3), 0x81 },
+	{ CCI_REG8(0x51d9), 0x81 },
+	{ CCI_REG8(0x51c2), 0xec },
+	{ CCI_REG8(0x51c8), 0xec },
+	{ CCI_REG8(0x51ce), 0xec },
+	{ CCI_REG8(0x51d4), 0xec },
+	{ CCI_REG8(0x51da), 0xec },
+	{ CCI_REG8(0x51c3), 0x00 },
+	{ CCI_REG8(0x51c9), 0x00 },
+	{ CCI_REG8(0x51cf), 0x00 },
+	{ CCI_REG8(0x51d5), 0x00 },
+	{ CCI_REG8(0x51db), 0x00 },
+	{ CCI_REG8(0x51c4), 0x55 },
+	{ CCI_REG8(0x51ca), 0x55 },
+	{ CCI_REG8(0x51d0), 0x54 },
+	{ CCI_REG8(0x51d6), 0x53 },
+	{ CCI_REG8(0x51dc), 0x53 },
+	{ CCI_REG8(0x51c5), 0x44 },
+	{ CCI_REG8(0x51cb), 0x24 },
+	{ CCI_REG8(0x51d1), 0x24 },
+	{ CCI_REG8(0x51d7), 0x14 },
+	{ CCI_REG8(0x51dd), 0x14 },
+	{ CCI_REG8(0x51e0), 0x09 },
+	{ CCI_REG8(0x51e2), 0x04 },
+	{ CCI_REG8(0x51e4), 0x08 },
+	{ CCI_REG8(0x51e6), 0x08 },
+	{ CCI_REG8(0x51e1), 0x03 },
+	{ CCI_REG8(0x51e3), 0x03 },
+	{ CCI_REG8(0x51e5), 0x07 },
+	{ CCI_REG8(0x51e8), 0x04 },
+	{ CCI_REG8(0x51e7), 0x07 },
+	{ CCI_REG8(0x51e9), 0x46 },
+	{ CCI_REG8(0x51eb), 0x62 },
+	{ CCI_REG8(0x51ea), 0x43 },
+	{ CCI_REG8(0x51ed), 0x00 },
+	{ CCI_REG8(0x51ec), 0x61 },
+	{ CCI_REG8(0x51ee), 0x00 },
+	{ CCI_REG8(0x5206), 0x80 },
+	{ CCI_REG8(0x3110), 0x02 },
+	{ CCI_REG8(0x3704), 0x02 },
+	{ CCI_REG8(0x3704), 0x02 },
+	{ CCI_REG8(0x4b20), 0x9e },
+	{ CCI_REG8(0x4b18), 0x00 },
+	{ CCI_REG8(0x4b3e), 0x00 },
+	{ CCI_REG8(0x4b0e), 0x0e },
+	{ CCI_REG8(0x4800), 0xac },
+	{ CCI_REG8(0x0104), 0x01 },
+	{ CCI_REG8(0x0104), 0x00 },
+	{ CCI_REG8(0x4801), 0xae },
+	{ CCI_REG8(0x0000), 0x00 },
+	{ CCI_REG8(0x0037), 0x30 },
+};
+
+struct hm1092_mode {
+	u32 width;
+	u32 height;
+	u32 hts;
+	u32 vts;
+};
+
+static const struct hm1092_mode hm1092_mode_560x360 = {
+	.width = 560,
+	.height = 360,
+	.hts = 0x0650,
+	.vts = 0x02ee,
+};
+
+static const char * const hm1092_supply_names[] = {
+	"dovdd",
+	"avdd",
+	"dvdd",
+};
+
+/*
+ * The available HM1092 documentation does not describe the test patterns.
+ * Their names are based on observed output from hardware testing; the
+ * register and mode values were reverse-engineered from the Windows driver.
+ */
+static const char * const hm1092_test_pattern_menu[] = {
+	"Disabled",
+	"Solid Color Fill",
+	"Standard Color Bars",
+	"Fade To Grey Color Bars",
+	"Pseudorandom data",
+};
+
+static const s64 hm1092_link_freq_menu[] = {
+	HM1092_LINK_FREQ_400MHZ,
+};
+
+struct hm1092 {
+	struct device *dev;
+	struct v4l2_subdev sd;
+	struct media_pad pad;
+	struct v4l2_ctrl_handler ctrl_handler;
+	struct regmap *regmap;
+	struct clk *img_clk;
+	struct gpio_desc *reset;
+	struct regulator_bulk_data supplies[ARRAY_SIZE(hm1092_supply_names)];
+	struct v4l2_ctrl *link_freq;
+	struct v4l2_ctrl *pixel_rate;
+	struct v4l2_ctrl *hblank;
+	struct v4l2_ctrl *vblank;
+	u8 mipi_lanes;
+};
+
+static inline struct hm1092 *to_hm1092(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct hm1092, sd);
+}
+
+static int hm1092_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct hm1092 *hm1092 = container_of(ctrl->handler, struct hm1092,
+					     ctrl_handler);
+	int ret;
+
+	/*
+	 * The control value is cached by the framework and (re)applied from
+	 * hm1092_enable_streams() once the device is powered up for streaming.
+	 */
+	if (pm_runtime_get_if_in_use(hm1092->dev) == 0)
+		return 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_TEST_PATTERN:
+		ret = cci_write(hm1092->regmap, HM1092_REG_TEST_PATTERN,
+				ctrl->val, NULL);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	pm_runtime_put(hm1092->dev);
+
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops hm1092_ctrl_ops = {
+	.s_ctrl = hm1092_set_ctrl,
+};
+
+static int hm1092_init_controls(struct hm1092 *hm1092)
+{
+	struct v4l2_ctrl_handler *ctrl_hdlr = &hm1092->ctrl_handler;
+	const struct hm1092_mode *mode = &hm1092_mode_560x360;
+	struct v4l2_fwnode_device_properties props;
+	s64 hblank, pixel_rate;
+	int ret;
+
+	ret = v4l2_fwnode_device_parse(hm1092->dev, &props);
+	if (ret)
+		return ret;
+
+	v4l2_ctrl_handler_init(ctrl_hdlr, 6);
+
+	hm1092->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
+						   &hm1092_ctrl_ops,
+						   V4L2_CID_LINK_FREQ,
+						   0, 0,
+						   hm1092_link_freq_menu);
+
+	pixel_rate = div_u64(HM1092_LINK_FREQ_400MHZ * 2 * hm1092->mipi_lanes,
+			     HM1092_BITS_PER_SAMPLE);
+	hm1092->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &hm1092_ctrl_ops,
+					       V4L2_CID_PIXEL_RATE, 0,
+					       pixel_rate, 1, pixel_rate);
+
+	hblank = mode->hts - mode->width;
+	hm1092->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &hm1092_ctrl_ops,
+					   V4L2_CID_HBLANK, hblank, hblank, 1,
+					   hblank);
+
+	hm1092->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &hm1092_ctrl_ops,
+					   V4L2_CID_VBLANK,
+					   mode->vts - mode->height,
+					   0xffff - mode->height, 1,
+					   mode->vts - mode->height);
+
+	v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &hm1092_ctrl_ops,
+				     V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(hm1092_test_pattern_menu) - 1,
+				     0, 0, hm1092_test_pattern_menu);
+
+	v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &hm1092_ctrl_ops, &props);
+
+	if (ctrl_hdlr->error) {
+		ret = ctrl_hdlr->error;
+		goto err_free;
+	}
+
+	hm1092->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	hm1092->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	hm1092->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	hm1092->sd.ctrl_handler = ctrl_hdlr;
+
+	return 0;
+
+err_free:
+	v4l2_ctrl_handler_free(ctrl_hdlr);
+
+	return ret;
+}
+
+static void hm1092_update_pad_format(struct v4l2_mbus_framefmt *fmt)
+{
+	fmt->width = hm1092_mode_560x360.width;
+	fmt->height = hm1092_mode_560x360.height;
+	fmt->code = MEDIA_BUS_FMT_Y10_1X10;
+	fmt->field = V4L2_FIELD_NONE;
+	fmt->colorspace = V4L2_COLORSPACE_RAW;
+	fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
+	fmt->xfer_func = V4L2_XFER_FUNC_NONE;
+}
+
+static int hm1092_enable_streams(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state,
+				 u32 pad, u64 streams_mask)
+{
+	struct hm1092 *hm1092 = to_hm1092(sd);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(hm1092->dev);
+	if (ret)
+		return ret;
+
+	ret = cci_multi_reg_write(hm1092->regmap, hm1092_init_regs,
+				  ARRAY_SIZE(hm1092_init_regs), NULL);
+	if (ret) {
+		dev_err(hm1092->dev, "failed to write init registers\n");
+		goto out;
+	}
+
+	ret = __v4l2_ctrl_handler_setup(hm1092->sd.ctrl_handler);
+	if (ret)
+		goto out;
+
+	ret = cci_write(hm1092->regmap, HM1092_REG_STREAM,
+			HM1092_STREAM_ON, NULL);
+	if (ret)
+		dev_err(hm1092->dev, "failed to start streaming\n");
+
+out:
+	if (ret)
+		pm_runtime_put(hm1092->dev);
+
+	return ret;
+}
+
+static int hm1092_disable_streams(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state,
+				  u32 pad, u64 streams_mask)
+{
+	struct hm1092 *hm1092 = to_hm1092(sd);
+	int ret;
+
+	ret = cci_write(hm1092->regmap, HM1092_REG_STREAM,
+			HM1092_STREAM_STANDBY, NULL);
+	pm_runtime_put(hm1092->dev);
+
+	return ret;
+}
+
+static int hm1092_enum_mbus_code(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state,
+				 struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->index)
+		return -EINVAL;
+
+	code->code = MEDIA_BUS_FMT_Y10_1X10;
+
+	return 0;
+}
+
+static int hm1092_enum_frame_size(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state,
+				  struct v4l2_subdev_frame_size_enum *fse)
+{
+	if (fse->index)
+		return -EINVAL;
+
+	if (fse->code != MEDIA_BUS_FMT_Y10_1X10)
+		return -EINVAL;
+
+	fse->min_width = hm1092_mode_560x360.width;
+	fse->max_width = hm1092_mode_560x360.width;
+	fse->min_height = hm1092_mode_560x360.height;
+	fse->max_height = hm1092_mode_560x360.height;
+
+	return 0;
+}
+
+static int hm1092_get_selection(struct v4l2_subdev *sd,
+				struct v4l2_subdev_state *state,
+				struct v4l2_subdev_selection *sel)
+{
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		sel->r.left = HM1092_ACTIVE_LEFT;
+		sel->r.top = HM1092_ACTIVE_TOP;
+		sel->r.width = HM1092_ACTIVE_WIDTH;
+		sel->r.height = HM1092_ACTIVE_HEIGHT;
+		return 0;
+	case V4L2_SEL_TGT_NATIVE_SIZE:
+		sel->r.left = 0;
+		sel->r.top = 0;
+		sel->r.width = HM1092_NATIVE_WIDTH;
+		sel->r.height = HM1092_NATIVE_HEIGHT;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int hm1092_init_state(struct v4l2_subdev *sd,
+			     struct v4l2_subdev_state *state)
+{
+	hm1092_update_pad_format(v4l2_subdev_state_get_format(state, 0));
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops hm1092_video_ops = {
+	.s_stream = v4l2_subdev_s_stream_helper,
+};
+
+static const struct v4l2_subdev_pad_ops hm1092_pad_ops = {
+	.set_fmt = v4l2_subdev_get_fmt,
+	.get_fmt = v4l2_subdev_get_fmt,
+	.enum_mbus_code = hm1092_enum_mbus_code,
+	.enum_frame_size = hm1092_enum_frame_size,
+	.get_selection = hm1092_get_selection,
+	.enable_streams = hm1092_enable_streams,
+	.disable_streams = hm1092_disable_streams,
+};
+
+static const struct v4l2_subdev_ops hm1092_subdev_ops = {
+	.video = &hm1092_video_ops,
+	.pad = &hm1092_pad_ops,
+};
+
+static const struct media_entity_operations hm1092_entity_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static const struct v4l2_subdev_internal_ops hm1092_internal_ops = {
+	.init_state = hm1092_init_state,
+};
+
+static int hm1092_check_hwcfg(struct hm1092 *hm1092)
+{
+	struct v4l2_fwnode_endpoint bus_cfg = {
+		.bus = {
+			.mipi_csi2 = {
+				.num_data_lanes = 1,
+			},
+		},
+		.bus_type = V4L2_MBUS_CSI2_DPHY,
+	};
+	struct device *dev = hm1092->dev;
+	struct fwnode_handle *ep, *fwnode = dev_fwnode(dev);
+	unsigned long link_freq_bitmap;
+	int ret;
+
+	ep = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0, 0);
+	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
+	fwnode_handle_put(ep);
+	if (ret)
+		return dev_err_probe(dev, ret, "parsing endpoint failed\n");
+
+	ret = v4l2_link_freq_to_bitmap(dev, bus_cfg.link_frequencies,
+				       bus_cfg.nr_of_link_frequencies,
+				       hm1092_link_freq_menu,
+				       ARRAY_SIZE(hm1092_link_freq_menu),
+				       &link_freq_bitmap);
+	if (ret)
+		goto out;
+
+	if (bus_cfg.bus.mipi_csi2.num_data_lanes != 1) {
+		ret = dev_err_probe(dev, -EINVAL,
+				    "only 1 data lane is supported, got %u\n",
+				    bus_cfg.bus.mipi_csi2.num_data_lanes);
+		goto out;
+	}
+
+	hm1092->mipi_lanes = bus_cfg.bus.mipi_csi2.num_data_lanes;
+
+out:
+	v4l2_fwnode_endpoint_free(&bus_cfg);
+	return ret;
+}
+
+static int hm1092_get_pm_resources(struct hm1092 *hm1092)
+{
+	hm1092->reset = devm_gpiod_get_optional(hm1092->dev, "reset",
+						GPIOD_OUT_HIGH);
+	if (IS_ERR(hm1092->reset))
+		return dev_err_probe(hm1092->dev, PTR_ERR(hm1092->reset),
+				     "failed to get reset gpio\n");
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(hm1092_supply_names); i++)
+		hm1092->supplies[i].supply = hm1092_supply_names[i];
+
+	return devm_regulator_bulk_get(hm1092->dev,
+				       ARRAY_SIZE(hm1092_supply_names),
+				       hm1092->supplies);
+}
+
+static int hm1092_power_off(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct hm1092 *hm1092 = to_hm1092(sd);
+
+	gpiod_set_value_cansleep(hm1092->reset, 1);
+	clk_disable_unprepare(hm1092->img_clk);
+	regulator_bulk_disable(ARRAY_SIZE(hm1092_supply_names),
+			       hm1092->supplies);
+
+	return 0;
+}
+
+static int hm1092_power_on(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct hm1092 *hm1092 = to_hm1092(sd);
+	int ret;
+
+	/*
+	 * Power-up sequence:
+	 *   1. enable all rails (~3 ms ramp)
+	 *   2. hold reset asserted
+	 *   3. start MCLK and let the sensor clock for ~1 ms
+	 *   4. release reset and wait 18 ms for the sensor to come up
+	 */
+	ret = regulator_bulk_enable(ARRAY_SIZE(hm1092_supply_names),
+				    hm1092->supplies);
+	if (ret)
+		return ret;
+	fsleep(3000);
+
+	gpiod_set_value_cansleep(hm1092->reset, 1);
+
+	ret = clk_prepare_enable(hm1092->img_clk);
+	if (ret) {
+		regulator_bulk_disable(ARRAY_SIZE(hm1092_supply_names),
+				       hm1092->supplies);
+		return ret;
+	}
+	fsleep(1000);
+
+	gpiod_set_value_cansleep(hm1092->reset, 0);
+	fsleep(18000);
+
+	return 0;
+}
+
+static void hm1092_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct hm1092 *hm1092 = to_hm1092(sd);
+
+	v4l2_async_unregister_subdev(sd);
+	v4l2_subdev_cleanup(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
+	pm_runtime_disable(hm1092->dev);
+	if (!pm_runtime_status_suspended(hm1092->dev)) {
+		hm1092_power_off(hm1092->dev);
+		pm_runtime_set_suspended(hm1092->dev);
+	}
+}
+
+static int hm1092_probe(struct i2c_client *client)
+{
+	struct hm1092 *hm1092;
+	unsigned long freq;
+	int ret;
+
+	hm1092 = devm_kzalloc(&client->dev, sizeof(*hm1092), GFP_KERNEL);
+	if (!hm1092)
+		return -ENOMEM;
+
+	hm1092->dev = &client->dev;
+
+	hm1092->img_clk = devm_v4l2_sensor_clk_get(hm1092->dev, NULL);
+	if (IS_ERR(hm1092->img_clk))
+		return dev_err_probe(hm1092->dev, PTR_ERR(hm1092->img_clk),
+				     "failed to get imaging clock\n");
+
+	freq = clk_get_rate(hm1092->img_clk);
+	if (freq != HM1092_MCLK)
+		return dev_err_probe(hm1092->dev, -EINVAL,
+				     "external clock %lu is not supported\n",
+				     freq);
+
+	v4l2_i2c_subdev_init(&hm1092->sd, client, &hm1092_subdev_ops);
+
+	ret = hm1092_check_hwcfg(hm1092);
+	if (ret)
+		return ret;
+
+	ret = hm1092_get_pm_resources(hm1092);
+	if (ret)
+		return ret;
+
+	hm1092->regmap = devm_cci_regmap_init_i2c(client, 16);
+	if (IS_ERR(hm1092->regmap))
+		return PTR_ERR(hm1092->regmap);
+
+	ret = hm1092_power_on(hm1092->dev);
+	if (ret)
+		return dev_err_probe(hm1092->dev, ret, "failed to power on\n");
+
+	ret = hm1092_init_controls(hm1092);
+	if (ret)
+		goto err_power_off;
+
+	hm1092->sd.internal_ops = &hm1092_internal_ops;
+	hm1092->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	hm1092->sd.entity.ops = &hm1092_entity_ops;
+	hm1092->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+	hm1092->pad.flags = MEDIA_PAD_FL_SOURCE;
+	ret = media_entity_pads_init(&hm1092->sd.entity, 1, &hm1092->pad);
+	if (ret)
+		goto err_ctrls;
+
+	hm1092->sd.state_lock = hm1092->ctrl_handler.lock;
+	ret = v4l2_subdev_init_finalize(&hm1092->sd);
+	if (ret)
+		goto err_entity;
+
+	pm_runtime_set_active(hm1092->dev);
+	pm_runtime_enable(hm1092->dev);
+
+	ret = v4l2_async_register_subdev_sensor(&hm1092->sd);
+	if (ret)
+		goto err_subdev;
+
+	pm_runtime_idle(hm1092->dev);
+
+	return 0;
+
+err_subdev:
+	pm_runtime_disable(hm1092->dev);
+	pm_runtime_set_suspended(hm1092->dev);
+	v4l2_subdev_cleanup(&hm1092->sd);
+
+err_entity:
+	media_entity_cleanup(&hm1092->sd.entity);
+
+err_ctrls:
+	v4l2_ctrl_handler_free(hm1092->sd.ctrl_handler);
+
+err_power_off:
+	hm1092_power_off(hm1092->dev);
+
+	return ret;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(hm1092_pm_ops, hm1092_power_off,
+				 hm1092_power_on, NULL);
+
+static const struct of_device_id hm1092_of_match[] = {
+	{ .compatible = "himax,hm1092" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, hm1092_of_match);
+
+static struct i2c_driver hm1092_i2c_driver = {
+	.driver = {
+		.name = "hm1092",
+		.pm = pm_ptr(&hm1092_pm_ops),
+		.of_match_table = hm1092_of_match,
+	},
+	.probe = hm1092_probe,
+	.remove = hm1092_remove,
+};
+
+module_i2c_driver(hm1092_i2c_driver);
+
+MODULE_DESCRIPTION("Himax HM1092 monochrome NIR image sensor driver");
+MODULE_LICENSE("GPL");

-- 
2.53.0

Re: [PATCH v5 0/2] media: Add Himax HM1092 mono NIR sensor driver

[Jake Steinman]

Elgin, thank you for the pointer — I had not found this series and I am glad to
have it before anyone spent review time on mine.

Ramshouriesh, your series is further through review than mine — v5, a binding
with a Reviewed-by, and a driver already shaped the way the subsystem wants. I
have no interest in a turf fight over which of us writes drivers/media/i2c
support for this part, and I am not going to keep pushing mine in parallel.

I do want to be direct about the technical picture before anything is applied,
because I think my series is ahead of yours on the hardware itself and some of
the difference is load-bearing:

  * The active pixel array your driver reports to userspace does not match the
    vendor's published figure, and cannot be right.
  * Your LINK_FREQ does not reconcile with your own hts/vts at either frame
    rate Himax specifies for this part.
  * The exposure and gain register layout you describe as unknown, I have.

I have also validated differently, and I think the difference matters. Your
cover letter reports the on-chip test patterns arriving through the CSI-2
pipeline. What I have is the complete function this part exists for:

  * Live imaging, not test patterns: 8 captured frames with an 87.9 % byte
    difference between consecutive frames, i.e. scene content rather than a
    static or repeated buffer.

  * Working infrared face authentication — the Windows Hello equivalent — via
    Howdy, unlocking a real lock screen against an enrolled model. That is the
    whole application: it exercises the sensor, the illuminator, exposure, and
    the capture path together, under an unprivileged process, in the dark.

  * s2idle suspend and resume with the sensor bound; it survives and still
    streams afterwards.

  * A second machine, someone else's, confirmed independently this week: a
    different Dell laptop where the same sensor binds and streams at
    link_frequency 180480000 on CSI-2 port 1, I2C bus 15, capture node
    /dev/video8 — every one of those different from mine. Nine SOF events and
    frames out. That is two distinct boards and two distinct wiring
    configurations.

  * The register table cross-checked two independent ways — extracted from the
    vendor Windows binary, and decoded from USB/I2C captures of that driver
    programming the part. The two agree.

None of that makes my driver the one that should land. It does mean these three
items are worth resolving before yours does, and I would rather hand you the
answers than have the tree carry them.

## 1. LINK_FREQ does not match your own timing at either supported frame rate

Your driver declares:

  #define HM1092_LINK_FREQ_400MHZ   400000000ULL

and your mode declares .hts = 0x0650 (1616) and .vts = 0x02ee (750), which your
own register writes confirm (0x0342/0x0343 = 0x06,0x50 and 0x0340/0x0341 =
0x02,0xee). V4L2_CID_LINK_FREQ is the DDR clock, i.e. half the per-lane bit
rate, so for one lane at 10 bpp:

  30 fps:  1616 x 750 x 30 x 10 / 2 = 181,800,000
  60 fps:  1616 x 750 x 60 x 10 / 2 = 363,600,000

400,000,000 is 2.2x the first and 1.10x the second. Himax gives only those two
rates for this part — 30 FPS at full HD, 60 FPS in the 640x360 binned mode —
and I could not find a frame rate stated anywhere in your driver to say which
one you are in.

If you are running the binned mode at 60 fps then you are close, and 400 MHz
looks like a rounded or maximum figure rather than the actual link rate; the
derived pixel_rate would then be about 10 % high, which shows up in userspace as
frame durations and exposure times that are consistently off by that much. If
you are at 30 fps, it is out by 2.2x.

I ask because I lost three months to exactly this control. My driver published
360,960,000 — which is what the vendor's Windows mode descriptor states, and
which is the per-lane BIT rate, not the DDR clock. The IPU7 receiver programmed
its D-PHY for ~721 Mbps against a sensor transmitting ~361, the clock lane came
up, no data packet ever framed, and there was no error anywhere. It presents
exactly like dead silicon. Halving it to 180,480,000 made it stream.

One data point that may or may not apply to you: your init sequence programs the
PLL identically to mine — 0x0307 = 0x00, 0x0309 = 0x01, 0x0310 = 0x00 are
byte-for-byte what my table writes. On my board that PLL gives 19.2 MHz x 94 /
5 = 360.96 MHz bit rate, i.e. 180.48 MHz DDR. If your MCLK is also 19.2 MHz then
the part is putting the same rate on the wire regardless of which mode is
selected, and 400,000,000 does not correspond to it. Himax specify EXTCLK
anywhere from 6 to 27 MHz, so your board may well clock it differently — that
is the part I cannot check from here.

You report that the sensor streams and the on-chip test patterns arrive through
the full CSI-2 pipeline, and I do not doubt it. If your receiver takes its PHY
configuration from DT or from a separate PHY driver rather than from this
control, a wrong value here would stream fine on your board and still be wrong
for a receiver that honours it. Mine honours it, and the failure is silent and
total.

## 2. Exposure and gain — you say the register map is unknown, and I have it

From your cover letter:

  "this driver does not implement the V4L2_CID_ANALOGUE_GAIN and
   V4L2_CID_EXPOSURE controls that libcamera lists as mandatory for RAW
   sensors: the same lack of documentation means the gain and exposure
   register layout is not known"

I recovered it:

  0x0202 / 0x0203   exposure, 16-bit, high byte first
  0x0205            analogue gain, 8-bit
  0x020e / 0x020f   digital gain, 16-bit, high byte first
  0x0104            group hold: write 1, program the group, write 0

You already have 0x0104 and 0x0202/0x0203 in your init table — your sequence
sets 0x0203 to 0xbe (190) where mine sets 0xb4 (180), which is consistent with
these being the same exposure register at slightly different defaults.

I have exercised these on my hardware but not against your 560x360 mode, so
they need testing on your board before you take them. But the gap they close is
not cosmetic: your own cover letter identifies these as controls libcamera lists
as mandatory for RAW sensors, and a sensor driven at whatever fixed exposure the
init sequence happens to bake in cannot do face authentication in varying light,
which is the entire application for this part. I would rather that not be a
known hole in the merged driver.

Happy to send them as a patch on top of your series.

## 3. Geometry — the vendor's public page disagrees with your pixel array

Himax publish the part's parameters here:

  https://www.himax.com.tw/products/cmos-image-sensor/image-sensors/hm1092/

  Active Pixel Array   1280 x 720
  Readout Modes        Full (HD), Sub / Bin
  Frame Rate           30 FPS HD, 60 FPS 640 x 360
  Reference Clock      6 - 27 MHz
                       1-lane MIPI CSI-2, RAW8/10, monochrome

Your get_selection reports NATIVE 1198x726 and ACTIVE 1150x718. I do not think
either can be right: the active array is 1280x720, and the tables I extracted
from the Windows driver contain a 1296x736 mode, which is larger than the native
size your driver reports. A native size smaller than a mode the part can output
would confuse anything that uses these targets to reason about scaling or crop.

For what it is worth, my own 648x368 falls out of the same page: the binned mode
is 640x360, and 648x368 is that plus 8 columns and 8 rows of overscan. Your
560x360 is a narrower crop of the same binned window, which is entirely
plausible as a different vendor configuration — I am not suggesting your output
size is wrong, only the pixel-array geometry you report alongside it.

## What I would still like to send

My remaining patch adds a HIMX1092 entry to ipu-bridge so the software fwnode
graph can be built on ACPI x86 platforms. It touches
drivers/media/pci/intel/ipu-bridge.c only, does not depend on which sensor
driver is used, and is orthogonal to your series. I will rebase it on top of
yours.

I can also offer testing on two platforms very different from yours, both x86
with the sensor ACPI-enumerated as HIMX1092 behind a Synaptics SVP7500 bridge
feeding an Intel IPU7 — one lane, but on different CSI-2 ports, different I2C
buses and different bridge ACPI ids on the two machines. Once your driver runs
on them I will send a Tested-by from each, and report honestly if it does not.
The second machine is another user who has been debugging this with me in the
open and who is willing to test.

That is worth something specific to your series: it is currently validated on
one board, and your cover letter says as much. A driver that has been shown to
work on an ARM device-tree platform and two x86 ACPI ones, with the sensor
wired differently on each, is a much easier thing for a maintainer to take.

One note on provenance, since you describe your programming as recovered from
the Windows driver too: my register table was extracted from hm1092.sys
(version 81.26100.0.13, offset 0x26B34 in .rdata) and cross-checked against
USB/I2C bus captures of that driver programming the sensor. I state it plainly
because a maintainer should know that before taking a table.

To be plain about what I am proposing: take the three corrections, and I will
drop my sensor driver, send a Tested-by from a second architecture, and keep
only the ipu-bridge patch. That is a better outcome than two drivers, and a much
better one than a merged driver carrying a pixel array that does not exist and
no exposure control.

If you would rather not take them, say so and I will send my findings as
standalone patches against your driver once it lands, so the information is in
the tree either way. What I am not willing to do is stay quiet about them and
let someone else rediscover the link frequency the hard way — that one cost me
three months.

Thanks for doing this work. I would much rather help finish your driver than
land my own.

Jake Steinman

Re: [PATCH v5 0/2] media: Add Himax HM1092 mono NIR sensor driver

[Ramshouriesh R]

Hi Jake,

Honestly, thank you for this. I'd much rather end up with one good driver than
have the two of us shipping HM1092 support in parallel. I went through all three
of your points against my own numbers and you're right on every one of them.

On the link frequency: yeah, it doesn't add up, and I don't even need your board
to see it. My own registers contradict it. 1616 x 750 x 10 / 2 is 181.8 MHz at
30 fps and 363.6 at 60, and 400 MHz is neither. The pixel_rate I derive from it
works out to about 66 fps, which isn't a mode this part even has. So it's just
wrong.

The one thing I don't want to do is blindly paste your number in, because my
EXTCLK is 24 MHz, not 19.2, so your 19.2 x 94 / 5 = 360.96 doesn't land on my
board the same way. What I'll do instead is actually pin down the link rate here
(measured fps against hts x vts, plus the PLL dividers for a 24 MHz input) and
put whatever falls out of that in the cover letter (or commit message), with the
working shown. My guess is it ends up a lot closer to your 180.48 than to 400,
but I'd rather measure it than assert it. You already ate the three months on
this one, so I can at least not guess.

And you nailed why it streams fine for me anyway. My CSIPHY gets its config from
the PHY driver and DT, not from this control, so a bad LINK_FREQ is completely
invisible on my setup and totally fatal on a receiver that actually reads it.
That's exactly the kind of thing that shouldn't reach the tree, so I'm glad you
flagged it.

On exposure and gain: fair hit, I'll fix the cover. Saying the map is "unknown"
is just wrong now that you've posted it, and it lines up with what I already
have anyway. I write 0x0202/0x0203 and 0x0104 in my init table (0x0203 is 0xbe
for me vs your 0xb4, same register, different default), which matches your
exposure and group-hold registers. I'd really like that patch on top of my
series. Like you said they need testing against my 560x360 mode first, so let's
just do it in the open: send them over, I'll run them here and tell you straight
whether they work. A driver that can't do AE/AGC can't do the actual job this
sensor exists for, so that gap matters a lot more to me than any of the cosmetic
stuff.

Geometry: you're right, and I can see where I went wrong. Those get_selectio
numbers came off the binned readout-window registers, so I was describing the
crop window, not the pixel array. Reporting a native size (1198x726) that's
narrower than both the 1280x720 active array and the 1296x736 mode in the
Windows tables makes no sense, exactly like you said. I'll redo them against
real array geometry so the selection targets actually describe the sensor
instead of my one crop.

On the bigger picture, your offer is clearly the better outcome, so I'll tak
it. Keep your ipu-bridge HIMX1092 patch, it's orthogonal and I've got nothing
like it, and I'll fold the three fixes in. Getting this onto two x86/IPU7 bo
with the sensor wired differently on each is worth a lot for a series
that's only
been proven on one ARM DT board so far, and I'd be glad to carry a Tested-by
each once it runs there. And if it doesn't run, tell me, that's just as useful.

On provenance, since you were upfront about yours: mine came out of
com.qti.sensormodule.hm1092.bin, the Qualcomm sensor-module blob in the ASUS
Windows camera stack (QualcommBSP/qccamauxsensor_extension8380),
regSetting block
at offset 0x9e8f. That's a different vendor artifact from your hm1092.sys, s
between the two of us the table's now been pulled from two independent Windows
sources and they agree, which is about as good as this kind of thing gets. A
maintainer taking either should know where it came from.

So from my end: I'll respin with the link-freq derivation, fixed geometry, and
the cover corrected on exposure/gain, then sort out the gain/exposure patch
getting it onto your boards with you. If you'd sooner send your findings as
standalone patches instead, that's fine too. I just think folding them in is
cleaner.

Anyway, thanks for doing this out in the open instead of just landing
a competing
driver. Better result for the part, and for whoever has to debug it next.

Cheers,
Ramshouriesh

Re: [PATCH v5 0/2] media: Add Himax HM1092 mono NIR sensor driver

[Jake Steinman]

Ramshouriesh,

Good — and measuring your own link rate rather than pasting mine is the right
call. 24 MHz EXTCLK is exactly the sort of thing that would have made a
copied constant wrong in a new way, and my derivation only holds for a 19.2 MHz
part. I would rather your v6 carry a number you can defend than one of mine.

Your provenance is genuinely useful, by the way. Mine came out of Dell's
hm1092.sys, yours out of Qualcomm's com.qti.sensormodule.hm1092.bin in the ASUS
stack — two different vendors' Windows artifacts, extracted independently, and
the register sequences agree. That is much better evidence for whoever reviews
this than either of us had separately, and I think it belongs in the cover
letter.

If it would help, I can make that concrete rather than assertive: send me your
table as (addr, value) pairs in order and I will diff it against my 238 entries
and post the result — how many entries match exactly, which differ, and whether
the differences are all board-level defaults like the 0xb4/0xbe exposure you
already spotted. "Two independent extractions from two vendors' Windows stacks
agree on N of 238 entries, differing only in <list>" is a much stronger sentence
for a cover letter than either of us saying we trust our own dump. It also has a
real chance of catching a transcription error in one of them, which is worth
knowing before this lands rather than after.

## Exposure and gain

Here is what I have. Rather than send a patch against v5 that your respin will
immediately invalidate, take this as the implementation plus the notes that
matter, and I will send a proper patch against v6 once your geometry and
link-frequency changes have landed. Say the word if you would rather have it the
other way round.

Registers:

  0x0202 / 0x0203   exposure, 16-bit, high byte first
  0x0205            analogue gain, 8-bit
  0x020e / 0x020f   digital gain, 16-bit, high byte first
  0x0104            group hold: write 1, program the group, write 0

Ranges I use, on a 648x368 mode with VTS 741:

  exposure          min 2, max 720, step 1
  analogue gain     min 0, max 0xff, step 1
  digital gain      min 0x0100, max 0x0fff, step 1   (0x0100 = 1.0x)

The writes themselves, in CCI terms since you are already there:

  case V4L2_CID_EXPOSURE:
          cci_write(regmap, CCI_REG8(0x0202), ctrl->val >> 8, &ret);
          cci_write(regmap, CCI_REG8(0x0203), ctrl->val & 0xff, &ret);
          break;
  case V4L2_CID_ANALOGUE_GAIN:
          cci_write(regmap, CCI_REG8(0x0205), ctrl->val, &ret);
          break;
  case V4L2_CID_DIGITAL_GAIN:
          cci_write(regmap, CCI_REG8(0x020e), ctrl->val >> 8, &ret);
          cci_write(regmap, CCI_REG8(0x020f), ctrl->val & 0xff, &ret);
          break;

Four things worth knowing before you wire it up, because three of them cost me
time:

**1. Your control defaults must match what your init table leaves behind, not
mine.** This is the one I would most like you to check. The control handler
setup that runs at stream-on writes the defaults to the sensor, so if a default
differs from the value the init sequence already programmed, you silently stream
a configuration the part has never been shown to work in. My table ends with
0x0203 = 0xb4, so my exposure default is 180. **Yours ends with 0xbe, so your
default should be 190, not 180.** Same for gain: my table leaves 0x0205 = 0x00
so my default is 0. Check what yours leaves.

I hit this exactly: a refactor made the stream-on control setup a real hardware
write for the first time, and it started programming exposure 256 and gain 0x10
over a sequence that had only ever been validated at 180 and 0.

**2. Do not fold the 8-bit halves into CCI_REG16.** It is tempting — 0x0202/0x0203
and 0x020e/0x020f are adjacent — but a CCI_REG16 write becomes one
auto-incrementing burst rather than two transactions, and that no longer matches
the sequence the sensor is known to accept. I kept them as pairs deliberately.
Same reasoning applies to the 0x0340/0x0341 and 0x0342/0x0343 pairs in the init
table.

**3. Derive exposure max from VTS rather than hardcoding it.** Mine is 720
against VTS 741. Yours is VTS 750, so the equivalent is not the same number, and
a max above the frame length will just clip or misbehave. Whatever you choose,
worth confirming the sensor actually accepts it at the top of the range rather
than assuming.

**4. Group hold.** Bracket each group of exposure/gain writes with 0x0104 = 1
then 0x0104 = 0. You already have 0x0104 in your init table, so it is the same
mechanism; the vendor sequence uses it around every AE group.

I have exercised all of this on 648x368. It has never run against your 560x360
mode, and since that is a different crop of the binned window I would not assume
the exposure range transfers unchanged.

## The ipu-bridge patch, and testing

I will rebase the ipu-bridge HIMX1092 entry onto your v6 and send it as part of
your series rather than separately, unless you would rather I kept it standalone.

Once v6 is out I will build it on both x86/IPU7 machines — mine is a Dell XPS 16
DA16260, one lane on CSI-2 port 2 behind a Synaptics SVP7500; the other user is
on a different Dell, CSI-2 port 1, I2C bus 15, different bridge ACPI id — and
report back either way. If your driver does not come up on ACPI x86 I would
rather find that now than after it lands, and honestly it is the more likely
outcome for a first attempt: the fwnode graph on these machines is built by
ipu-bridge from ACPI SSDB data rather than described in DT, so the endpoint your
driver parses arrives by a different route than on your board.

Thanks for taking this the way you did.

Jake

Re: [PATCH v5 0/2] media: Add Himax HM1092 mono NIR sensor driver

[Ramshouriesh R]

Hi Jake,

I have v6 ready now. I went back to the ASUS Windows files rather than
deriving the link frequency from an assumed 30 fps measurement.

The Qualcomm power sequence explicitly requests a 24 MHz EXTCLK. The
sensor initialization programs:

0x030d = 0x0c = 12 (PLL pre-divider)
0x030f = 0x5a = 90 (PLL multiplier)

The resulting CSI-2 DDR link frequency is:

link_freq
= EXTCLK * multiplier / pre-divider
= 24,000,000 * 90 / 12
= 180,000,000 Hz

Because CSI-2 D-PHY transfers on both clock edges, the per-lane bit rate is:

lane_bit_rate
= 2 * link_freq
= 2 * 180,000,000
= 360,000,000 bit/s

The mode uses one data lane and RAW10, so the corresponding pixel rate is:

pixel_rate
= lane_bit_rate * number_of_lanes / bits_per_pixel
= 360,000,000 * 1 / 10
= 36,000,000 pixels/s

The initialization table programs:

HTS = 0x0650 = 1616 pixel clocks per line
VTS = 0x02ee = 750 lines per frame

Therefore, the calculated frame rate is:

frame_rate
= pixel_rate / (HTS * VTS)
= 36,000,000 / (1616 * 750)
= 36,000,000 / 1,212,000
= 29.702970... fps

So v6 now reports:

link frequency: 180 MHz
lane bit rate: 360 Mbit/s
pixel rate: 36 MHz
frame rate: approximately 29.7 fps

The binding and driver both use the 180 MHz link frequency.

I folded exposure, analogue gain and digital gain into v6 using your
register map and notes:

exposure uses separate writes to 0x0202 and 0x0203
analogue gain uses 0x0205
digital gain uses separate writes to 0x020e and 0x020f
group hold uses 0x0104 = 1 before an update and 0x0104 = 0 afterward

The exposure maximum is derived from the frame length:

exposure_max
= VTS - margin
= 750 - 21
= 729 lines

The exposure range is therefore 2..729 lines. I selected 500 lines as
the default from unstretched illuminated captures on the ASUS system:

500 decimal = 0x01f4

The init sequence and control setup both program it as:

0x0202 = 0x01
0x0203 = 0xf4

This keeps the sensor state and V4L2 control default consistent.

Analogue gain has range 0..255 and default 0.

Digital gain has:

minimum: 0x0100 = 256 = 1.0x
maximum: 0x0fff = 4095
default: 0x0100

I tested exposure 2, 500 and 729 and analogue gain 0 and 255 on the
560x360 mode. Those settings were accepted and changed real frame
levels in the expected direction. I also exercised the digital-gain
endpoints while streaming and captured valid frames. Runtime PM
returned the sensor to suspend after capture.

Here is the ordered Qualcomm table you requested: 185 (address, value)
entries regenerated directly from com.qti.sensormodule.hm1092.bin:

https://gist.github.com/sppidy/8b6b57f047343c88d4bb6c8ef22a910d

Please post the comparison against your Dell hm1092.sys table when you have it.

For ipu-bridge, I would prefer a follow-up patch on top of v6. That
keeps the x86/IPU7 ownership and testing explicit, since I cannot
exercise that path on the A14.

I currently credit your findings in the commit body, with Reported-by
and links to both messages, and in the cover letter. If you think
Suggested-by is more appropriate for the control implementation, or
want Co-developed-by with your Signed-off-by, let me know before I
send v6.

Regards,
Ramshouriesh