[PATCH v4 0/4] media: i2c: Add OX05B1S camera sensor driver

[PATCH v4 0/4] media: i2c: Add OX05B1S camera sensor driver

[Mirela Rabulea]

Add a v4l2 subdevice driver for the Omnivision OX05B1S RGB-IR sensor

The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an active array size
of 2592 x 1944.
    
The following features are supported for OX05B1S:
   - Manual exposure an gain control support
   - vblank/hblank control support
Supported resolution:
   - 2592 x 1944 @ 30fps (SGRBG10)

Support for another sensor, OS08A20, is added as a separate patch, using another compatible.
For OS08a20, HDR mode control is supported, with one HDR mode: staggered HDR with 2 exposures on separate virtual channels. However, for now, only one exposure (VC 0) is accessible via get_frame_desc.

Supported resolutions:
   - 1920 x 1080 @ 60fps (SBGGR10, no HDR)
   - 1920 x 1080 @ 30fps (SBGGR10, HDR)
   - 3840 x 2160 @ 30fps (SBGGR12, no HDR)
   - 3840 x 2160 @ 15fps (SBGGR12, HDR)
   - 3840 x 2160 @ 30fps (SBGGR10, no HDR)
   - 3840 x 2160 @ 15fps (SBGGR10, HDR)

The driver was tested on upstream 6.14-rc2 on imx8mp-evk, but also on nxp tree based on 6.12 on imx95-19x19-evk.

The results of v4l2-compliance test:

root@imx8mpevk:/unit_tests# ./v4l2-compliance -d /dev/video0
v4l2-compliance 1.29.0-5342, 64 bits, 64-bit time_t
v4l2-compliance SHA: 0b852765266e 2025-03-04 11:39:48

Compliance test for mxc-isi device /dev/video0:

Driver Info:
	Driver name      : mxc-isi
	Card type        : mxc-isi-cap
	Bus info         : platform:32e00000.isi
	Driver version   : 6.14.0
	Capabilities     : 0xa4201000
		Video Capture Multiplanar
		I/O MC
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x24201000
		Video Capture Multiplanar
		I/O MC
		Streaming
		Extended Pix Format
Media Driver Info:
	Driver name      : mxc-isi
	Model            : FSL Capture Media Device
	Serial           : 
	Bus info         : platform:32e00000.isi
	Media version    : 6.14.0
	Hardware revision: 0x00000000 (0)
	Driver version   : 6.14.0
Interface Info:
	ID               : 0x0300000c
	Type             : V4L Video
Entity Info:
	ID               : 0x0000000a (10)
	Name             : mxc_isi.0.capture
	Function         : V4L2 I/O
	Pad 0x0100000b   : 0: Sink
	  Link 0x0200000e: from remote pad 0x1000009 of entity 'mxc_isi.0' (Video Pixel Formatter): Data, Enabled, Immutable

Required ioctls:
	test MC information (see 'Media Driver Info' above): OK
	test VIDIOC_QUERYCAP: OK
	test invalid ioctls: OK

Allow for multiple opens:
	test second /dev/video0 open: OK
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
	test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 4 Private Controls: 0

Format ioctls (Input 0):
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK
	test VIDIOC_TRY_FMT: OK
	test VIDIOC_S_FMT: OK
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
	test Scaling: OK

Codec ioctls (Input 0):
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test CREATE_BUFS maximum buffers: OK
	test VIDIOC_REMOVE_BUFS: OK
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)
	test blocking wait: OK

Total for mxc-isi device /dev/video0: 49, Succeeded: 49, Failed: 0, Warnings: 0


Mirela Rabulea (4):
  dt-bindings: media: i2c: Add OX05B1S sensor
  media: ox05b1s: Add omnivision OX05B1S raw sensor driver
  MAINTAINERS: Add entry for OX05B1S sensor driver
  media: ox05b1s: Add support for Omnivision OS08A20 raw sensor

 .../bindings/media/i2c/ovti,ox05b1s.yaml      |  119 ++
 MAINTAINERS                                   |   10 +
 drivers/media/i2c/Kconfig                     |    1 +
 drivers/media/i2c/Makefile                    |    1 +
 drivers/media/i2c/ox05b1s/Kconfig             |   10 +
 drivers/media/i2c/ox05b1s/Makefile            |    2 +
 drivers/media/i2c/ox05b1s/ox05b1s.h           |   26 +
 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c      | 1136 +++++++++++++++++
 drivers/media/i2c/ox05b1s/ox05b1s_modes.c     |  187 +++
 9 files changed, 1492 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
 create mode 100644 drivers/media/i2c/ox05b1s/Kconfig
 create mode 100644 drivers/media/i2c/ox05b1s/Makefile
 create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s.h
 create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
 create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_modes.c

-- 
2.25.1

[PATCH v4 1/4] dt-bindings: media: i2c: Add OX05B1S sensor

[Mirela Rabulea]

Add bindings for Omnivision OX05B1S sensor.
Also add compatible for Omnivision OS08A20 sensor.

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---

Changes in v4:
	Collect Reviewed-by

Changes in v3:
	Use unevaluatedProperties: false and drop orientation/rotation
	Drop items and keep alphabetical order in compatible property
	Shorten the description for reset_gpio
	Make the supplies required.
	Use generic node name (camera instead of ox05b1s)

Changes in v2:
	Small updates on description
	Update subject, drop "bindings" and "driver"
	Just one binding patch (squash os08a20 bindings)
	Re-flow to 80 columns.
	Drop clock name (not needed in case of single clock)
	Make the clock required property, strictly from sensor module point of view, it is mandatory (will use a fixed clock for nxp board)
	Add regulators: avdd, dvdd, dovdd
	Add $ref: /schemas/media/video-interface-devices.yaml
	Drop assigned-clock* properties (defined in clocks.yaml)
	Keep "additionalProperties : false" and orientation/rotation (unevaluatedProperties: false was suggested, but only orientation/rotation are needed from video-interface-devices.yaml)
	Include assigned-clock* in the example, for completeness sake (although it was also suggested to omit them)

 .../bindings/media/i2c/ovti,ox05b1s.yaml      | 119 ++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
new file mode 100644
index 000000000000..9f35b4e67bea
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ox05b1s.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Omnivision OX05B1S Image Sensor
+
+maintainers:
+  - Mirela Rabulea <mirela.rabulea@nxp.com>
+
+description:
+  The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an active
+  array size of 2592 x 1944. It is programmable through I2C interface.
+  Image data is available via MIPI CSI-2 serial data output.
+
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+  compatible:
+    enum:
+      - ovti,os08a20
+      - ovti,ox05b1s
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: Input clock (24 MHz)
+    maxItems: 1
+
+  reset-gpios:
+    description: Active low XSHUTDOWN pin
+    maxItems: 1
+
+  avdd-supply:
+    description: Power for analog circuit (2.8V)
+
+  dovdd-supply:
+    description: Power for I/O circuit (1.8V)
+
+  dvdd-supply:
+    description: Power for digital circuit (1.2V)
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+    description: MIPI CSI-2 transmitter port
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            anyOf:
+              - items:
+                  - const: 1
+                  - const: 2
+              - items:
+                  - const: 1
+                  - const: 2
+                  - const: 3
+                  - const: 4
+        required:
+          - data-lanes
+
+    required:
+      - endpoint
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - port
+  - avdd-supply
+  - dovdd-supply
+  - dvdd-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        camera@36 {
+            compatible = "ovti,ox05b1s";
+            reg = <0x36>;
+            clocks = <&ox05b1s_clk>;
+
+            assigned-clocks = <&ox05b1s_clk>;
+            assigned-clock-parents = <&ox05b1s_clk_parent>;
+            assigned-clock-rates = <24000000>;
+
+            reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
+
+            avdd-supply = <&camera_avdd_2v8>;
+            dovdd-supply = <&camera_dovdd_1v8>;
+            dvdd-supply = <&camera_dvdd_1v2>;
+
+            orientation = <2>;
+            rotation = <0>;
+
+            port {
+                ox05b1s_mipi_0_ep: endpoint {
+                    remote-endpoint = <&mipi_csi0_ep>;
+                    data-lanes = <1 2 3 4>;
+                };
+            };
+        };
+    };
+...
-- 
2.25.1

[PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Mirela Rabulea]

Add a v4l2 subdevice driver for the Omnivision OX05B1S RGB-IR sensor.

The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an
active array size of 2592 x 1944.

The following features are supported for OX05B1S:
- Manual exposure an gain control support
- vblank/hblank control support
- Supported resolution: 2592 x 1944 @ 30fps (SGRBG10)

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
Changes in v4:
	Switch to Y media bus codes. The CFA pattern control will be implemented when patches get merged, or maybe separatelly as RFC?
	Add pixel_rate member to mode struct, remove fps member. We do not have information how to calculate the pixel rate from the PLL parameters that can be made public.
	Use register macros for the registers that are documented. User register group macros, where individual registers are not documented
	Remove some uneeded local variable initialisations
	Fix extra/missing spaces
	Add missing ending \n
	Use return -ENODEV & return 0 to ease reading
	Rename retval to ret in probe for consistency
	Use devm_mutex_init instead of mutex_init
	Replace more dev_err's with dev_err_probe
	Constify more structs
	Remove some unneded ending commas after a terminator
	Fix smatch error in ox05b1s_s_ctrl: error: typename in expression
	Fix a seeries of smatch warnings like: warning: symbol 'ovx5b_init_setting_2592x1944' was not declared. Should it be static?
	Shorten some more lines to 80 columns

Changes in v3:
	Use helpers from v4l2-cci.h (drop ox05b1s_write_reg, ox05b1s_read_reg, ox05b1s_set_hts/vts/exp/analog_gain, ox05b1s_regmap_config)
	Don't hardcode timing registers: remove timing registers x_output_size/y_output_size from register configuration list, add them to ox05b1s_apply_current_mode
	Remove HTS,VTS from register config list as they are written by HBLANK and VBLANK controls through __v4l2_ctrl_handler_setup
	ox05b1s register config cleaning (remove all registers that were at their default value, and more, keep only what seems mandatory to be able to stream)
	Use const for ox05b1s_supported_modes
	Device should be silent on success, use dev_dbg.
	Drop unneeded {}
	Fixed an error introduced in v2 in ox05b1s_nearest_size (set_fmt for 4k BGGR12 mode was stuck)
	Fix an issue in ox05b1s_set_fmt, the format was saved in subdev state only for _TRY, save it also for _ACTIVE

Changes in v2:
	Use dev_err_probe for missing clock, since it is now required property, and use NULL for devm_clk_get (no name for single clock), remove check for non NULL sensor->sensor_clk
	Remove dev_err message for devm_regmap_init_i2c allocation error
	Added spaces inside brackets, wrap lines to 80
	Remove some redundant initializations
	Add regulators
	Make "sizes" a pointer
		Use struct v4l2_area instead of u32[2] array
		Remove the count for supported_modes[] and supported_codes[], instead use sentinel element at the end
		Consequently, update ox05b1s_enum_mbus_code, ox05b1s_enum_frame_size, ox05b1s_nearest_size, ox05b1s_find_code, to not use the count
	Remove .h files for modes, however did not move this code in the driver file but added a separate c file for all supported modes
	Refactor register lists to allow multiple register arrays per mode
	Use GPL-2.0-only instead of GPL-2.0

 drivers/media/i2c/Kconfig                 |   1 +
 drivers/media/i2c/Makefile                |   1 +
 drivers/media/i2c/ox05b1s/Kconfig         |  10 +
 drivers/media/i2c/ox05b1s/Makefile        |   2 +
 drivers/media/i2c/ox05b1s/ox05b1s.h       |  22 +
 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c  | 951 ++++++++++++++++++++++
 drivers/media/i2c/ox05b1s/ox05b1s_modes.c |  77 ++
 7 files changed, 1064 insertions(+)
 create mode 100644 drivers/media/i2c/ox05b1s/Kconfig
 create mode 100644 drivers/media/i2c/ox05b1s/Makefile
 create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s.h
 create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
 create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_modes.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 8ba096b8ebca..5cda062c0463 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -700,6 +700,7 @@ config VIDEO_VGXY61
 
 source "drivers/media/i2c/ccs/Kconfig"
 source "drivers/media/i2c/et8ek8/Kconfig"
+source "drivers/media/i2c/ox05b1s/Kconfig"
 
 endif
 
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index fbb988bd067a..028eb08e648c 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -114,6 +114,7 @@ obj-$(CONFIG_VIDEO_OV9282) += ov9282.o
 obj-$(CONFIG_VIDEO_OV9640) += ov9640.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
 obj-$(CONFIG_VIDEO_OV9734) += ov9734.o
+obj-$(CONFIG_VIDEO_OX05B1S) += ox05b1s/
 obj-$(CONFIG_VIDEO_RDACM20) += rdacm20.o
 obj-$(CONFIG_VIDEO_RDACM21) += rdacm21.o
 obj-$(CONFIG_VIDEO_RJ54N1) += rj54n1cb0c.o
diff --git a/drivers/media/i2c/ox05b1s/Kconfig b/drivers/media/i2c/ox05b1s/Kconfig
new file mode 100644
index 000000000000..48fcd04b20d5
--- /dev/null
+++ b/drivers/media/i2c/ox05b1s/Kconfig
@@ -0,0 +1,10 @@
+config VIDEO_OX05B1S
+	tristate "OmniVision raw sensor support OX05B1S"
+	depends on OF
+	depends on GPIOLIB
+	select REGMAP_I2C
+	help
+	  This is a Video4Linux2 sensor driver for the Omnivision OX05B1S RGB-IR sensor.
+	  This is a 1/2.5-Inch CMOS image sensor with an active array size of 2592 x 1944.
+	  It is programmable through I2C interface.
+	  The output is on MIPI CSI-2 interface.
diff --git a/drivers/media/i2c/ox05b1s/Makefile b/drivers/media/i2c/ox05b1s/Makefile
new file mode 100644
index 000000000000..0b38dbf98bcd
--- /dev/null
+++ b/drivers/media/i2c/ox05b1s/Makefile
@@ -0,0 +1,2 @@
+ox05b1s-objs := ox05b1s_modes.o ox05b1s_mipi.o
+obj-$(CONFIG_VIDEO_OX05B1S) += ox05b1s.o
diff --git a/drivers/media/i2c/ox05b1s/ox05b1s.h b/drivers/media/i2c/ox05b1s/ox05b1s.h
new file mode 100644
index 000000000000..2a87d69864f9
--- /dev/null
+++ b/drivers/media/i2c/ox05b1s/ox05b1s.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024, NXP
+ */
+
+#ifndef OX05B1S_H
+#define OX05B1S_H
+
+#include <linux/types.h>
+
+struct ox05b1s_reg {
+	u32 addr;
+	u32 data;
+};
+
+struct ox05b1s_reglist {
+	const struct ox05b1s_reg *regs;
+};
+
+extern const struct ox05b1s_reglist ox05b1s_reglist_2592x1944[];
+
+#endif /* OX05B1S_H */
diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
new file mode 100644
index 000000000000..1026216ddd5b
--- /dev/null
+++ b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
@@ -0,0 +1,951 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * A V4L2 driver for Omnivision OX05B1S RGB-IR camera.
+ * Copyright (C) 2024, NXP
+ *
+ * Inspired from Sony imx219, imx290, imx214 and imx334 camera drivers
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <media/v4l2-cci.h>
+#include <media/mipi-csi2.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+
+#include "ox05b1s.h"
+
+#define OX05B1S_SENS_PAD_SOURCE	0
+#define OX05B1S_SENS_PADS_NUM	1
+
+#define OX05B1S_REG_SW_STB		CCI_REG8(0x0100)
+#define OX05B1S_REG_SW_RST		CCI_REG8(0x0103)
+#define OX05B1S_REG_CHIP_ID		CCI_REG24(0x300a)
+#define OX05B1S_REG_TIMING_HTS		CCI_REG16(0x380c)
+#define OX05B1S_REG_TIMING_VTS		CCI_REG16(0x380e)
+#define OX05B1S_REG_EXPOSURE		CCI_REG16(0x3501)
+#define OX05B1S_REG_GAIN		CCI_REG16(0x3508)
+#define OX05B1S_REG_X_OUTPUT_SIZE	CCI_REG16(0x3808)
+#define OX05B1S_REG_Y_OUTPUT_SIZE	CCI_REG16(0x380a)
+
+#define client_to_ox05b1s(client)\
+	container_of(i2c_get_clientdata(client), struct ox05b1s, subdev)
+
+struct ox05b1s_sizes {
+	u32	code;
+	const struct v4l2_area *sizes;
+};
+
+struct ox05b1s_plat_data {
+	char				name[20];
+	u32				chip_id;
+	u32				native_width;
+	u32				native_height;
+	u32				active_top;
+	u32				active_left;
+	u32				active_width;
+	u32				active_height;
+	const struct ox05b1s_mode	*supported_modes;
+	u32				default_mode_index;
+	const struct ox05b1s_sizes	*supported_codes;
+};
+
+struct ox05b1s_ctrls {
+	struct v4l2_ctrl_handler handler;
+	struct v4l2_ctrl *link_freq;
+	struct v4l2_ctrl *pixel_rate;
+	struct v4l2_ctrl *hblank;
+	struct v4l2_ctrl *vblank;
+	struct v4l2_ctrl *gain;
+	struct v4l2_ctrl *exposure;
+};
+
+struct ox05b1s_mode {
+	u32 index;
+	u32 width;
+	u32 height;
+	u32 code;
+	u32 bpp;
+	u32 vts; /* default VTS */
+	u32 hts; /* default HTS */
+	u32 exp; /* max exposure */
+	bool h_bin; /* horizontal binning */
+	s64 pixel_rate;
+	const struct ox05b1s_reglist *reg_data;
+};
+
+/* regulator supplies */
+static const char * const ox05b1s_supply_name[] = {
+	"AVDD",  /* Analog voltage supply, 2.8 volts */
+	"DVDD",  /* Digital I/O voltage supply, 1.8 volts */
+	"DOVDD", /* Digital voltage supply, 1.2 volts */
+};
+
+#define OX05B1S_NUM_SUPPLIES ARRAY_SIZE(ox05b1s_supply_name)
+
+struct ox05b1s {
+	struct i2c_client *i2c_client;
+	struct regmap *regmap;
+	struct gpio_desc *rst_gpio;
+	struct regulator_bulk_data supplies[OX05B1S_NUM_SUPPLIES];
+	struct clk *sensor_clk;
+	const struct ox05b1s_plat_data *model;
+	struct v4l2_subdev subdev;
+	struct media_pad pads[OX05B1S_SENS_PADS_NUM];
+	const struct ox05b1s_mode *mode;
+	struct mutex lock; /* sensor lock */
+	u32 stream_status;
+	struct ox05b1s_ctrls ctrls;
+};
+
+#define OX05B1S_PIXEL_RATE_48M	48000000
+static const struct ox05b1s_mode ox05b1s_supported_modes[] = {
+	{
+		/* 5Mp GRBG10, 30fps */
+		.index		= 0,
+		.width		= 2592,
+		.height		= 1944,
+		.code		= MEDIA_BUS_FMT_Y10_1X10,
+		.bpp		= 10,
+		.vts		= 0x850,
+		.hts		= 0x2f0,
+		.exp		= 0x850 - 8,
+		.h_bin		= false,
+		.pixel_rate	= OX05B1S_PIXEL_RATE_48M,
+		.reg_data	= ox05b1s_reglist_2592x1944,
+	}, {
+		/* sentinel */
+	}
+};
+
+/* keep in sync with ox05b1s_supported_modes */
+static const struct v4l2_area ox05b1s_sgrbg10_sizes[] = {
+	{
+		.width = 2592,
+		.height = 1944,
+	}, {
+		/* sentinel */
+	}
+};
+
+static const struct ox05b1s_sizes ox05b1s_supported_codes[] = {
+	{
+		.code = MEDIA_BUS_FMT_Y10_1X10,
+		.sizes = ox05b1s_sgrbg10_sizes,
+	}, {
+		/* sentinel */
+	}
+};
+
+static int ox05b1s_power_on(struct ox05b1s *sensor)
+{
+	struct device *dev = &sensor->i2c_client->dev;
+	int ret;
+
+	ret = regulator_bulk_enable(OX05B1S_NUM_SUPPLIES, sensor->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulators\n");
+		return ret;
+	}
+
+	/* get out of powerdown and reset */
+	gpiod_set_value_cansleep(sensor->rst_gpio, 0);
+
+	ret = clk_prepare_enable(sensor->sensor_clk);
+	if (ret < 0) {
+		dev_err(dev, "Enable sensor clk fail ret=%d\n", ret);
+		goto reg_off;
+	}
+
+	/* with XVCLK@24MHz, t2 = 6ms before first ox05b1s SCCB transaction */
+	fsleep(6000);
+
+	return 0;
+
+reg_off:
+	regulator_bulk_disable(OX05B1S_NUM_SUPPLIES, sensor->supplies);
+
+	return ret;
+}
+
+static int ox05b1s_power_off(struct ox05b1s *sensor)
+{
+	gpiod_set_value_cansleep(sensor->rst_gpio, 1);
+
+	/* XVCLK must be active for 512 cycles after last SCCB transaction */
+	fsleep(350); /* 512 cycles = 0.34 ms at 24MHz */
+	clk_disable_unprepare(sensor->sensor_clk);
+
+	regulator_bulk_disable(OX05B1S_NUM_SUPPLIES, sensor->supplies);
+
+	return 0;
+}
+
+static int ox05b1s_runtime_suspend(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+
+	return ox05b1s_power_off(sensor);
+}
+
+static int ox05b1s_runtime_resume(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+
+	return ox05b1s_power_on(sensor);
+}
+
+#define OX05B1S_MAX_REG_BULK 16
+static int ox05b1s_write_reg_array(struct ox05b1s *sensor,
+				   const struct ox05b1s_reg *reg_array)
+{
+	struct device *dev = &sensor->i2c_client->dev;
+	const struct ox05b1s_reg *table = reg_array;
+	u8 vals[OX05B1S_MAX_REG_BULK];
+	int i;
+	int ret;
+
+	while (table->addr) {
+		for (i = 0; i < OX05B1S_MAX_REG_BULK; i++) {
+			if (table[i].addr != (table[0].addr + i))
+				break;
+			vals[i] = table[i].data;
+		}
+		ret = regmap_bulk_write(sensor->regmap, table->addr, vals, i);
+		if (ret) {
+			dev_err(dev, "Failed to write reg addr=%x, count %d\n",
+				table->addr, i);
+			return ret;
+		}
+		table += i;
+	}
+
+	return 0;
+}
+
+static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
+{
+	return &container_of(ctrl->handler, struct ox05b1s,
+			     ctrls.handler)->subdev;
+}
+
+static int ox05b1s_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+	u32 w = sensor->mode->width;
+	u32 h = sensor->mode->height;
+	int ret = 0;
+	u32 hts;
+
+	/* apply V4L2 controls values only if power is already up */
+	if (!pm_runtime_get_if_in_use(&client->dev))
+		return 0;
+
+	/* s_ctrl holds sensor lock */
+	switch (ctrl->id) {
+	case V4L2_CID_VBLANK:
+		ret = cci_write(sensor->regmap, OX05B1S_REG_TIMING_VTS,
+				h + ctrl->val, NULL);
+		break;
+	case V4L2_CID_HBLANK:
+		hts = (sensor->mode->h_bin) ?
+			  w + ctrl->val : (w + ctrl->val) / 2;
+		ret = cci_write(sensor->regmap, OX05B1S_REG_TIMING_HTS,
+				hts, NULL);
+		break;
+	case V4L2_CID_PIXEL_RATE:
+		/* Read-only, but we adjust it based on mode. */
+		break;
+	case V4L2_CID_ANALOGUE_GAIN:
+		ret = cci_write(sensor->regmap, OX05B1S_REG_GAIN,
+				ctrl->val, NULL);
+		break;
+	case V4L2_CID_EXPOSURE:
+		ret = cci_write(sensor->regmap, OX05B1S_REG_EXPOSURE,
+				ctrl->val, NULL);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	pm_runtime_put(&client->dev);
+
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops ox05b1s_ctrl_ops = {
+	.s_ctrl = ox05b1s_s_ctrl,
+};
+
+/*
+ * MIPI CSI-2 link frequencies.
+ * link_freq = (pixel_rate * bpp) / (2 * data_lanes)
+ */
+static const s64 ox05b1s_csi2_link_freqs[] = {
+	200000000
+};
+
+/* Link freq for default mode: 1080p RAW10, 4 data lanes 800 Mbps/lane. */
+#define OX05B1S_DEFAULT_LINK_FREQ	0
+
+static int ox05b1s_init_controls(struct ox05b1s *sensor)
+{
+	const struct v4l2_ctrl_ops *ops = &ox05b1s_ctrl_ops;
+	struct ox05b1s_ctrls *ctrls = &sensor->ctrls;
+	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
+	struct device *dev = &sensor->i2c_client->dev;
+	struct v4l2_fwnode_device_properties props;
+	int ret;
+
+	v4l2_ctrl_handler_init(hdl, 7);
+
+	/* we can use our own mutex for the ctrl lock */
+	hdl->lock = &sensor->lock;
+
+	/* Clock related controls */
+	ctrls->link_freq = v4l2_ctrl_new_int_menu(hdl, ops,
+						  V4L2_CID_LINK_FREQ,
+						  ARRAY_SIZE(ox05b1s_csi2_link_freqs) - 1,
+						  OX05B1S_DEFAULT_LINK_FREQ,
+						  ox05b1s_csi2_link_freqs);
+
+	/* mode dependent, actual range set in ox05b1s_update_controls */
+	ctrls->pixel_rate = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE,
+					      0, 0, 1, 0);
+
+	ctrls->hblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK,
+					  0, 0, 1, 0);
+
+	ctrls->vblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK,
+					  0, 0, 1, 0);
+
+	ctrls->exposure = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE,
+					    0, 0, 1, 0);
+
+	ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN,
+					0, 0xFFFF, 1, 0x80);
+
+	if (hdl->error) {
+		ret = hdl->error;
+		goto free_ctrls;
+	}
+
+	ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	ret = v4l2_fwnode_device_parse(dev, &props);
+	if (ret)
+		goto free_ctrls;
+
+	ret = v4l2_ctrl_new_fwnode_properties(hdl, ops, &props);
+	if (ret)
+		goto free_ctrls;
+
+	sensor->subdev.ctrl_handler = hdl;
+	return 0;
+
+free_ctrls:
+	dev_err(dev, "Failed to init controls\n");
+	v4l2_ctrl_handler_free(hdl);
+	return ret;
+}
+
+static int ox05b1s_apply_current_mode(struct ox05b1s *sensor);
+
+static int ox05b1s_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+	int ret;
+
+	if (enable) {
+		ret = pm_runtime_resume_and_get(&client->dev);
+		if (ret < 0)
+			return ret;
+		ret = ox05b1s_apply_current_mode(sensor);
+		if (!ret)
+			ret = cci_write(sensor->regmap, OX05B1S_REG_SW_STB,
+					0x01, NULL);
+	} else {
+		ret = cci_write(sensor->regmap, OX05B1S_REG_SW_STB, 0x00, NULL);
+	}
+
+	sensor->stream_status = enable;
+
+	if (!enable || ret) {
+		pm_runtime_mark_last_busy(&sensor->i2c_client->dev);
+		pm_runtime_put_autosuspend(&client->dev);
+	}
+
+	return 0;
+}
+
+static void ox05b1s_update_pad_format(struct ox05b1s *sensor,
+				      const struct ox05b1s_mode *mode,
+				      struct v4l2_mbus_framefmt *fmt)
+{
+	fmt->code = mode->code;
+	fmt->width = mode->width;
+	fmt->height = mode->height;
+	fmt->field = V4L2_FIELD_NONE;
+	fmt->colorspace = V4L2_COLORSPACE_RAW;
+	fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+	fmt->xfer_func = V4L2_XFER_FUNC_NONE;
+}
+
+static int ox05b1s_init_state(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *state)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+	struct v4l2_mbus_framefmt *format;
+
+	/* Initialize the format. */
+	format = v4l2_subdev_state_get_format(state, 0);
+	ox05b1s_update_pad_format(sensor, &sensor->model->supported_modes[0],
+				  format);
+
+	return 0;
+}
+
+static int ox05b1s_enum_mbus_code(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state,
+				  struct v4l2_subdev_mbus_code_enum *code)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+	const struct ox05b1s_sizes *supported_codes = sensor->model->supported_codes;
+	int i = 0;
+
+	while (i++ < code->index && supported_codes->code)
+		supported_codes++;
+	if (!supported_codes->code) /* code->index outside supported_codes[] */
+		return -EINVAL;
+
+	code->code = supported_codes->code;
+
+	return 0;
+}
+
+static int ox05b1s_enum_frame_size(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *sd_state,
+				   struct v4l2_subdev_frame_size_enum *fse)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+	const struct ox05b1s_sizes *supported_codes = sensor->model->supported_codes;
+	const struct v4l2_area *sizes;
+	int i = 0;
+
+	if (fse->pad != 0)
+		return -EINVAL;
+
+	while (supported_codes->code) {
+		if (supported_codes->code == fse->code)
+			break;
+		supported_codes++;
+	}
+
+	if (!supported_codes->code) /* fse->code not in supported_codes[] */
+		return -EINVAL;
+
+	sizes = supported_codes->sizes;
+	while (i++ < fse->index && sizes->width)
+		sizes++;
+	if (!sizes->width) /* fse->index outside sizes[] */
+		return -EINVAL;
+
+	fse->min_width = sizes->width;
+	fse->max_width = fse->min_width;
+	fse->min_height = sizes->height;
+	fse->max_height = fse->min_height;
+
+	return 0;
+}
+
+/* Update control ranges based on current streaming mode, needs sensor lock */
+static int ox05b1s_update_controls(struct ox05b1s *sensor)
+{
+	int ret;
+	struct device *dev = &sensor->i2c_client->dev;
+	u32 hts = sensor->mode->hts;
+	u32 hblank;
+	u32 vts = sensor->mode->vts;
+	u32 vblank = vts - sensor->mode->height;
+	u64 pixel_rate = sensor->mode->pixel_rate;
+	u32 min_exp = 8;
+	u32 max_exp = vts - 8;
+
+	ret = __v4l2_ctrl_modify_range(sensor->ctrls.pixel_rate, pixel_rate,
+				       pixel_rate, 1, pixel_rate);
+	if (ret) {
+		dev_err(dev, "Modify range for pixel_rate %llu-%llu failed\n",
+			pixel_rate, pixel_rate);
+		goto out;
+	}
+
+	if (sensor->mode->h_bin)
+		hblank = hts - sensor->mode->width;
+	else
+		hblank = 2 * hts - sensor->mode->width;
+
+	ret = __v4l2_ctrl_modify_range(sensor->ctrls.hblank, hblank, hblank,
+				       1, hblank);
+	if (ret) {
+		dev_err(dev, "Modify range for hblank %u-%u failed\n",
+			hblank, hblank);
+		goto out;
+	}
+	__v4l2_ctrl_s_ctrl(sensor->ctrls.hblank,
+			   sensor->ctrls.hblank->default_value);
+
+	ret = __v4l2_ctrl_modify_range(sensor->ctrls.vblank, 0, vblank * 4,
+				       1, vblank);
+	if (ret) {
+		dev_err(dev, "Modify range for vblank %u-%u failed\n",
+			vblank, vblank);
+		goto out;
+	}
+	__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank,
+			   sensor->ctrls.vblank->default_value);
+
+	ret = __v4l2_ctrl_modify_range(sensor->ctrls.exposure, min_exp, max_exp,
+				       1, max_exp / 2);
+	if (ret) {
+		dev_err(dev, "Modify range for exposure %u-%u failed\n",
+			min_exp, max_exp);
+		goto out;
+	}
+	__v4l2_ctrl_s_ctrl(sensor->ctrls.exposure,
+			   sensor->ctrls.exposure->default_value);
+
+out:
+	return ret;
+}
+
+/* needs sensor lock and power on */
+static int ox05b1s_apply_current_mode(struct ox05b1s *sensor)
+{
+	struct device *dev = &sensor->i2c_client->dev;
+	const struct ox05b1s_reglist *reg_data = sensor->mode->reg_data;
+	u32 w = sensor->mode->width;
+	u32 h = sensor->mode->height;
+	int ret;
+
+	cci_write(sensor->regmap, OX05B1S_REG_SW_RST, 0x01, &ret);
+
+	while (reg_data->regs) {
+		ret = ox05b1s_write_reg_array(sensor, reg_data->regs);
+		if (ret)
+			goto out;
+		reg_data++;
+	}
+
+	cci_write(sensor->regmap, OX05B1S_REG_X_OUTPUT_SIZE, w, &ret);
+	cci_write(sensor->regmap, OX05B1S_REG_Y_OUTPUT_SIZE, h, &ret);
+	if (ret)
+		goto out;
+
+	/* setup handler will write actual controls into sensor registers */
+	ret = __v4l2_ctrl_handler_setup(&sensor->ctrls.handler);
+
+out:
+	if (ret < 0)
+		dev_err(dev, "Failed to apply mode %dx%d,bpp=%d\n", w, h,
+			sensor->mode->bpp);
+
+	return ret;
+}
+
+/* similar with v4l2_find_nearest_size but filter for mbus code, needs sensor lock */
+static const struct ox05b1s_mode *ox05b1s_nearest_size(const struct ox05b1s_mode *supported_modes,
+						       struct v4l2_subdev_format *fmt)
+{
+	u32 err, min_error = U32_MAX;
+	const struct ox05b1s_mode *best = NULL;
+
+	if (!supported_modes)
+		return NULL;
+
+	for (; supported_modes->width; supported_modes++) {
+		const u32 w = supported_modes->width;
+		const u32 h = supported_modes->height;
+
+		if (supported_modes->code != fmt->format.code)
+			continue;
+
+		err = abs(w - fmt->format.width) + abs(h - fmt->format.height);
+		if (err > min_error)
+			continue;
+
+		min_error = err;
+		best = supported_modes;
+		if (!err)
+			break;
+	}
+
+	return best;
+}
+
+/* get a valid mbus code, either the requested one or the default one */
+static u32 ox05b1s_find_code(const struct ox05b1s_plat_data *model, u32 code)
+{
+	u32 found_code = 0;
+	const struct ox05b1s_sizes *supported_codes = model->supported_codes;
+
+	while (supported_codes->code) {
+		if (supported_codes->code == code) {
+			found_code = code;
+			break;
+		}
+		supported_codes++;
+	}
+
+	if (!supported_codes->code) /* code not in supported_codes[] */
+		found_code = supported_codes[model->default_mode_index].code;
+
+	return found_code;
+}
+
+static int ox05b1s_set_fmt(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_state *state,
+			   struct v4l2_subdev_format *fmt)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+	struct device *dev = &sensor->i2c_client->dev;
+	struct v4l2_mbus_framefmt *format;
+	const struct ox05b1s_mode *mode;
+
+	/* if no matching mbus code found, use the one from the default mode */
+	fmt->format.code = ox05b1s_find_code(sensor->model, fmt->format.code);
+	mode = ox05b1s_nearest_size(sensor->model->supported_modes, fmt);
+
+	fmt->format.width = mode->width;
+	fmt->format.height = mode->height;
+	fmt->format.field = V4L2_FIELD_NONE;
+
+	format = v4l2_subdev_state_get_format(state, 0);
+	*format = fmt->format;
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
+		return 0;
+
+	sensor->mode = mode;
+
+	/* update controls that depend on current mode */
+	ox05b1s_update_controls(sensor);
+
+	dev_dbg(dev, "Set mode index=%d, %d x %d, code=0x%x\n",
+		sensor->mode->index,
+		fmt->format.width, fmt->format.height, fmt->format.code);
+
+	return 0;
+}
+
+static u8 ox05b1s_code2dt(const u32 code)
+{
+	switch (code) {
+	case MEDIA_BUS_FMT_Y10_1X10:
+		return MIPI_CSI2_DT_RAW10;
+	default:
+		return MIPI_CSI2_DT_RAW10;
+	}
+}
+
+static int ox05b1s_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				  struct v4l2_mbus_frame_desc *fd)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+
+	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+	fd->num_entries = 1;
+
+	/* get sensor current code */
+	mutex_lock(&sensor->lock);
+	fd->entry[0].pixelcode = sensor->mode->code;
+	mutex_unlock(&sensor->lock);
+
+	fd->entry[0].bus.csi2.vc = 0;
+	fd->entry[0].bus.csi2.dt = ox05b1s_code2dt(fd->entry[0].pixelcode);
+
+	return 0;
+}
+
+static int ox05b1s_get_selection(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *sd_state,
+				 struct v4l2_subdev_selection *sel)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+
+	switch (sel->target) {
+	case V4L2_SEL_TGT_NATIVE_SIZE:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = sensor->model->native_width;
+		sel->r.height = sensor->model->native_height;
+		return 0;
+	case V4L2_SEL_TGT_CROP:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		sel->r.top = sensor->model->active_top;
+		sel->r.left = sensor->model->active_left;
+		sel->r.width = sensor->model->active_width;
+		sel->r.height = sensor->model->active_height;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static const struct v4l2_subdev_video_ops ox05b1s_subdev_video_ops = {
+	.s_stream = ox05b1s_s_stream
+};
+
+static const struct v4l2_subdev_pad_ops ox05b1s_subdev_pad_ops = {
+	.set_fmt		= ox05b1s_set_fmt,
+	.get_fmt		= v4l2_subdev_get_fmt,
+	.get_frame_desc		= ox05b1s_get_frame_desc,
+	.enum_mbus_code		= ox05b1s_enum_mbus_code,
+	.enum_frame_size	= ox05b1s_enum_frame_size,
+	.get_selection		= ox05b1s_get_selection
+};
+
+static const struct v4l2_subdev_ops ox05b1s_subdev_ops = {
+	.video = &ox05b1s_subdev_video_ops,
+	.pad   = &ox05b1s_subdev_pad_ops
+};
+
+static const struct v4l2_subdev_internal_ops ox05b1s_internal_ops = {
+	.init_state = ox05b1s_init_state
+};
+
+static void ox05b1s_get_gpios(struct ox05b1s *sensor)
+{
+	struct device *dev = &sensor->i2c_client->dev;
+
+	sensor->rst_gpio = devm_gpiod_get_optional(dev, "reset",
+						   GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->rst_gpio))
+		dev_warn(dev, "No sensor reset pin available\n");
+}
+
+static int ox05b1s_get_regulators(struct ox05b1s *sensor)
+{
+	struct device *dev = &sensor->i2c_client->dev;
+	unsigned int i;
+
+	for (i = 0; i < OX05B1S_NUM_SUPPLIES; i++)
+		sensor->supplies[i].supply = ox05b1s_supply_name[i];
+
+	return devm_regulator_bulk_get(dev, OX05B1S_NUM_SUPPLIES,
+				       sensor->supplies);
+}
+
+static int ox05b1s_read_chip_id(struct ox05b1s *sensor)
+{
+	struct device *dev = &sensor->i2c_client->dev;
+	u64 chip_id;
+	char *camera_name;
+	int ret;
+
+	ret = cci_read(sensor->regmap, OX05B1S_REG_CHIP_ID, &chip_id, NULL);
+	if (ret) {
+		dev_err(dev, "Camera chip_id read error\n");
+		return -ENODEV;
+	}
+
+	switch (chip_id) {
+	case 0x580542:
+		camera_name = "ox05b1s";
+		break;
+	default:
+		camera_name = "unknown";
+		break;
+	}
+
+	if (chip_id == sensor->model->chip_id) {
+		dev_dbg(dev, "Camera %s detected, chip_id=%llx\n",
+			camera_name, chip_id);
+	} else {
+		dev_err(dev, "Detected %s camera (chip_id=%llx), but expected %s (chip_id=%x)\n",
+			camera_name, chip_id,
+			sensor->model->name, sensor->model->chip_id);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int ox05b1s_probe(struct i2c_client *client)
+{
+	int ret;
+	struct device *dev = &client->dev;
+	struct v4l2_subdev *sd;
+	struct ox05b1s *sensor;
+
+	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
+	if (!sensor)
+		return -ENOMEM;
+
+	sensor->regmap = devm_cci_regmap_init_i2c(client, 16);
+	if (IS_ERR(sensor->regmap))
+		return PTR_ERR(sensor->regmap);
+
+	sensor->i2c_client = client;
+
+	sensor->model = of_device_get_match_data(dev);
+
+	ox05b1s_get_gpios(sensor);
+
+	/* Get system clock, xvclk */
+	sensor->sensor_clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(sensor->sensor_clk))
+		return dev_err_probe(dev, PTR_ERR(sensor->sensor_clk),
+				     "Failed to get xvclk\n");
+
+	ret = ox05b1s_get_regulators(sensor);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+	sd = &sensor->subdev;
+	v4l2_i2c_subdev_init(sd, client, &ox05b1s_subdev_ops);
+	sd->internal_ops = &ox05b1s_internal_ops;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->dev = &client->dev;
+	sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
+	sensor->pads[OX05B1S_SENS_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
+	ret = media_entity_pads_init(&sd->entity, OX05B1S_SENS_PADS_NUM,
+				     sensor->pads);
+	if (ret)
+		goto probe_out;
+
+	devm_mutex_init(dev, &sensor->lock);
+
+	ret = ox05b1s_init_controls(sensor);
+	if (ret)
+		goto probe_err_entity_cleanup;
+
+	/* power on manually */
+	ret = ox05b1s_power_on(sensor);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to power on\n");
+		goto probe_err_free_ctrls;
+	}
+
+	pm_runtime_set_active(dev);
+	pm_runtime_get_noresume(dev);
+	pm_runtime_enable(dev);
+
+	ret = ox05b1s_read_chip_id(sensor);
+	if (ret)
+		goto probe_err_pm_runtime;
+
+	v4l2_i2c_subdev_set_name(sd, client, sensor->model->name, NULL);
+
+	/* Centrally managed subdev active state */
+	sd->state_lock = &sensor->lock;
+	ret = v4l2_subdev_init_finalize(sd);
+	if (ret < 0) {
+		dev_err_probe(dev, ret, "Subdev init error: %d\n", ret);
+		goto probe_err_pm_runtime;
+	}
+
+	ret = v4l2_async_register_subdev_sensor(sd);
+	if (ret < 0) {
+		dev_err_probe(&client->dev, ret,
+			      "Async register failed, ret=%d\n", ret);
+		goto probe_err_subdev_cleanup;
+	}
+
+	sensor->mode = &sensor->model->supported_modes[0];
+	ox05b1s_update_controls(sensor);
+
+	pm_runtime_set_autosuspend_delay(dev, 1000);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
+
+probe_err_subdev_cleanup:
+	v4l2_subdev_cleanup(sd);
+probe_err_pm_runtime:
+	pm_runtime_put_noidle(dev);
+	pm_runtime_disable(dev);
+	ox05b1s_runtime_suspend(dev);
+probe_err_free_ctrls:
+	v4l2_ctrl_handler_free(&sensor->ctrls.handler);
+probe_err_entity_cleanup:
+	media_entity_cleanup(&sd->entity);
+probe_out:
+	return ret;
+}
+
+static void ox05b1s_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct ox05b1s *sensor = client_to_ox05b1s(client);
+	struct device *dev = &client->dev;
+
+	pm_runtime_disable(dev);
+	if (!pm_runtime_status_suspended(dev))
+		ox05b1s_runtime_suspend(dev);
+	pm_runtime_set_suspended(dev);
+	v4l2_async_unregister_subdev(sd);
+	v4l2_subdev_cleanup(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(&sensor->ctrls.handler);
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(ox05b1s_pm_ops, ox05b1s_runtime_suspend,
+				 ox05b1s_runtime_resume, NULL);
+
+static const struct ox05b1s_plat_data ox05b1s_data = {
+	.name			= "ox05b1s",
+	.chip_id		= 0x580542,
+	.native_width		= 2608, /* 8 dummy + 2592 active + 8 dummy */
+	.native_height		= 1960, /* 8 dummy + 1944 active + 8 dummy */
+	.active_top		= 8,
+	.active_left		= 8,
+	.active_width		= 2592,
+	.active_height		= 1944,
+	.supported_modes	= ox05b1s_supported_modes,
+	.default_mode_index	= 0,
+	.supported_codes	= ox05b1s_supported_codes
+};
+
+static const struct of_device_id ox05b1s_of_match[] = {
+	{
+		.compatible = "ovti,ox05b1s",
+		.data = &ox05b1s_data,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ox05b1s_of_match);
+
+static struct i2c_driver ox05b1s_i2c_driver = {
+	.driver = {
+		.name = "ox05b1s",
+		.pm = pm_ptr(&ox05b1s_pm_ops),
+		.of_match_table	= ox05b1s_of_match,
+	},
+	.probe	= ox05b1s_probe,
+	.remove = ox05b1s_remove
+};
+
+module_i2c_driver(ox05b1s_i2c_driver);
+MODULE_DESCRIPTION("Omnivision OX05B1S MIPI Camera Subdev Driver");
+MODULE_AUTHOR("Mirela Rabulea <mirela.rabulea@nxp.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_modes.c b/drivers/media/i2c/ox05b1s/ox05b1s_modes.c
new file mode 100644
index 000000000000..9a1f3a89077c
--- /dev/null
+++ b/drivers/media/i2c/ox05b1s/ox05b1s_modes.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Register configurations for all sensor supported modes
+ * Copyright (C) 2024, NXP
+ * Copyright (C) 2024, Omnivision
+ * Copyright (C) 2024, Verisilicon
+ *
+ */
+
+#include <media/v4l2-cci.h>
+#include "ox05b1s.h"
+
+#define OX05B1S_REG_PLL1_CTRL_REG07	CCI_REG8(0x0307)
+#define OX05B1S_REG_PLL3_CTRL_REG4A	CCI_REG8(0x034a)
+#define OX05B1S_REG_PLL_MONITOR_REG0B	CCI_REG8(0x040b)
+#define OX05B1S_REG_PLL_MONITOR_REG0C	CCI_REG8(0x040c)
+#define OX05B1S_REG_SC_CMMN_REG09	CCI_REG8(0x3009)
+#define OX05B1S_REG_GROUP_HLD_REG19	CCI_REG8(0x3219)
+#define OX05B1S_REG_ANA_REG		CCI_REG8(0x3600)
+#define OX05B1S_REG_SENSOR_CTRL02	CCI_REG8(0x3702)
+#define OX05B1S_REG_TIMING_CTRL	CCI_REG8(0x3800)
+#define OX05B1S_REG_MIPI_CORE_REG02	CCI_REG8(0x4802)
+#define OX05B1S_REG_MIPI_CORE_REG1B	CCI_REG8(0x481b)
+#define OX05B1S_REG_PCLK_PERIOD		CCI_REG8(0x4837)
+
+/*
+ * Register configuration for Omnivision OX05B1S raw camera
+ * 2592X1944_30FPS_FULL_RGBIr 2592 1944
+ */
+static const struct ox05b1s_reg ovx5b_init_setting_2592x1944[] = {
+	{ 0x0107, 0x01 }, /* Reserved */
+	{ OX05B1S_REG_PLL1_CTRL_REG07, 0x02 },
+	{ OX05B1S_REG_PLL3_CTRL_REG4A, 0x05 },
+	{ OX05B1S_REG_PLL_MONITOR_REG0B, 0x5c },
+	{ OX05B1S_REG_PLL_MONITOR_REG0C, 0xcd },
+	{ OX05B1S_REG_SC_CMMN_REG09, 0x2e },
+	{ OX05B1S_REG_GROUP_HLD_REG19, 0x08 },
+	{ OX05B1S_REG_ANA_REG + 0x84, 0x6d },
+	{ OX05B1S_REG_ANA_REG + 0x85, 0x6d },
+	{ OX05B1S_REG_ANA_REG + 0x86, 0x6d },
+	{ OX05B1S_REG_ANA_REG + 0x87, 0x6d },
+	{ OX05B1S_REG_ANA_REG + 0x8c, 0x07 },
+	{ OX05B1S_REG_ANA_REG + 0x8d, 0x07 },
+	{ OX05B1S_REG_ANA_REG + 0x8e, 0x07 },
+	{ OX05B1S_REG_ANA_REG + 0x8f, 0x00 },
+	{ OX05B1S_REG_ANA_REG + 0x90, 0x04 },
+	{ OX05B1S_REG_ANA_REG + 0x91, 0x04 },
+	{ OX05B1S_REG_ANA_REG + 0x92, 0x04 },
+	{ OX05B1S_REG_ANA_REG + 0x93, 0x04 },
+	{ OX05B1S_REG_ANA_REG + 0x98, 0x00 },
+	{ OX05B1S_REG_ANA_REG + 0xa0, 0x05 },
+	{ OX05B1S_REG_ANA_REG + 0xa2, 0x16 },
+	{ OX05B1S_REG_ANA_REG + 0xa3, 0x03 },
+	{ OX05B1S_REG_ANA_REG + 0xa4, 0x07 },
+	{ OX05B1S_REG_ANA_REG + 0xa5, 0x24 },
+	{ OX05B1S_REG_ANA_REG + 0xe3, 0x09 },
+	{ OX05B1S_REG_SENSOR_CTRL02, 0x0a },
+	{ OX05B1S_REG_TIMING_CTRL + 0x21, 0x04 }, /* mirror */
+	{ OX05B1S_REG_TIMING_CTRL + 0x22, 0x10 },
+	{ OX05B1S_REG_TIMING_CTRL + 0x2b, 0x03 },
+	{ OX05B1S_REG_TIMING_CTRL + 0x66, 0x10 },
+	{ OX05B1S_REG_TIMING_CTRL + 0x6c, 0x46 },
+	{ OX05B1S_REG_TIMING_CTRL + 0x6d, 0x08 },
+	{ OX05B1S_REG_TIMING_CTRL + 0x6e, 0x7b },
+	{ OX05B1S_REG_MIPI_CORE_REG02, 0x00 },
+	{ OX05B1S_REG_MIPI_CORE_REG1B, 0x3c },
+	{ OX05B1S_REG_PCLK_PERIOD, 0x19 },
+	{ /* sentinel*/ }
+};
+
+const struct ox05b1s_reglist ox05b1s_reglist_2592x1944[] = {
+	{
+		.regs = ovx5b_init_setting_2592x1944
+	}, {
+		/* sentinel */
+	}
+};
-- 
2.25.1

[PATCH v4 3/4] MAINTAINERS: Add entry for OX05B1S sensor driver

[Mirela Rabulea]

Add maintainer for Omnivision OX05B1S sensor driver.

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
Changes in v4:
	None

Changes in v3:
	None

Changes in v2:
	None

 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3c10a18fac78..a9d0fa44403a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17635,6 +17635,16 @@ S:	Maintained
 T:	git git://linuxtv.org/media.git
 F:	drivers/media/i2c/ov9734.c
 
+OMNIVISION OX05B1S SENSOR DRIVER
+M:	Mirela Rabulea <mirela.rabulea@nxp.com>
+R:	Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
+R:	Robert Chiras <robert.chiras@oss.nxp.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
+F:	drivers/media/i2c/ox05b1s/*
+
 ONBOARD USB HUB DRIVER
 M:	Matthias Kaehlcke <mka@chromium.org>
 L:	linux-usb@vger.kernel.org
-- 
2.25.1

[PATCH v4 4/4] media: ox05b1s: Add support for Omnivision OS08A20 raw sensor

[Mirela Rabulea]

This is an 8 megapixel raw10/raw12 sensor with HDR capabilities.
HDR mode control is supported, with one HDR mode: staggered HDR
with 2 exposures on separate virtual channels. However, for now,
only one exposure (VC 0) is accessible via get_frame_desc.

Supported resolutions:
   - 1920 x 1080 @ 60fps (SBGGR10, no HDR)
   - 1920 x 1080 @ 30fps (SBGGR10, HDR)
   - 3840 x 2160 @ 30fps (SBGGR12, no HDR)
   - 3840 x 2160 @ 15fps (SBGGR12, HDR)
   - 3840 x 2160 @ 30fps (SBGGR10, no HDR)
   - 3840 x 2160 @ 15fps (SBGGR10, HDR)

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
Changes in v4:
	Switch to Y media bus codes. The CFA pattern control will be implemented when patches get merged, or maybe separatelly as RFC?
	Add pixel_rate member to mode struct, remove fps member. We do not have information how to calculate the pixel rate from the PLL parameters that can be made public.
	Use register macros for the registers that are documented. User register group macros, where individual registers are not documented
	Constify more structs
	Remove some unneded ending commas after a terminator
	Fix a seeries of smatch warnings like: warning: symbol 'os08a20_init_setting_common' was not declared. Should it be static?
	Shorten some more lines to 80 columns

Changes in v3:
	Don't hardcode timing registers: remove timing registers x_output_size/y_output_size from register configuration list, add them to ox05b1s_apply_current_mode
	Remove HTS,VTS from register config list as they are written by HBLANK and VBLANK controls through __v4l2_ctrl_handler_setup
	Use const for os08a20_supported_modes	
	os08a20 register config cleaning (remove all registers that were at their default value, and more, keep only what seems mandatory to be able to stream)
	Let the 4k 10bit mode by default without hdr, all 3 modes are now by default without hdr, staggered hdr may be enabled via v4l2-ctl for any of them.
	Separate the 10/12 bit register settings into separate lists: os08a20_init_setting_10bit, os08a20_init_setting_12bit
	Update commit description: rearrange supported resolutions and remove 1 duplicate line, state HDR limitation
	Increase a bit the default vts for 1080p, to get exactly 60fps, it was 62.61
	Use regmap_update_bits() directly and remove ox05b1s_regmap_update_bits()

Changes in v2:
	Add spaces inside brackets, wrap lines to 80
	Remove some redundant initialization
	Use a loop in os08a20_enable_staggered_hdr/os08a20_disable_staggered_hdr, for that, add a register settings array for HDR enabling/disabling
	Make "sizes" a pointer
	Remove mode headers, add supported modes in the dedicated c file, ox05b1s_modes.c 
	Refactor register lists, for os08a20 use a common list for all modes, and also specific lists per mode

 drivers/media/i2c/ox05b1s/ox05b1s.h       |   4 +
 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c  | 187 +++++++++++++++++++++-
 drivers/media/i2c/ox05b1s/ox05b1s_modes.c | 110 +++++++++++++
 3 files changed, 300 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ox05b1s/ox05b1s.h b/drivers/media/i2c/ox05b1s/ox05b1s.h
index 2a87d69864f9..58454e10150b 100644
--- a/drivers/media/i2c/ox05b1s/ox05b1s.h
+++ b/drivers/media/i2c/ox05b1s/ox05b1s.h
@@ -17,6 +17,10 @@ struct ox05b1s_reglist {
 	const struct ox05b1s_reg *regs;
 };
 
+extern const struct ox05b1s_reglist os08a20_reglist_4k_10b[];
+extern const struct ox05b1s_reglist os08a20_reglist_4k_12b[];
+extern const struct ox05b1s_reglist os08a20_reglist_1080p_10b[];
+
 extern const struct ox05b1s_reglist ox05b1s_reglist_2592x1944[];
 
 #endif /* OX05B1S_H */
diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
index 1026216ddd5b..bc578fc61f1f 100644
--- a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
+++ b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
@@ -40,6 +40,7 @@ struct ox05b1s_sizes {
 	const struct v4l2_area *sizes;
 };
 
+struct ox05b1s;
 struct ox05b1s_plat_data {
 	char				name[20];
 	u32				chip_id;
@@ -52,6 +53,9 @@ struct ox05b1s_plat_data {
 	const struct ox05b1s_mode	*supported_modes;
 	u32				default_mode_index;
 	const struct ox05b1s_sizes	*supported_codes;
+	const char * const		*hdr_modes;
+	u32				hdr_modes_count;
+	int (*set_hdr_mode)(struct ox05b1s *sensor, u32 hdr_mode);
 };
 
 struct ox05b1s_ctrls {
@@ -62,6 +66,7 @@ struct ox05b1s_ctrls {
 	struct v4l2_ctrl *vblank;
 	struct v4l2_ctrl *gain;
 	struct v4l2_ctrl *exposure;
+	struct v4l2_ctrl *hdr_mode;
 };
 
 struct ox05b1s_mode {
@@ -102,6 +107,87 @@ struct ox05b1s {
 	struct ox05b1s_ctrls ctrls;
 };
 
+#define OS08A20_PIXEL_RATE_144M	144000000
+#define OS08A20_PIXEL_RATE_288M	288000000
+static const struct ox05b1s_mode os08a20_supported_modes[] = {
+	{
+		/* 1080p BGGR10, no hdr, 60fps */
+		.index		= 0,
+		.width		= 1920,
+		.height		= 1080,
+		.code		= MEDIA_BUS_FMT_Y10_1X10,
+		.bpp		= 10,
+		.vts		= 0x4d3,
+		.hts		= 0x790,
+		.exp		= 0x4d3 - 8,
+		.h_bin		= true,
+		.pixel_rate	= OS08A20_PIXEL_RATE_144M,
+		.reg_data	= os08a20_reglist_1080p_10b,
+	}, {
+		/* 4k BGGR10, no hdr, 30fps */
+		.index		= 1,
+		.width		= 3840,
+		.height		= 2160,
+		.code		= MEDIA_BUS_FMT_Y10_1X10,
+		.bpp		= 10,
+		.vts		= 0x90a,
+		.hts		= 0x818,
+		.exp		= 0x90a - 8,
+		.h_bin		= false,
+		.pixel_rate	= OS08A20_PIXEL_RATE_288M,
+		.reg_data	= os08a20_reglist_4k_10b,
+	}, {
+		/* 4k BGGR12, no hdr, 30fps */
+		.index		= 2,
+		.width		= 3840,
+		.height		= 2160,
+		.code		= MEDIA_BUS_FMT_Y12_1X12,
+		.bpp		= 12,
+		.vts		= 0x8f0,
+		.hts		= 0x814,
+		.exp		= 0x8f0 - 8,
+		.h_bin		= false,
+		.pixel_rate	= OS08A20_PIXEL_RATE_288M,
+		.reg_data	= os08a20_reglist_4k_12b,
+	}, {
+		/* sentinel */
+	}
+};
+
+/* keep in sync with os08a20_supported_modes */
+static const struct v4l2_area os08a20_sbggr10_sizes[] = {
+	{
+		.width = 1920,
+		.height = 1080,
+	}, {
+		.width = 3840,
+		.height = 2160,
+	}, {
+		/* sentinel */
+	}
+};
+
+static const struct v4l2_area os08a20_sbggr12_sizes[] = {
+	{
+		.width = 3840,
+		.height = 2160,
+	}, {
+		/* sentinel */
+	}
+};
+
+static const struct ox05b1s_sizes os08a20_supported_codes[] = {
+	{
+		.code = MEDIA_BUS_FMT_Y10_1X10,
+		.sizes = os08a20_sbggr10_sizes,
+	}, {
+		.code = MEDIA_BUS_FMT_Y12_1X12,
+		.sizes = os08a20_sbggr12_sizes,
+	}, {
+		/* sentinel */
+	}
+};
+
 #define OX05B1S_PIXEL_RATE_48M	48000000
 static const struct ox05b1s_mode ox05b1s_supported_modes[] = {
 	{
@@ -231,6 +317,62 @@ static int ox05b1s_write_reg_array(struct ox05b1s *sensor,
 	return 0;
 }
 
+static const char * const os08a20_hdr_modes[] = {
+	"NO HDR", /* No HDR, single exposure */
+	"HDR Staggered", /* Staggered HDR mode, 2 exposures on separate VC */
+};
+
+static const struct ox05b1s_reg os08a20_init_setting_hdr_en[] = {
+	{ 0x3661, BIT(0) }, /* CORE1[0] STG_HDR_ALIGN_EN */
+	{ 0x3821, BIT(5) }, /* FORMAT2[5] STG_HDR_EN */
+	{ 0x4813, BIT(3) }, /* MIPI_CTRL_13[3] */
+	{ 0x486e, BIT(2) }, /* MIPI_CTRL_6E[2] MIPI_VC_ENABLE */
+};
+
+static int os08a20_enable_staggered_hdr(struct ox05b1s *sensor)
+{
+	int ret;
+
+	for (int i = 0; i < ARRAY_SIZE(os08a20_init_setting_hdr_en); i++) {
+		ret = regmap_update_bits(sensor->regmap,
+					 os08a20_init_setting_hdr_en[i].addr,
+					 os08a20_init_setting_hdr_en[i].data,
+					 os08a20_init_setting_hdr_en[i].data);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int os08a20_disable_staggered_hdr(struct ox05b1s *sensor)
+{
+	int ret;
+
+	for (int i = 0; i < ARRAY_SIZE(os08a20_init_setting_hdr_en); i++) {
+		ret = regmap_update_bits(sensor->regmap,
+					 os08a20_init_setting_hdr_en[i].addr,
+					 os08a20_init_setting_hdr_en[i].data,
+					 0);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int os08a20_set_hdr_mode(struct ox05b1s *sensor, u32 hdr_mode)
+{
+	switch (hdr_mode) {
+	case 0:
+		return os08a20_disable_staggered_hdr(sensor);
+	case 1:
+		return os08a20_enable_staggered_hdr(sensor);
+	default:
+		return -EINVAL;
+	}
+}
+
 static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
 {
 	return &container_of(ctrl->handler, struct ox05b1s,
@@ -274,6 +416,12 @@ static int ox05b1s_s_ctrl(struct v4l2_ctrl *ctrl)
 		ret = cci_write(sensor->regmap, OX05B1S_REG_EXPOSURE,
 				ctrl->val, NULL);
 		break;
+	case V4L2_CID_HDR_SENSOR_MODE:
+		if (sensor->model->set_hdr_mode)
+			ret = sensor->model->set_hdr_mode(sensor, ctrl->val);
+		else
+			ret = -EINVAL;
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -336,6 +484,13 @@ static int ox05b1s_init_controls(struct ox05b1s *sensor)
 	ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN,
 					0, 0xFFFF, 1, 0x80);
 
+	if (sensor->model->hdr_modes)
+		ctrls->hdr_mode = v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_HDR_SENSOR_MODE,
+							       sensor->model->hdr_modes_count - 1,
+								0, 0, sensor->model->hdr_modes);
+	else
+		ctrls->hdr_mode = NULL;
+
 	if (hdl->error) {
 		ret = hdl->error;
 		goto free_ctrls;
@@ -658,6 +813,8 @@ static u8 ox05b1s_code2dt(const u32 code)
 	switch (code) {
 	case MEDIA_BUS_FMT_Y10_1X10:
 		return MIPI_CSI2_DT_RAW10;
+	case MEDIA_BUS_FMT_Y12_1X12:
+		return MIPI_CSI2_DT_RAW12;
 	default:
 		return MIPI_CSI2_DT_RAW10;
 	}
@@ -768,6 +925,9 @@ static int ox05b1s_read_chip_id(struct ox05b1s *sensor)
 	}
 
 	switch (chip_id) {
+	case 0x530841:
+		camera_name = "os08a20";
+		break;
 	case 0x580542:
 		camera_name = "ox05b1s";
 		break;
@@ -912,6 +1072,24 @@ static void ox05b1s_remove(struct i2c_client *client)
 static DEFINE_RUNTIME_DEV_PM_OPS(ox05b1s_pm_ops, ox05b1s_runtime_suspend,
 				 ox05b1s_runtime_resume, NULL);
 
+static const struct ox05b1s_plat_data os08a20_data = {
+	.name			= "os08a20",
+	.chip_id		= 0x530841,
+	.native_width		= 3872, /* 16 dummy + 3840 active + 16 dummy */
+	.native_height		= 2192, /* 16 dummy + 2160 active + 16 dummy */
+	.active_top		= 16,
+	.active_left		= 16,
+	.active_width		= 3840,
+	.active_height		= 2160,
+	.supported_modes	= os08a20_supported_modes,
+	.default_mode_index	= 0,
+	.supported_codes	= os08a20_supported_codes,
+	.hdr_modes		= os08a20_hdr_modes,
+	.hdr_modes_count	= ARRAY_SIZE(os08a20_hdr_modes),
+	.set_hdr_mode		= os08a20_set_hdr_mode,
+
+};
+
 static const struct ox05b1s_plat_data ox05b1s_data = {
 	.name			= "ox05b1s",
 	.chip_id		= 0x580542,
@@ -923,10 +1101,17 @@ static const struct ox05b1s_plat_data ox05b1s_data = {
 	.active_height		= 1944,
 	.supported_modes	= ox05b1s_supported_modes,
 	.default_mode_index	= 0,
-	.supported_codes	= ox05b1s_supported_codes
+	.supported_codes	= ox05b1s_supported_codes,
+	.hdr_modes		= NULL,
+	.hdr_modes_count	= 0,
+	.set_hdr_mode		= NULL
 };
 
 static const struct of_device_id ox05b1s_of_match[] = {
+	{
+		.compatible = "ovti,os08a20",
+		.data = &os08a20_data,
+	},
 	{
 		.compatible = "ovti,ox05b1s",
 		.data = &ox05b1s_data,
diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_modes.c b/drivers/media/i2c/ox05b1s/ox05b1s_modes.c
index 9a1f3a89077c..e17c32a90cba 100644
--- a/drivers/media/i2c/ox05b1s/ox05b1s_modes.c
+++ b/drivers/media/i2c/ox05b1s/ox05b1s_modes.c
@@ -10,6 +10,116 @@
 #include <media/v4l2-cci.h>
 #include "ox05b1s.h"
 
+#define OS08A20_REG_MIPI_BIT_10_12	CCI_REG8(0x031e)
+/* Analog Control Registers 0x3600-0x3637 */
+#define OS08A20_REG_ANA_CTRL		CCI_REG8(0x3600)
+#define OS08A20_REG_CORE_0		CCI_REG8(0x3660)
+/* Sensor Timing Control Registers 0x3700-0x37ff */
+#define OS08A20_REG_SENSOR_TIMING_CTRL	CCI_REG8(0x3700)
+#define OS08A20_REG_X_ODD_INC		CCI_REG8(0x3814)
+#define OS08A20_REG_Y_ODD_INC		CCI_REG8(0x3816)
+#define OS08A20_REG_FORMAT1		CCI_REG8(0x3820)
+#define OS08A20_REG_FORMAT2		CCI_REG8(0x3821)
+#define OS08A20_REG_PCLK_PERIOD		CCI_REG8(0x4837)
+#define OS08A20_REG_ISP_CTRL_1		CCI_REG8(0x5001)
+#define OS08A20_REG_ISP_CTRL_5		CCI_REG8(0x5005)
+
+/* Common register configuration for Omnivision OS08A20 raw camera */
+static const struct ox05b1s_reg os08a20_init_setting_common[] = {
+	{ OS08A20_REG_ANA_CTRL + 0x05, 0x50 },
+	{ OS08A20_REG_ANA_CTRL + 0x10, 0x39 },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x5e, 0x0b },
+	{ OS08A20_REG_ISP_CTRL_1, 0x42 },
+	{ OS08A20_REG_ISP_CTRL_5, 0x00 },
+	{ /* sentinel*/ }
+};
+
+/* Common register configuration for Omnivision OS08A20 10 bit */
+static const struct ox05b1s_reg os08a20_init_setting_10bit[] = {
+	{ OS08A20_REG_MIPI_BIT_10_12, 0x09 },
+	{ OS08A20_REG_ANA_CTRL + 0x09, 0xb5 },
+	{ OS08A20_REG_CORE_0, 0x43 },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x06, 0x35 },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x0a, 0x00 },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x0b, 0x98 },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x09, 0x49 },
+	{ /* sentinel*/ }
+};
+
+/* Common register configuration for Omnivision OS08A20 12 bit */
+static const struct ox05b1s_reg os08a20_init_setting_12bit[] = {
+	{ OS08A20_REG_MIPI_BIT_10_12, 0x0a },
+	{ OS08A20_REG_ANA_CTRL + 0x09, 0xdb },
+	{ OS08A20_REG_CORE_0, 0xd3 },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x06, 0x6a },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x0a, 0x01 },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x0b, 0x30 },
+	{ OS08A20_REG_SENSOR_TIMING_CTRL + 0x09, 0x48 },
+	{ /* sentinel*/ }
+};
+
+/* Mode specific register configurations for Omnivision OS08A20 raw camera */
+
+/* OS08A20 3840 x 2160 @30fps BGGR10 no more HDR */
+static const struct ox05b1s_reg os08a20_init_setting_4k_10b[] = {
+	{ OS08A20_REG_FORMAT2, 0x04 }, /* mirror */
+	{ OS08A20_REG_PCLK_PERIOD, 0x10 },
+	{ /* sentinel*/ }
+};
+
+/* OS08A20 3840 x 2160 @30fps BGGR12 */
+static const struct ox05b1s_reg os08a20_init_setting_4k_12b[] = {
+	{ OS08A20_REG_FORMAT2, 0x04 }, /* mirror */
+	{ OS08A20_REG_PCLK_PERIOD, 0x10 },
+	{ /* sentinel*/ }
+};
+
+/* OS08A20 1920 x 1080 @60fps BGGR10 */
+static const struct ox05b1s_reg os08a20_init_setting_1080p_10b[] = {
+	{ OS08A20_REG_X_ODD_INC, 0x03 },
+	{ OS08A20_REG_Y_ODD_INC, 0x03 },
+	{ OS08A20_REG_FORMAT1, 0x01 }, /* vertical bining */
+	{ OS08A20_REG_FORMAT2, 0x05 }, /* mirror, horizontal bining */
+	{ OS08A20_REG_PCLK_PERIOD, 0x16 },
+	{ /* sentinel*/ }
+};
+
+const struct ox05b1s_reglist os08a20_reglist_4k_10b[] = {
+	{
+		.regs = os08a20_init_setting_common
+	}, {
+		.regs = os08a20_init_setting_10bit
+	}, {
+		.regs = os08a20_init_setting_4k_10b
+	}, {
+		/* sentinel */
+	}
+};
+
+const struct ox05b1s_reglist os08a20_reglist_4k_12b[] = {
+	{
+		.regs = os08a20_init_setting_common
+	}, {
+		.regs = os08a20_init_setting_12bit
+	}, {
+		.regs = os08a20_init_setting_4k_12b
+	}, {
+		/* sentinel */
+	}
+};
+
+const struct ox05b1s_reglist os08a20_reglist_1080p_10b[] = {
+	{
+		.regs = os08a20_init_setting_common
+	}, {
+		.regs = os08a20_init_setting_10bit
+	}, {
+		.regs = os08a20_init_setting_1080p_10b
+	}, {
+		/* sentinel */
+	}
+};
+
 #define OX05B1S_REG_PLL1_CTRL_REG07	CCI_REG8(0x0307)
 #define OX05B1S_REG_PLL3_CTRL_REG4A	CCI_REG8(0x034a)
 #define OX05B1S_REG_PLL_MONITOR_REG0B	CCI_REG8(0x040b)
-- 
2.25.1

Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Jai Luthra]

[-- Attachment #1: Type: text/plain, Size: 6817 bytes --]

Hi Mirela,

Thanks a lot for your patch/series.

On Mar 05, 2025 at 11:43:57 +0200, Mirela Rabulea wrote:
> Add a v4l2 subdevice driver for the Omnivision OX05B1S RGB-IR sensor.
> 
> The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an
> active array size of 2592 x 1944.
> 
> The following features are supported for OX05B1S:
> - Manual exposure an gain control support
> - vblank/hblank control support
> - Supported resolution: 2592 x 1944 @ 30fps (SGRBG10)
> 
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
> Changes in v4:
> 	Switch to Y media bus codes. The CFA pattern control will be implemented when patches get merged, or maybe separatelly as RFC?
> 	Add pixel_rate member to mode struct, remove fps member. We do not have information how to calculate the pixel rate from the PLL parameters that can be made public.
> 	Use register macros for the registers that are documented. User register group macros, where individual registers are not documented
> 	Remove some uneeded local variable initialisations
> 	Fix extra/missing spaces
> 	Add missing ending \n
> 	Use return -ENODEV & return 0 to ease reading
> 	Rename retval to ret in probe for consistency
> 	Use devm_mutex_init instead of mutex_init
> 	Replace more dev_err's with dev_err_probe
> 	Constify more structs
> 	Remove some unneded ending commas after a terminator
> 	Fix smatch error in ox05b1s_s_ctrl: error: typename in expression
> 	Fix a seeries of smatch warnings like: warning: symbol 'ovx5b_init_setting_2592x1944' was not declared. Should it be static?
> 	Shorten some more lines to 80 columns
> 
> Changes in v3:
> 	Use helpers from v4l2-cci.h (drop ox05b1s_write_reg, ox05b1s_read_reg, ox05b1s_set_hts/vts/exp/analog_gain, ox05b1s_regmap_config)
> 	Don't hardcode timing registers: remove timing registers x_output_size/y_output_size from register configuration list, add them to ox05b1s_apply_current_mode
> 	Remove HTS,VTS from register config list as they are written by HBLANK and VBLANK controls through __v4l2_ctrl_handler_setup
> 	ox05b1s register config cleaning (remove all registers that were at their default value, and more, keep only what seems mandatory to be able to stream)
> 	Use const for ox05b1s_supported_modes
> 	Device should be silent on success, use dev_dbg.
> 	Drop unneeded {}
> 	Fixed an error introduced in v2 in ox05b1s_nearest_size (set_fmt for 4k BGGR12 mode was stuck)
> 	Fix an issue in ox05b1s_set_fmt, the format was saved in subdev state only for _TRY, save it also for _ACTIVE
> 
> Changes in v2:
> 	Use dev_err_probe for missing clock, since it is now required property, and use NULL for devm_clk_get (no name for single clock), remove check for non NULL sensor->sensor_clk
> 	Remove dev_err message for devm_regmap_init_i2c allocation error
> 	Added spaces inside brackets, wrap lines to 80
> 	Remove some redundant initializations
> 	Add regulators
> 	Make "sizes" a pointer
> 		Use struct v4l2_area instead of u32[2] array
> 		Remove the count for supported_modes[] and supported_codes[], instead use sentinel element at the end
> 		Consequently, update ox05b1s_enum_mbus_code, ox05b1s_enum_frame_size, ox05b1s_nearest_size, ox05b1s_find_code, to not use the count
> 	Remove .h files for modes, however did not move this code in the driver file but added a separate c file for all supported modes
> 	Refactor register lists to allow multiple register arrays per mode
> 	Use GPL-2.0-only instead of GPL-2.0
> 
>  drivers/media/i2c/Kconfig                 |   1 +
>  drivers/media/i2c/Makefile                |   1 +
>  drivers/media/i2c/ox05b1s/Kconfig         |  10 +
>  drivers/media/i2c/ox05b1s/Makefile        |   2 +
>  drivers/media/i2c/ox05b1s/ox05b1s.h       |  22 +
>  drivers/media/i2c/ox05b1s/ox05b1s_mipi.c  | 951 ++++++++++++++++++++++
>  drivers/media/i2c/ox05b1s/ox05b1s_modes.c |  77 ++
>  7 files changed, 1064 insertions(+)
>  create mode 100644 drivers/media/i2c/ox05b1s/Kconfig
>  create mode 100644 drivers/media/i2c/ox05b1s/Makefile
>  create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s.h
>  create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>  create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_modes.c
> 
[snip]
> diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c 
> b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> new file mode 100644
> index 000000000000..1026216ddd5b
> --- /dev/null
> +++ b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> @@ -0,0 +1,951 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * A V4L2 driver for Omnivision OX05B1S RGB-IR camera.
> + * Copyright (C) 2024, NXP
> + *
> + * Inspired from Sony imx219, imx290, imx214 and imx334 camera drivers
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <media/v4l2-cci.h>
> +#include <media/mipi-csi2.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +
> +#include "ox05b1s.h"
> +
> +#define OX05B1S_SENS_PAD_SOURCE	0
> +#define OX05B1S_SENS_PADS_NUM	1
> +
> +#define OX05B1S_REG_SW_STB		CCI_REG8(0x0100)
> +#define OX05B1S_REG_SW_RST		CCI_REG8(0x0103)

> +#define OX05B1S_REG_CHIP_ID		CCI_REG24(0x300a)
> +#define OX05B1S_REG_TIMING_HTS		CCI_REG16(0x380c)
> +#define OX05B1S_REG_TIMING_VTS		CCI_REG16(0x380e)
> +#define OX05B1S_REG_EXPOSURE		CCI_REG16(0x3501)
> +#define OX05B1S_REG_GAIN		CCI_REG16(0x3508)

There is a non-trivial overlap of registers between this driver and 
ov9282.c which supports OV9281/OV9282 (1MP Mono).

There are two other Omnivision sensors, OV2311 (2MP Mono) and OV2312 
(2MP 4x4 RGB-IR Bayer) with an even larger register overlap with OX05B1S 
and OS08A20. Unfortunately those two have separate downstream drivers in 
RPi and TI linux downstream trees respectively, and haven't yet been 
posted upstream.

It would be ideal to have a single driver for all of these Omnivision 
sensors, or if not, at least a common C module that can implement the 
shared functionality like gain, exposure, blanking (vts & hts) in a 
single place, as this will make maintenance much easier.

My question here to you and the maintainers is, would it be okay to use 
this driver as a baseline to integrate all these different sensors 
together? And secondly, would you like to take a look at supporting 
ov9282, so the other driver can be dropped?

Anyway thanks again for your series, hopefully this will give a good 
starting point for upstreaming OV2311 and OV2312 soon.

Thanks,
Jai

> +#define OX05B1S_REG_X_OUTPUT_SIZE	CCI_REG16(0x3808)
> +#define OX05B1S_REG_Y_OUTPUT_SIZE	CCI_REG16(0x380a)
> +

[snip]

> 

-- 
Thanks,
Jai

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Mirela Rabulea]

Hi Jai and all,

On 19.03.2025 13:10, Jai Luthra wrote:

    Hi Mirela,

    Thanks a lot for your patch/series.

    On Mar 05, 2025 at 11:43:57 +0200, Mirela Rabulea wrote:

        Add a v4l2subdevice  driver for theOmnivision  OX05B1S RGB-IR sensor.

        TheOmnivision  OX05B1S is a 1/2.5-Inch CMOS image sensor with an

        active array size of 2592 x 1944.

        The following features are supported for OX05B1S:

        - Manual exposure an gain control support

        -vblank/hblank  control support

        - Supported resolution: 2592 x 1944 @ 30fps (SGRBG10)

        Signed-off-by: Mirela Rabulea<mirela.rabulea@nxp.com>  <mailto:mirela.rabulea@nxp.com>

        ---

        Changes in v4:

        Switch to Y media bus codes. The CFA pattern control will be implemented when patches get merged, or maybeseparatelly  as RFC?

        Addpixel_rate  member to mode struct, remove fps member. We do not have information how to calculate the pixel rate from the PLL parameters that can be made public.

        Use register macros for the registers that are documented. User register group macros, where individual registers are not documented

        Remove someuneeded  local variableinitialisations

        Fix extra/missing spaces

        Add missing ending \n

        Use return -ENODEV & return 0 to ease reading

        Renameretval  to ret in probe for consistency

        Usedevm_mutex_init  instead ofmutex_init

        Replace moredev_err's  withdev_err_probe

        Constify  more structs

        Remove someunneded  ending commas after a terminator

        Fixsmatch  error in ox05b1s_s_ctrl: error:typename  in expression

        Fix aseeries  ofsmatch  warnings like: warning: symbol 'ovx5b_init_setting_2592x1944' was not declared. Should it be static?

        Shorten some more lines to 80 columns

        Changes in v3:

        Use helpers from v4l2-cci.h (drop ox05b1s_write_reg, ox05b1s_read_reg, ox05b1s_set_hts/vts/exp/analog_gain, ox05b1s_regmap_config)

        Don't hardcode timing registers: remove timing registersx_output_size/y_output_size  from register configuration list, add them to ox05b1s_apply_current_mode

        Remove HTS,VTS from register config list as they are written by HBLANK and VBLANK controls through __v4l2_ctrl_handler_setup

        ox05b1s register config cleaning (remove all registers that were at their default value, and more, keep only what seems mandatory to be able to stream)

        Use const for ox05b1s_supported_modes

        Device should be silent on success, usedev_dbg.

        Drop unneeded {}

        Fixed an error introduced in v2 in ox05b1s_nearest_size (set_fmt  for 4k BGGR12 mode was stuck)

        Fix an issue in ox05b1s_set_fmt, the format was saved insubdev  state only for _TRY, save it also for _ACTIVE

        Changes in v2:

        Usedev_err_probe  for missing clock, since it is now required property, and use NULL fordevm_clk_get  (no name for single clock), remove check fornon NULL  sensor->sensor_clk

        Removedev_err  message for devm_regmap_init_i2c allocation error

        Added spaces inside brackets, wrap lines to 80

        Remove some redundant initializations

        Add regulators

        Make "sizes" a pointer

        Use struct v4l2_area instead of u32[2] array

        Remove the count forsupported_modes[] andsupported_codes[], instead use sentinel element at the end

        Consequently, update ox05b1s_enum_mbus_code, ox05b1s_enum_frame_size, ox05b1s_nearest_size, ox05b1s_find_code, to not use the count

        Remove .h files for modes, however did not move this code in the driver file but added a separate c file for all supported modes

        Refactor register lists to allow multiple register arrays per mode

        Use GPL-2.0-only instead of GPL-2.0

          drivers/media/i2c/Kconfig|1 +

          drivers/media/i2c/Makefile|1 +

          drivers/media/i2c/ox05b1s/Kconfig|10 +

          drivers/media/i2c/ox05b1s/Makefile|2 +

          drivers/media/i2c/ox05b1s/ox05b1s.h|22 +

          drivers/media/i2c/ox05b1s/ox05b1s_mipi.c| 951 ++++++++++++++++++++++

          drivers/media/i2c/ox05b1s/ox05b1s_modes.c |77 ++

          7 files changed, 1064 insertions(+)

          create mode 100644 drivers/media/i2c/ox05b1s/Kconfig

          create mode 100644 drivers/media/i2c/ox05b1s/Makefile

          create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s.h

          create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c

          create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_modes.c

    [snip]

        diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c

        b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c

        new file mode 100644

        index 000000000000..1026216ddd5b

        --- /dev/null

        +++ b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c

        @@ -0,0 +1,951 @@

        +// SPDX-License-Identifier: GPL-2.0-only

        +/*

        + * A V4L2 driver forOmnivision  OX05B1S RGB-IR camera.

        + * Copyright (C) 2024, NXP

        + *

        + * Inspired from Sony imx219, imx290, imx214 and imx334 camera drivers

        + *

        + */

        +

        +#include <linux/clk.h>

        +#include <linux/pm_runtime.h>

        +#include <linux/regmap.h>

        +#include <linux/regulator/consumer.h>

        +#include <media/v4l2-cci.h>

        +#include <media/mipi-csi2.h>

        +#include <media/v4l2-ctrls.h>

        +#include <media/v4l2-device.h>

        +#include <media/v4l2-fwnode.h>

        +

        +#include "ox05b1s.h"

        +

        +#define OX05B1S_SENS_PAD_SOURCE0

        +#define OX05B1S_SENS_PADS_NUM1

        +

        +#define OX05B1S_REG_SW_STBCCI_REG8(0x0100)

        +#define OX05B1S_REG_SW_RSTCCI_REG8(0x0103)

        +#define OX05B1S_REG_CHIP_IDCCI_REG24(0x300a)

        +#define OX05B1S_REG_TIMING_HTSCCI_REG16(0x380c)

        +#define OX05B1S_REG_TIMING_VTSCCI_REG16(0x380e)

        +#define OX05B1S_REG_EXPOSURECCI_REG16(0x3501)

        +#define OX05B1S_REG_GAINCCI_REG16(0x3508)

    There is a non-trivial overlap of registers between this driver and

    ov9282.c which supports OV9281/OV9282 (1MP Mono).

    There are two otherOmnivision  sensors, OV2311 (2MP Mono) and OV2312

    (2MP 4x4 RGB-IR Bayer) with an even larger register overlap with OX05B1S

    and OS08A20. Unfortunately those two have separate downstream drivers in

    RPi and TIlinux  downstream trees respectively, and haven't yet been

    posted upstream.

Thanks for sharing this information, I was unaware. The question of how 
much similarity should two sensors share, in order to stay in the same 
driver, was also on my mind for some time, and I’d be glad to hear more 
opinions on it ;)

    It would be ideal to have a single driver for all of theseOmnivision  

    sensors, or if not, at least a common C module that can implement the

    shared functionality like gain, exposure, blanking (vts  &hts) in a

    single place, as this will make maintenance much easier.

I would need to get more information on the sensors you mentioned in 
order to issue an informed opinion. So far, with the OX05B1S and 
OS08A20, I have found some small differences regarding exposure and 
digital gain registers, so the overlap is not perfect, I expect it will 
also not be a perfect overlap with the other sensors you mentioned.

    My question here to you and the maintainers is, would it be okay to use

    this driver as a baseline to integrate all these different sensors

    together? And secondly, would you like to take a look at supporting

    ov9282, so the other driver can be dropped?

For the first question, I don't know what to say, and I cannot tell if 
we are far or close for this patch-set to be accepted. Also, I am unsure 
about how maintenance would go on a driver claiming to support a 
multitude of sensors, who could test them all, whenever something 
changes? Are you thinking to add ov2311/12 as other compatibles to this 
driver?

I agree there is a great deal of similarity shared across many raw, 
mode-based sensor drivers, and it sounds good to have some common 
framework. Some steps were done with the common raw sensor model. I 
would definitely also like to hear more expert opinions on this.

For the second question, as of now, we do not have any of the sensors 
you mentioned, unfortunately. I could help in the future to test patches 
for this driver on the sensors that we already have, but cannot make any 
promises for what I do not have, best effort if we find these sensors in 
a form factor that will work for our boards.

    Anyway thanks again for your series, hopefully this will give a good

    starting point for upstreaming OV2311 and OV2312 soon.

I would like to know more about the OV2312 (RGB-Ir) sensor and if it has 
many similarities with OX05B1S.  What hardware are you using to test 
this sensor? And what interface to connect the sensor? We are working 
with MIPI-CSI on most imx boards, and also RPI on imx93.

  Regards,

Mirela

    Thanks,

    Jai

        +#define OX05B1S_REG_X_OUTPUT_SIZECCI_REG16(0x3808)

        +#define OX05B1S_REG_Y_OUTPUT_SIZECCI_REG16(0x380a)

        +

    [snip]

Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Jai Luthra]

[-- Attachment #1: Type: text/plain, Size: 10961 bytes --]

On Mar 24, 2025 at 17:32:01 +0200, Mirela Rabulea wrote:
> Hi Jai and all,
> 
> On Mar 19, 2025 at 16:40:30 +0530, Jai Luthra wrote:
> > Hi Mirela,
> > 
> > Thanks a lot for your patch/series.
> > 
> > On Mar 05, 2025 at 11:43:57 +0200, Mirela Rabulea wrote:
> > > Add a v4l2 subdevice driver for the Omnivision OX05B1S RGB-IR sensor.
> > > 
> > > The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an
> > > active array size of 2592 x 1944.
> > > 
> > > The following features are supported for OX05B1S:
> > > - Manual exposure an gain control support
> > > - vblank/hblank control support
> > > - Supported resolution: 2592 x 1944 @ 30fps (SGRBG10)
> > > 
> > > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> > > ---
> > > Changes in v4:
> > > 	Switch to Y media bus codes. The CFA pattern control will be implemented when patches get merged, or maybe separatelly as RFC?
> > > 	Add pixel_rate member to mode struct, remove fps member. We do not have information how to calculate the pixel rate from the PLL parameters that can be made public.
> > > 	Use register macros for the registers that are documented. User register group macros, where individual registers are not documented
> > > 	Remove some uneeded local variable initialisations
> > > 	Fix extra/missing spaces
> > > 	Add missing ending \n
> > > 	Use return -ENODEV & return 0 to ease reading
> > > 	Rename retval to ret in probe for consistency
> > > 	Use devm_mutex_init instead of mutex_init
> > > 	Replace more dev_err's with dev_err_probe
> > > 	Constify more structs
> > > 	Remove some unneded ending commas after a terminator
> > > 	Fix smatch error in ox05b1s_s_ctrl: error: typename in expression
> > > 	Fix a seeries of smatch warnings like: warning: symbol 'ovx5b_init_setting_2592x1944' was not declared. Should it be static?
> > > 	Shorten some more lines to 80 columns
> > > 
> > > Changes in v3:
> > > 	Use helpers from v4l2-cci.h (drop ox05b1s_write_reg, ox05b1s_read_reg, ox05b1s_set_hts/vts/exp/analog_gain, ox05b1s_regmap_config)
> > > 	Don't hardcode timing registers: remove timing registers x_output_size/y_output_size from register configuration list, add them to ox05b1s_apply_current_mode
> > > 	Remove HTS,VTS from register config list as they are written by HBLANK and VBLANK controls through __v4l2_ctrl_handler_setup
> > > 	ox05b1s register config cleaning (remove all registers that were at their default value, and more, keep only what seems mandatory to be able to stream)
> > > 	Use const for ox05b1s_supported_modes
> > > 	Device should be silent on success, use dev_dbg.
> > > 	Drop unneeded {}
> > > 	Fixed an error introduced in v2 in ox05b1s_nearest_size (set_fmt for 4k BGGR12 mode was stuck)
> > > 	Fix an issue in ox05b1s_set_fmt, the format was saved in subdev state only for _TRY, save it also for _ACTIVE
> > > 
> > > Changes in v2:
> > > 	Use dev_err_probe for missing clock, since it is now required property, and use NULL for devm_clk_get (no name for single clock), remove check for non NULL sensor->sensor_clk
> > > 	Remove dev_err message for devm_regmap_init_i2c allocation error
> > > 	Added spaces inside brackets, wrap lines to 80
> > > 	Remove some redundant initializations
> > > 	Add regulators
> > > 	Make "sizes" a pointer
> > > 		Use struct v4l2_area instead of u32[2] array
> > > 		Remove the count for supported_modes[] and supported_codes[], instead use sentinel element at the end
> > > 		Consequently, update ox05b1s_enum_mbus_code, ox05b1s_enum_frame_size, ox05b1s_nearest_size, ox05b1s_find_code, to not use the count
> > > 	Remove .h files for modes, however did not move this code in the driver file but added a separate c file for all supported modes
> > > 	Refactor register lists to allow multiple register arrays per mode
> > > 	Use GPL-2.0-only instead of GPL-2.0
> > > 
> > >  drivers/media/i2c/Kconfig                 |   1 +
> > >  drivers/media/i2c/Makefile                |   1 +
> > >  drivers/media/i2c/ox05b1s/Kconfig         |  10 +
> > >  drivers/media/i2c/ox05b1s/Makefile        |   2 +
> > >  drivers/media/i2c/ox05b1s/ox05b1s.h       |  22 +
> > >  drivers/media/i2c/ox05b1s/ox05b1s_mipi.c  | 951 ++++++++++++++++++++++
> > >  drivers/media/i2c/ox05b1s/ox05b1s_modes.c |  77 ++
> > >  7 files changed, 1064 insertions(+)
> > >  create mode 100644 drivers/media/i2c/ox05b1s/Kconfig
> > >  create mode 100644 drivers/media/i2c/ox05b1s/Makefile
> > >  create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s.h
> > >  create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> > >  create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_modes.c
> > > 
> > [snip]
> > > diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c 
> > > b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> > > new file mode 100644
> > > index 000000000000..1026216ddd5b
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> > > @@ -0,0 +1,951 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * A V4L2 driver for Omnivision OX05B1S RGB-IR camera.
> > > + * Copyright (C) 2024, NXP
> > > + *
> > > + * Inspired from Sony imx219, imx290, imx214 and imx334 camera drivers
> > > + *
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <media/v4l2-cci.h>
> > > +#include <media/mipi-csi2.h>
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +
> > > +#include "ox05b1s.h"
> > > +
> > > +#define OX05B1S_SENS_PAD_SOURCE	0
> > > +#define OX05B1S_SENS_PADS_NUM	1
> > > +
> > > +#define OX05B1S_REG_SW_STB		CCI_REG8(0x0100)
> > > +#define OX05B1S_REG_SW_RST		CCI_REG8(0x0103)
> > 
> > > +#define OX05B1S_REG_CHIP_ID		CCI_REG24(0x300a)
> > > +#define OX05B1S_REG_TIMING_HTS		CCI_REG16(0x380c)
> > > +#define OX05B1S_REG_TIMING_VTS		CCI_REG16(0x380e)
> > > +#define OX05B1S_REG_EXPOSURE		CCI_REG16(0x3501)
> > > +#define OX05B1S_REG_GAIN		CCI_REG16(0x3508)
> > 
> > There is a non-trivial overlap of registers between this driver and 
> > ov9282.c which supports OV9281/OV9282 (1MP Mono).
> > 
> > There are two other Omnivision sensors, OV2311 (2MP Mono) and OV2312 
> > (2MP 4x4 RGB-IR Bayer) with an even larger register overlap with OX05B1S 
> > and OS08A20. Unfortunately those two have separate downstream drivers in 
> > RPi and TI linux downstream trees respectively, and haven't yet been 
> > posted upstream.
>
> Thanks for sharing this information, I was unaware. The question of 
> how much similarity should two sensors share, in order to stay in the 
> same driver, was also on my mind for some time, and I’d be glad to 
> hear more opinions on it ;)
> 

Same here :)

> > 
> > It would be ideal to have a single driver for all of these Omnivision 
> > sensors, or if not, at least a common C module that can implement the 
> > shared functionality like gain, exposure, blanking (vts & hts) in a 
> > single place, as this will make maintenance much easier.
> 
> I would need to get more information on the sensors you mentioned in order
> to issue an informed opinion. So far, with the OX05B1S and OS08A20, I have
> found some small differences regarding exposure and digital gain registers,
> so the overlap is not perfect, I expect it will also not be a perfect
> overlap with the other sensors you mentioned.
> 

Sure, I had some experience with supporting OV2312 and OX05B1S in the 
downstream TI linux tree, and while they share the registers for 
exposure and gain, there are some other differences in features, aside 
from the 2MP vs 5MP resolution.

> > 
> > My question here to you and the maintainers is, would it be okay to use 
> > this driver as a baseline to integrate all these different sensors 
> > together? And secondly, would you like to take a look at supporting 
> > ov9282, so the other driver can be dropped?
> > 
> For the first question, I don't know what to say, and I cannot tell if 
> we are far or close for this patch-set to be accepted. Also, I am 
> unsure about how maintenance would go on a driver claiming to support 
> a multitude of sensors, who could test them all, whenever something 
> changes? Are you thinking to add ov2311/12 as other compatibles to 
> this driver?
> 

While it would be ideal to have OV2312 support within this driver if 
there is a significant register overlap, it might still require some 
effort, as TI's downstream drivers for the RGBIR sensors capture two 
streams with different exposure, gain and IR flash values, and different 
MIPI CSI virtual channels, using the group hold functionality. Which 
IIUC may be quite different from what your patches implement, and will 
require adding streams/routing support so the userspace can configure.

> I agree there is a great deal of similarity shared across many raw,
> mode-based sensor drivers, and it sounds good to have some common framework.
> Some steps were done with the common raw sensor model. I would definitely
> also like to hear more expert opinions on this.
> 
> For the second question, as of now, we do not have any of the sensors you
> mentioned, unfortunately. I could help in the future to test patches for
> this driver on the sensors that we already have, but cannot make any
> promises for what I do not have, best effort if we find these sensors in a
> form factor that will work for our boards.
> 

I agree, having a single maintainer would not be feasible given 
different sensor modules may have incompatible connectors. But yes it 
should be okay to provide a T-By tag or a Nack on the shared driver if a 
patch breaks your particular hardware or usecase, similar to how other 
popular sensor drivers are maintained like IMX219 or OV5640.

> > Anyway thanks again for your series, hopefully this will give a good 
> > starting point for upstreaming OV2311 and OV2312 soon.
> > 
> I would like to know more about the OV2312 (RGB-Ir) sensor and if it 
> has many similarities with OX05B1S.  What hardware are you using to 
> test this sensor? And what interface to connect the sensor? We are 
> working with MIPI-CSI on most imx boards, and also RPI on imx93.

For OV2312 I have used this FPDLink module [1] with the Arducam V3Link 
board [2] that connects to the EVM using 22-pin FFC MIPI-CSI connector 
that is pin-compatible with the RPi5 connector.

[1]: https://leopardimaging.com/wp-content/uploads/2024/03/LI-OV2312-FPDLINKIII_Datasheet.pdf 
[2]: https://www.arducam.com/downloads/datasheet/Arducam_V3Link_Datasheet.pdf

> 
>  Regards,
> 
> Mirela

PS: Your mail client broke the quotations on your reply. I have fixed 
those here, but might be a good idea to double-check your client 
settings.

-- 
Thanks,
Jai

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Mirela Rabulea]

Hi Jai,

On 25.03.2025 18:02, Jai Luthra wrote:
> On Mar 24, 2025 at 17:32:01 +0200, Mirela Rabulea wrote:
>> Hi Jai and all,
>>
>> On Mar 19, 2025 at 16:40:30 +0530, Jai Luthra wrote:
>>> Hi Mirela,
>>>
>>> Thanks a lot for your patch/series.
>>>
>>> On Mar 05, 2025 at 11:43:57 +0200, Mirela Rabulea wrote:
>>>> Add a v4l2 subdevice driver for the Omnivision OX05B1S RGB-IR sensor.
>>>>
>>>> The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an
>>>> active array size of 2592 x 1944.
>>>>
>>>> The following features are supported for OX05B1S:
>>>> - Manual exposure an gain control support
>>>> - vblank/hblank control support
>>>> - Supported resolution: 2592 x 1944 @ 30fps (SGRBG10)
>>>>
>>>> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
>>>> ---
>>>> Changes in v4:
>>>> 	Switch to Y media bus codes. The CFA pattern control will be implemented when patches get merged, or maybe separatelly as RFC?
>>>> 	Add pixel_rate member to mode struct, remove fps member. We do not have information how to calculate the pixel rate from the PLL parameters that can be made public.
>>>> 	Use register macros for the registers that are documented. User register group macros, where individual registers are not documented
>>>> 	Remove some uneeded local variable initialisations
>>>> 	Fix extra/missing spaces
>>>> 	Add missing ending \n
>>>> 	Use return -ENODEV & return 0 to ease reading
>>>> 	Rename retval to ret in probe for consistency
>>>> 	Use devm_mutex_init instead of mutex_init
>>>> 	Replace more dev_err's with dev_err_probe
>>>> 	Constify more structs
>>>> 	Remove some unneded ending commas after a terminator
>>>> 	Fix smatch error in ox05b1s_s_ctrl: error: typename in expression
>>>> 	Fix a seeries of smatch warnings like: warning: symbol 'ovx5b_init_setting_2592x1944' was not declared. Should it be static?
>>>> 	Shorten some more lines to 80 columns
>>>>
>>>> Changes in v3:
>>>> 	Use helpers from v4l2-cci.h (drop ox05b1s_write_reg, ox05b1s_read_reg, ox05b1s_set_hts/vts/exp/analog_gain, ox05b1s_regmap_config)
>>>> 	Don't hardcode timing registers: remove timing registers x_output_size/y_output_size from register configuration list, add them to ox05b1s_apply_current_mode
>>>> 	Remove HTS,VTS from register config list as they are written by HBLANK and VBLANK controls through __v4l2_ctrl_handler_setup
>>>> 	ox05b1s register config cleaning (remove all registers that were at their default value, and more, keep only what seems mandatory to be able to stream)
>>>> 	Use const for ox05b1s_supported_modes
>>>> 	Device should be silent on success, use dev_dbg.
>>>> 	Drop unneeded {}
>>>> 	Fixed an error introduced in v2 in ox05b1s_nearest_size (set_fmt for 4k BGGR12 mode was stuck)
>>>> 	Fix an issue in ox05b1s_set_fmt, the format was saved in subdev state only for _TRY, save it also for _ACTIVE
>>>>
>>>> Changes in v2:
>>>> 	Use dev_err_probe for missing clock, since it is now required property, and use NULL for devm_clk_get (no name for single clock), remove check for non NULL sensor->sensor_clk
>>>> 	Remove dev_err message for devm_regmap_init_i2c allocation error
>>>> 	Added spaces inside brackets, wrap lines to 80
>>>> 	Remove some redundant initializations
>>>> 	Add regulators
>>>> 	Make "sizes" a pointer
>>>> 		Use struct v4l2_area instead of u32[2] array
>>>> 		Remove the count for supported_modes[] and supported_codes[], instead use sentinel element at the end
>>>> 		Consequently, update ox05b1s_enum_mbus_code, ox05b1s_enum_frame_size, ox05b1s_nearest_size, ox05b1s_find_code, to not use the count
>>>> 	Remove .h files for modes, however did not move this code in the driver file but added a separate c file for all supported modes
>>>> 	Refactor register lists to allow multiple register arrays per mode
>>>> 	Use GPL-2.0-only instead of GPL-2.0
>>>>
>>>>   drivers/media/i2c/Kconfig                 |   1 +
>>>>   drivers/media/i2c/Makefile                |   1 +
>>>>   drivers/media/i2c/ox05b1s/Kconfig         |  10 +
>>>>   drivers/media/i2c/ox05b1s/Makefile        |   2 +
>>>>   drivers/media/i2c/ox05b1s/ox05b1s.h       |  22 +
>>>>   drivers/media/i2c/ox05b1s/ox05b1s_mipi.c  | 951 ++++++++++++++++++++++
>>>>   drivers/media/i2c/ox05b1s/ox05b1s_modes.c |  77 ++
>>>>   7 files changed, 1064 insertions(+)
>>>>   create mode 100644 drivers/media/i2c/ox05b1s/Kconfig
>>>>   create mode 100644 drivers/media/i2c/ox05b1s/Makefile
>>>>   create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s.h
>>>>   create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>>>>   create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_modes.c
>>>>
>>> [snip]
>>>> diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>>>> b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>>>> new file mode 100644
>>>> index 000000000000..1026216ddd5b
>>>> --- /dev/null
>>>> +++ b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>>>> @@ -0,0 +1,951 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * A V4L2 driver for Omnivision OX05B1S RGB-IR camera.
>>>> + * Copyright (C) 2024, NXP
>>>> + *
>>>> + * Inspired from Sony imx219, imx290, imx214 and imx334 camera drivers
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/clk.h>
>>>> +#include <linux/pm_runtime.h>
>>>> +#include <linux/regmap.h>
>>>> +#include <linux/regulator/consumer.h>
>>>> +#include <media/v4l2-cci.h>
>>>> +#include <media/mipi-csi2.h>
>>>> +#include <media/v4l2-ctrls.h>
>>>> +#include <media/v4l2-device.h>
>>>> +#include <media/v4l2-fwnode.h>
>>>> +
>>>> +#include "ox05b1s.h"
>>>> +
>>>> +#define OX05B1S_SENS_PAD_SOURCE	0
>>>> +#define OX05B1S_SENS_PADS_NUM	1
>>>> +
>>>> +#define OX05B1S_REG_SW_STB		CCI_REG8(0x0100)
>>>> +#define OX05B1S_REG_SW_RST		CCI_REG8(0x0103)
>>>> +#define OX05B1S_REG_CHIP_ID		CCI_REG24(0x300a)
>>>> +#define OX05B1S_REG_TIMING_HTS		CCI_REG16(0x380c)
>>>> +#define OX05B1S_REG_TIMING_VTS		CCI_REG16(0x380e)
>>>> +#define OX05B1S_REG_EXPOSURE		CCI_REG16(0x3501)
>>>> +#define OX05B1S_REG_GAIN		CCI_REG16(0x3508)
>>> There is a non-trivial overlap of registers between this driver and
>>> ov9282.c which supports OV9281/OV9282 (1MP Mono).
>>>
>>> There are two other Omnivision sensors, OV2311 (2MP Mono) and OV2312
>>> (2MP 4x4 RGB-IR Bayer) with an even larger register overlap with OX05B1S
>>> and OS08A20. Unfortunately those two have separate downstream drivers in
>>> RPi and TI linux downstream trees respectively, and haven't yet been
>>> posted upstream.
>> Thanks for sharing this information, I was unaware. The question of
>> how much similarity should two sensors share, in order to stay in the
>> same driver, was also on my mind for some time, and I’d be glad to
>> hear more opinions on it ;)
>>
> Same here :)
>
>>> It would be ideal to have a single driver for all of these Omnivision
>>> sensors, or if not, at least a common C module that can implement the
>>> shared functionality like gain, exposure, blanking (vts & hts) in a
>>> single place, as this will make maintenance much easier.
>> I would need to get more information on the sensors you mentioned in order
>> to issue an informed opinion. So far, with the OX05B1S and OS08A20, I have
>> found some small differences regarding exposure and digital gain registers,
>> so the overlap is not perfect, I expect it will also not be a perfect
>> overlap with the other sensors you mentioned.
>>
> Sure, I had some experience with supporting OV2312 and OX05B1S in the
> downstream TI linux tree, and while they share the registers for
> exposure and gain, there are some other differences in features, aside
> from the 2MP vs 5MP resolution.
>
>>> My question here to you and the maintainers is, would it be okay to use
>>> this driver as a baseline to integrate all these different sensors
>>> together? And secondly, would you like to take a look at supporting
>>> ov9282, so the other driver can be dropped?
>>>
>> For the first question, I don't know what to say, and I cannot tell if
>> we are far or close for this patch-set to be accepted. Also, I am
>> unsure about how maintenance would go on a driver claiming to support
>> a multitude of sensors, who could test them all, whenever something
>> changes? Are you thinking to add ov2311/12 as other compatibles to
>> this driver?
>>
> While it would be ideal to have OV2312 support within this driver if
> there is a significant register overlap, it might still require some
> effort, as TI's downstream drivers for the RGBIR sensors capture two
> streams with different exposure, gain and IR flash values, and different
> MIPI CSI virtual channels, using the group hold functionality. Which
> IIUC may be quite different from what your patches implement, and will
> require adding streams/routing support so the userspace can configure.

You are not alone on this ;)

We have an internal solution for adding streams/routing support to this 
driver, we used it both for ox05b1s (AB mode with 2 pixel streams on 
separate virtual channels) and for os08a20 (staggered hdr mode with 2 
pixel streams on separate virtual channels), and also a separate stream 
for embedded data (same VC but a different mipi data type). I did not 
post these patches because of 2 reasons:

1. We are waiting for internal pads to be accepted, and possibly the 
common raw sensor model

2. There are issues with the individual control (exposure, analog and 
digital gain) per exposure/context, with the current available standard 
controls. This is an entire topic on its own, maybe I should start a 
separate discussion thread on that.

>
>> I agree there is a great deal of similarity shared across many raw,
>> mode-based sensor drivers, and it sounds good to have some common framework.
>> Some steps were done with the common raw sensor model. I would definitely
>> also like to hear more expert opinions on this.
>>
>> For the second question, as of now, we do not have any of the sensors you
>> mentioned, unfortunately. I could help in the future to test patches for
>> this driver on the sensors that we already have, but cannot make any
>> promises for what I do not have, best effort if we find these sensors in a
>> form factor that will work for our boards.
>>
> I agree, having a single maintainer would not be feasible given
> different sensor modules may have incompatible connectors. But yes it
> should be okay to provide a T-By tag or a Nack on the shared driver if a
> patch breaks your particular hardware or usecase, similar to how other
> popular sensor drivers are maintained like IMX219 or OV5640.
Sounds ok to me, we cannot guarantee to test the other sensors, but 
having T-by tag from other users will hopefully cover it.
>
>>> Anyway thanks again for your series, hopefully this will give a good
>>> starting point for upstreaming OV2311 and OV2312 soon.
>>>
>> I would like to know more about the OV2312 (RGB-Ir) sensor and if it
>> has many similarities with OX05B1S.  What hardware are you using to
>> test this sensor? And what interface to connect the sensor? We are
>> working with MIPI-CSI on most imx boards, and also RPI on imx93.
> For OV2312 I have used this FPDLink module [1] with the Arducam V3Link
> board [2] that connects to the EVM using 22-pin FFC MIPI-CSI connector
> that is pin-compatible with the RPi5 connector.
>
> [1]: https://leopardimaging.com/wp-content/uploads/2024/03/LI-OV2312-FPDLINKIII_Datasheet.pdf
> [2]: https://www.arducam.com/downloads/datasheet/Arducam_V3Link_Datasheet.pdf
Thanks, we don't have any ser/des involved in the ox05b1s/os08a20 case, 
if we will ever get into the position to try ov2312, probably we will 
look for some solution for imx95-15x15 board, on the RPi connector 
(22-pin), cannot tell if it will work, one can never tell what may go 
wrong.
>
>>   Regards,
>>
>> Mirela
> PS: Your mail client broke the quotations on your reply. I have fixed
> those here, but might be a good idea to double-check your client
> settings.

Sorry about that, I may have edited the draft from both Thunderbird and 
Outlook. Hope this will be ok, sending from Thunderbird now as plain 
text only.

Regards,

Mirela

Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Jai Luthra]

[-- Attachment #1: Type: text/plain, Size: 14281 bytes --]

On Mar 26, 2025 at 17:32:55 +0200, Mirela Rabulea wrote:
> Hi Jai,
> 
> On 25.03.2025 18:02, Jai Luthra wrote:
> > On Mar 24, 2025 at 17:32:01 +0200, Mirela Rabulea wrote:
> > > Hi Jai and all,
> > > 
> > > On Mar 19, 2025 at 16:40:30 +0530, Jai Luthra wrote:
> > > > Hi Mirela,
> > > > 
> > > > Thanks a lot for your patch/series.
> > > > 
> > > > On Mar 05, 2025 at 11:43:57 +0200, Mirela Rabulea wrote:
> > > > > Add a v4l2 subdevice driver for the Omnivision OX05B1S RGB-IR sensor.
> > > > > 
> > > > > The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an
> > > > > active array size of 2592 x 1944.
> > > > > 
> > > > > The following features are supported for OX05B1S:
> > > > > - Manual exposure an gain control support
> > > > > - vblank/hblank control support
> > > > > - Supported resolution: 2592 x 1944 @ 30fps (SGRBG10)
> > > > > 
> > > > > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> > > > > ---
> > > > > Changes in v4:
> > > > > 	Switch to Y media bus codes. The CFA pattern control will be implemented when patches get merged, or maybe separatelly as RFC?
> > > > > 	Add pixel_rate member to mode struct, remove fps member. We do not have information how to calculate the pixel rate from the PLL parameters that can be made public.
> > > > > 	Use register macros for the registers that are documented. User register group macros, where individual registers are not documented
> > > > > 	Remove some uneeded local variable initialisations
> > > > > 	Fix extra/missing spaces
> > > > > 	Add missing ending \n
> > > > > 	Use return -ENODEV & return 0 to ease reading
> > > > > 	Rename retval to ret in probe for consistency
> > > > > 	Use devm_mutex_init instead of mutex_init
> > > > > 	Replace more dev_err's with dev_err_probe
> > > > > 	Constify more structs
> > > > > 	Remove some unneded ending commas after a terminator
> > > > > 	Fix smatch error in ox05b1s_s_ctrl: error: typename in expression
> > > > > 	Fix a seeries of smatch warnings like: warning: symbol 'ovx5b_init_setting_2592x1944' was not declared. Should it be static?
> > > > > 	Shorten some more lines to 80 columns
> > > > > 
> > > > > Changes in v3:
> > > > > 	Use helpers from v4l2-cci.h (drop ox05b1s_write_reg, ox05b1s_read_reg, ox05b1s_set_hts/vts/exp/analog_gain, ox05b1s_regmap_config)
> > > > > 	Don't hardcode timing registers: remove timing registers x_output_size/y_output_size from register configuration list, add them to ox05b1s_apply_current_mode
> > > > > 	Remove HTS,VTS from register config list as they are written by HBLANK and VBLANK controls through __v4l2_ctrl_handler_setup
> > > > > 	ox05b1s register config cleaning (remove all registers that were at their default value, and more, keep only what seems mandatory to be able to stream)
> > > > > 	Use const for ox05b1s_supported_modes
> > > > > 	Device should be silent on success, use dev_dbg.
> > > > > 	Drop unneeded {}
> > > > > 	Fixed an error introduced in v2 in ox05b1s_nearest_size (set_fmt for 4k BGGR12 mode was stuck)
> > > > > 	Fix an issue in ox05b1s_set_fmt, the format was saved in subdev state only for _TRY, save it also for _ACTIVE
> > > > > 
> > > > > Changes in v2:
> > > > > 	Use dev_err_probe for missing clock, since it is now required property, and use NULL for devm_clk_get (no name for single clock), remove check for non NULL sensor->sensor_clk
> > > > > 	Remove dev_err message for devm_regmap_init_i2c allocation error
> > > > > 	Added spaces inside brackets, wrap lines to 80
> > > > > 	Remove some redundant initializations
> > > > > 	Add regulators
> > > > > 	Make "sizes" a pointer
> > > > > 		Use struct v4l2_area instead of u32[2] array
> > > > > 		Remove the count for supported_modes[] and supported_codes[], instead use sentinel element at the end
> > > > > 		Consequently, update ox05b1s_enum_mbus_code, ox05b1s_enum_frame_size, ox05b1s_nearest_size, ox05b1s_find_code, to not use the count
> > > > > 	Remove .h files for modes, however did not move this code in the driver file but added a separate c file for all supported modes
> > > > > 	Refactor register lists to allow multiple register arrays per mode
> > > > > 	Use GPL-2.0-only instead of GPL-2.0
> > > > > 
> > > > >   drivers/media/i2c/Kconfig                 |   1 +
> > > > >   drivers/media/i2c/Makefile                |   1 +
> > > > >   drivers/media/i2c/ox05b1s/Kconfig         |  10 +
> > > > >   drivers/media/i2c/ox05b1s/Makefile        |   2 +
> > > > >   drivers/media/i2c/ox05b1s/ox05b1s.h       |  22 +
> > > > >   drivers/media/i2c/ox05b1s/ox05b1s_mipi.c  | 951 ++++++++++++++++++++++
> > > > >   drivers/media/i2c/ox05b1s/ox05b1s_modes.c |  77 ++
> > > > >   7 files changed, 1064 insertions(+)
> > > > >   create mode 100644 drivers/media/i2c/ox05b1s/Kconfig
> > > > >   create mode 100644 drivers/media/i2c/ox05b1s/Makefile
> > > > >   create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s.h
> > > > >   create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> > > > >   create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_modes.c
> > > > > 
> > > > [snip]
> > > > > diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> > > > > b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> > > > > new file mode 100644
> > > > > index 000000000000..1026216ddd5b
> > > > > --- /dev/null
> > > > > +++ b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
> > > > > @@ -0,0 +1,951 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/*
> > > > > + * A V4L2 driver for Omnivision OX05B1S RGB-IR camera.
> > > > > + * Copyright (C) 2024, NXP
> > > > > + *
> > > > > + * Inspired from Sony imx219, imx290, imx214 and imx334 camera drivers
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +#include <linux/clk.h>
> > > > > +#include <linux/pm_runtime.h>
> > > > > +#include <linux/regmap.h>
> > > > > +#include <linux/regulator/consumer.h>
> > > > > +#include <media/v4l2-cci.h>
> > > > > +#include <media/mipi-csi2.h>
> > > > > +#include <media/v4l2-ctrls.h>
> > > > > +#include <media/v4l2-device.h>
> > > > > +#include <media/v4l2-fwnode.h>
> > > > > +
> > > > > +#include "ox05b1s.h"
> > > > > +
> > > > > +#define OX05B1S_SENS_PAD_SOURCE	0
> > > > > +#define OX05B1S_SENS_PADS_NUM	1
> > > > > +
> > > > > +#define OX05B1S_REG_SW_STB		CCI_REG8(0x0100)
> > > > > +#define OX05B1S_REG_SW_RST		CCI_REG8(0x0103)
> > > > > +#define OX05B1S_REG_CHIP_ID		CCI_REG24(0x300a)
> > > > > +#define OX05B1S_REG_TIMING_HTS		CCI_REG16(0x380c)
> > > > > +#define OX05B1S_REG_TIMING_VTS		CCI_REG16(0x380e)
> > > > > +#define OX05B1S_REG_EXPOSURE		CCI_REG16(0x3501)
> > > > > +#define OX05B1S_REG_GAIN		CCI_REG16(0x3508)
> > > > There is a non-trivial overlap of registers between this driver and
> > > > ov9282.c which supports OV9281/OV9282 (1MP Mono).
> > > > 
> > > > There are two other Omnivision sensors, OV2311 (2MP Mono) and OV2312
> > > > (2MP 4x4 RGB-IR Bayer) with an even larger register overlap with OX05B1S
> > > > and OS08A20. Unfortunately those two have separate downstream drivers in
> > > > RPi and TI linux downstream trees respectively, and haven't yet been
> > > > posted upstream.
> > > Thanks for sharing this information, I was unaware. The question of
> > > how much similarity should two sensors share, in order to stay in the
> > > same driver, was also on my mind for some time, and I’d be glad to
> > > hear more opinions on it ;)
> > > 
> > Same here :)
> > 
> > > > It would be ideal to have a single driver for all of these Omnivision
> > > > sensors, or if not, at least a common C module that can implement the
> > > > shared functionality like gain, exposure, blanking (vts & hts) in a
> > > > single place, as this will make maintenance much easier.
> > > I would need to get more information on the sensors you mentioned in order
> > > to issue an informed opinion. So far, with the OX05B1S and OS08A20, I have
> > > found some small differences regarding exposure and digital gain registers,
> > > so the overlap is not perfect, I expect it will also not be a perfect
> > > overlap with the other sensors you mentioned.
> > > 
> > Sure, I had some experience with supporting OV2312 and OX05B1S in the
> > downstream TI linux tree, and while they share the registers for
> > exposure and gain, there are some other differences in features, aside
> > from the 2MP vs 5MP resolution.
> > 
> > > > My question here to you and the maintainers is, would it be okay to use
> > > > this driver as a baseline to integrate all these different sensors
> > > > together? And secondly, would you like to take a look at supporting
> > > > ov9282, so the other driver can be dropped?
> > > > 
> > > For the first question, I don't know what to say, and I cannot tell if
> > > we are far or close for this patch-set to be accepted. Also, I am
> > > unsure about how maintenance would go on a driver claiming to support
> > > a multitude of sensors, who could test them all, whenever something
> > > changes? Are you thinking to add ov2311/12 as other compatibles to
> > > this driver?
> > > 
> > While it would be ideal to have OV2312 support within this driver if
> > there is a significant register overlap, it might still require some
> > effort, as TI's downstream drivers for the RGBIR sensors capture two
> > streams with different exposure, gain and IR flash values, and different
> > MIPI CSI virtual channels, using the group hold functionality. Which
> > IIUC may be quite different from what your patches implement, and will
> > require adding streams/routing support so the userspace can configure.
> 
> You are not alone on this ;)
> 
> We have an internal solution for adding streams/routing support to this
> driver, we used it both for ox05b1s (AB mode with 2 pixel streams on
> separate virtual channels) and for os08a20 (staggered hdr mode with 2 pixel
> streams on separate virtual channels), and also a separate stream for
> embedded data (same VC but a different mipi data type). I did not post these
> patches because of 2 reasons:

Ah that's good to know that you also use AB mode, as combining the 
drivers makes even more sense now.

> 
> 1. We are waiting for internal pads to be accepted, and possibly the common
> raw sensor model

I wasn't aware of the raw sensor model series, will read up more on that 
as that also seems to have an easier way to represent RGB-Ir bayer 
formats.

> 
> 2. There are issues with the individual control (exposure, analog and
> digital gain) per exposure/context, with the current available standard
> controls. This is an entire topic on its own, maybe I should start a
> separate discussion thread on that.
> 

Yes individual controls, be it for internal pads or per-stream, will be 
a requirement for multi-stream sensor drivers. I have proposed a topic 
to discuss this with the rest of the community at the Media Summit being 
held on May 13 in Nice. [1]

Do you have plans to attend the summit? In any case, please feel free to 
start a RFC discussion thread on it :)

One idea is to move the controls in the v4l2_subdev_state, which would 
make it easier to specify pad/stream combinations, but I am not yet sure 
on what the userspace ioctls will look like for that.

[1] https://lore.kernel.org/linux-media/sbhsskq7kzrl5bkbqbijxszz7hfg34pjajgdmw23x7aseztyy3@26zcjwnjkpl4/

> > 
> > > I agree there is a great deal of similarity shared across many raw,
> > > mode-based sensor drivers, and it sounds good to have some common framework.
> > > Some steps were done with the common raw sensor model. I would definitely
> > > also like to hear more expert opinions on this.
> > > 
> > > For the second question, as of now, we do not have any of the sensors you
> > > mentioned, unfortunately. I could help in the future to test patches for
> > > this driver on the sensors that we already have, but cannot make any
> > > promises for what I do not have, best effort if we find these sensors in a
> > > form factor that will work for our boards.
> > > 
> > I agree, having a single maintainer would not be feasible given
> > different sensor modules may have incompatible connectors. But yes it
> > should be okay to provide a T-By tag or a Nack on the shared driver if a
> > patch breaks your particular hardware or usecase, similar to how other
> > popular sensor drivers are maintained like IMX219 or OV5640.
> Sounds ok to me, we cannot guarantee to test the other sensors, but having
> T-by tag from other users will hopefully cover it.
> > 

Thanks.

> > > > Anyway thanks again for your series, hopefully this will give a good
> > > > starting point for upstreaming OV2311 and OV2312 soon.
> > > > 
> > > I would like to know more about the OV2312 (RGB-Ir) sensor and if it
> > > has many similarities with OX05B1S.  What hardware are you using to
> > > test this sensor? And what interface to connect the sensor? We are
> > > working with MIPI-CSI on most imx boards, and also RPI on imx93.
> > For OV2312 I have used this FPDLink module [1] with the Arducam V3Link
> > board [2] that connects to the EVM using 22-pin FFC MIPI-CSI connector
> > that is pin-compatible with the RPi5 connector.
> > 
> > [1]: https://leopardimaging.com/wp-content/uploads/2024/03/LI-OV2312-FPDLINKIII_Datasheet.pdf
> > [2]: https://www.arducam.com/downloads/datasheet/Arducam_V3Link_Datasheet.pdf
> Thanks, we don't have any ser/des involved in the ox05b1s/os08a20 case, if
> we will ever get into the position to try ov2312, probably we will look for
> some solution for imx95-15x15 board, on the RPi connector (22-pin), cannot
> tell if it will work, one can never tell what may go wrong.
> > 

Makes sense.

> > >   Regards,
> > > 
> > > Mirela
> > PS: Your mail client broke the quotations on your reply. I have fixed
> > those here, but might be a good idea to double-check your client
> > settings.
> 
> Sorry about that, I may have edited the draft from both Thunderbird and
> Outlook. Hope this will be ok, sending from Thunderbird now as plain text
> only.
> 

No problem, this mail looks okay.

> Regards,
> 
> Mirela
> 

Thanks,
Jai

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [EXT] Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Mirela Rabulea]

Hi Jai,

On 28.03.2025 10:37, Jai Luthra wrote:
> On Mar 26, 2025 at 17:32:55 +0200, Mirela Rabulea wrote:
>> Hi Jai,
>>
>> On 25.03.2025 18:02, Jai Luthra wrote:
>>> On Mar 24, 2025 at 17:32:01 +0200, Mirela Rabulea wrote:
>>>> Hi Jai and all,
>>>>
>>>> On Mar 19, 2025 at 16:40:30 +0530, Jai Luthra wrote:
>>>>> Hi Mirela,
>>>>>
>>>>> Thanks a lot for your patch/series.
>>>>>
>>>>> On Mar 05, 2025 at 11:43:57 +0200, Mirela Rabulea wrote:
>>>>>> Add a v4l2 subdevice driver for the Omnivision OX05B1S RGB-IR sensor.
>>>>>>
>>>>>> The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an
>>>>>> active array size of 2592 x 1944.
>>>>>>
>>>>>> The following features are supported for OX05B1S:
>>>>>> - Manual exposure an gain control support
>>>>>> - vblank/hblank control support
>>>>>> - Supported resolution: 2592 x 1944 @ 30fps (SGRBG10)
>>>>>>
>>>>>> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
>>>>>> ---
>>>>>> Changes in v4:
>>>>>> 	Switch to Y media bus codes. The CFA pattern control will be implemented when patches get merged, or maybe separatelly as RFC?
>>>>>> 	Add pixel_rate member to mode struct, remove fps member. We do not have information how to calculate the pixel rate from the PLL parameters that can be made public.
>>>>>> 	Use register macros for the registers that are documented. User register group macros, where individual registers are not documented
>>>>>> 	Remove some uneeded local variable initialisations
>>>>>> 	Fix extra/missing spaces
>>>>>> 	Add missing ending \n
>>>>>> 	Use return -ENODEV & return 0 to ease reading
>>>>>> 	Rename retval to ret in probe for consistency
>>>>>> 	Use devm_mutex_init instead of mutex_init
>>>>>> 	Replace more dev_err's with dev_err_probe
>>>>>> 	Constify more structs
>>>>>> 	Remove some unneded ending commas after a terminator
>>>>>> 	Fix smatch error in ox05b1s_s_ctrl: error: typename in expression
>>>>>> 	Fix a seeries of smatch warnings like: warning: symbol 'ovx5b_init_setting_2592x1944' was not declared. Should it be static?
>>>>>> 	Shorten some more lines to 80 columns
>>>>>>
>>>>>> Changes in v3:
>>>>>> 	Use helpers from v4l2-cci.h (drop ox05b1s_write_reg, ox05b1s_read_reg, ox05b1s_set_hts/vts/exp/analog_gain, ox05b1s_regmap_config)
>>>>>> 	Don't hardcode timing registers: remove timing registers x_output_size/y_output_size from register configuration list, add them to ox05b1s_apply_current_mode
>>>>>> 	Remove HTS,VTS from register config list as they are written by HBLANK and VBLANK controls through __v4l2_ctrl_handler_setup
>>>>>> 	ox05b1s register config cleaning (remove all registers that were at their default value, and more, keep only what seems mandatory to be able to stream)
>>>>>> 	Use const for ox05b1s_supported_modes
>>>>>> 	Device should be silent on success, use dev_dbg.
>>>>>> 	Drop unneeded {}
>>>>>> 	Fixed an error introduced in v2 in ox05b1s_nearest_size (set_fmt for 4k BGGR12 mode was stuck)
>>>>>> 	Fix an issue in ox05b1s_set_fmt, the format was saved in subdev state only for _TRY, save it also for _ACTIVE
>>>>>>
>>>>>> Changes in v2:
>>>>>> 	Use dev_err_probe for missing clock, since it is now required property, and use NULL for devm_clk_get (no name for single clock), remove check for non NULL sensor->sensor_clk
>>>>>> 	Remove dev_err message for devm_regmap_init_i2c allocation error
>>>>>> 	Added spaces inside brackets, wrap lines to 80
>>>>>> 	Remove some redundant initializations
>>>>>> 	Add regulators
>>>>>> 	Make "sizes" a pointer
>>>>>> 		Use struct v4l2_area instead of u32[2] array
>>>>>> 		Remove the count for supported_modes[] and supported_codes[], instead use sentinel element at the end
>>>>>> 		Consequently, update ox05b1s_enum_mbus_code, ox05b1s_enum_frame_size, ox05b1s_nearest_size, ox05b1s_find_code, to not use the count
>>>>>> 	Remove .h files for modes, however did not move this code in the driver file but added a separate c file for all supported modes
>>>>>> 	Refactor register lists to allow multiple register arrays per mode
>>>>>> 	Use GPL-2.0-only instead of GPL-2.0
>>>>>>
>>>>>>    drivers/media/i2c/Kconfig                 |   1 +
>>>>>>    drivers/media/i2c/Makefile                |   1 +
>>>>>>    drivers/media/i2c/ox05b1s/Kconfig         |  10 +
>>>>>>    drivers/media/i2c/ox05b1s/Makefile        |   2 +
>>>>>>    drivers/media/i2c/ox05b1s/ox05b1s.h       |  22 +
>>>>>>    drivers/media/i2c/ox05b1s/ox05b1s_mipi.c  | 951 ++++++++++++++++++++++
>>>>>>    drivers/media/i2c/ox05b1s/ox05b1s_modes.c |  77 ++
>>>>>>    7 files changed, 1064 insertions(+)
>>>>>>    create mode 100644 drivers/media/i2c/ox05b1s/Kconfig
>>>>>>    create mode 100644 drivers/media/i2c/ox05b1s/Makefile
>>>>>>    create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s.h
>>>>>>    create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>>>>>>    create mode 100644 drivers/media/i2c/ox05b1s/ox05b1s_modes.c
>>>>>>
>>>>> [snip]
>>>>>> diff --git a/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>>>>>> b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..1026216ddd5b
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c
>>>>>> @@ -0,0 +1,951 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>>>> +/*
>>>>>> + * A V4L2 driver for Omnivision OX05B1S RGB-IR camera.
>>>>>> + * Copyright (C) 2024, NXP
>>>>>> + *
>>>>>> + * Inspired from Sony imx219, imx290, imx214 and imx334 camera drivers
>>>>>> + *
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/clk.h>
>>>>>> +#include <linux/pm_runtime.h>
>>>>>> +#include <linux/regmap.h>
>>>>>> +#include <linux/regulator/consumer.h>
>>>>>> +#include <media/v4l2-cci.h>
>>>>>> +#include <media/mipi-csi2.h>
>>>>>> +#include <media/v4l2-ctrls.h>
>>>>>> +#include <media/v4l2-device.h>
>>>>>> +#include <media/v4l2-fwnode.h>
>>>>>> +
>>>>>> +#include "ox05b1s.h"
>>>>>> +
>>>>>> +#define OX05B1S_SENS_PAD_SOURCE	0
>>>>>> +#define OX05B1S_SENS_PADS_NUM	1
>>>>>> +
>>>>>> +#define OX05B1S_REG_SW_STB		CCI_REG8(0x0100)
>>>>>> +#define OX05B1S_REG_SW_RST		CCI_REG8(0x0103)
>>>>>> +#define OX05B1S_REG_CHIP_ID		CCI_REG24(0x300a)
>>>>>> +#define OX05B1S_REG_TIMING_HTS		CCI_REG16(0x380c)
>>>>>> +#define OX05B1S_REG_TIMING_VTS		CCI_REG16(0x380e)
>>>>>> +#define OX05B1S_REG_EXPOSURE		CCI_REG16(0x3501)
>>>>>> +#define OX05B1S_REG_GAIN		CCI_REG16(0x3508)
>>>>> There is a non-trivial overlap of registers between this driver and
>>>>> ov9282.c which supports OV9281/OV9282 (1MP Mono).
>>>>>
>>>>> There are two other Omnivision sensors, OV2311 (2MP Mono) and OV2312
>>>>> (2MP 4x4 RGB-IR Bayer) with an even larger register overlap with OX05B1S
>>>>> and OS08A20. Unfortunately those two have separate downstream drivers in
>>>>> RPi and TI linux downstream trees respectively, and haven't yet been
>>>>> posted upstream.
>>>> Thanks for sharing this information, I was unaware. The question of
>>>> how much similarity should two sensors share, in order to stay in the
>>>> same driver, was also on my mind for some time, and I’d be glad to
>>>> hear more opinions on it ;)
>>>>
>>> Same here :)
>>>
>>>>> It would be ideal to have a single driver for all of these Omnivision
>>>>> sensors, or if not, at least a common C module that can implement the
>>>>> shared functionality like gain, exposure, blanking (vts & hts) in a
>>>>> single place, as this will make maintenance much easier.
>>>> I would need to get more information on the sensors you mentioned in order
>>>> to issue an informed opinion. So far, with the OX05B1S and OS08A20, I have
>>>> found some small differences regarding exposure and digital gain registers,
>>>> so the overlap is not perfect, I expect it will also not be a perfect
>>>> overlap with the other sensors you mentioned.
>>>>
>>> Sure, I had some experience with supporting OV2312 and OX05B1S in the
>>> downstream TI linux tree, and while they share the registers for
>>> exposure and gain, there are some other differences in features, aside
>>> from the 2MP vs 5MP resolution.
>>>
>>>>> My question here to you and the maintainers is, would it be okay to use
>>>>> this driver as a baseline to integrate all these different sensors
>>>>> together? And secondly, would you like to take a look at supporting
>>>>> ov9282, so the other driver can be dropped?
>>>>>
>>>> For the first question, I don't know what to say, and I cannot tell if
>>>> we are far or close for this patch-set to be accepted. Also, I am
>>>> unsure about how maintenance would go on a driver claiming to support
>>>> a multitude of sensors, who could test them all, whenever something
>>>> changes? Are you thinking to add ov2311/12 as other compatibles to
>>>> this driver?
>>>>
>>> While it would be ideal to have OV2312 support within this driver if
>>> there is a significant register overlap, it might still require some
>>> effort, as TI's downstream drivers for the RGBIR sensors capture two
>>> streams with different exposure, gain and IR flash values, and different
>>> MIPI CSI virtual channels, using the group hold functionality. Which
>>> IIUC may be quite different from what your patches implement, and will
>>> require adding streams/routing support so the userspace can configure.
>> You are not alone on this ;)
>>
>> We have an internal solution for adding streams/routing support to this
>> driver, we used it both for ox05b1s (AB mode with 2 pixel streams on
>> separate virtual channels) and for os08a20 (staggered hdr mode with 2 pixel
>> streams on separate virtual channels), and also a separate stream for
>> embedded data (same VC but a different mipi data type). I did not post these
>> patches because of 2 reasons:
> Ah that's good to know that you also use AB mode, as combining the
> drivers makes even more sense now.
>
>> 1. We are waiting for internal pads to be accepted, and possibly the common
>> raw sensor model
> I wasn't aware of the raw sensor model series, will read up more on that
> as that also seems to have an easier way to represent RGB-Ir bayer
> formats.
>
>> 2. There are issues with the individual control (exposure, analog and
>> digital gain) per exposure/context, with the current available standard
>> controls. This is an entire topic on its own, maybe I should start a
>> separate discussion thread on that.
>>
> Yes individual controls, be it for internal pads or per-stream, will be
> a requirement for multi-stream sensor drivers. I have proposed a topic
> to discuss this with the rest of the community at the Media Summit being
> held on May 13 in Nice. [1]
>
> Do you have plans to attend the summit? In any case, please feel free to
> start a RFC discussion thread on it :)
>
> One idea is to move the controls in the v4l2_subdev_state, which would
> make it easier to specify pad/stream combinations, but I am not yet sure
> on what the userspace ioctls will look like for that.
>
> [1] https://lore.kernel.org/linux-media/sbhsskq7kzrl5bkbqbijxszz7hfg34pjajgdmw23x7aseztyy3@26zcjwnjkpl4/

Thanks for letting me know, yes that is interesting, I will participate, 
if not in person, at least remotely. I will reply to the indicated 
thread, after we decide on our agenda.

Regards,

Mirela

>
>>>> I agree there is a great deal of similarity shared across many raw,
>>>> mode-based sensor drivers, and it sounds good to have some common framework.
>>>> Some steps were done with the common raw sensor model. I would definitely
>>>> also like to hear more expert opinions on this.
>>>>
>>>> For the second question, as of now, we do not have any of the sensors you
>>>> mentioned, unfortunately. I could help in the future to test patches for
>>>> this driver on the sensors that we already have, but cannot make any
>>>> promises for what I do not have, best effort if we find these sensors in a
>>>> form factor that will work for our boards.
>>>>
>>> I agree, having a single maintainer would not be feasible given
>>> different sensor modules may have incompatible connectors. But yes it
>>> should be okay to provide a T-By tag or a Nack on the shared driver if a
>>> patch breaks your particular hardware or usecase, similar to how other
>>> popular sensor drivers are maintained like IMX219 or OV5640.
>> Sounds ok to me, we cannot guarantee to test the other sensors, but having
>> T-by tag from other users will hopefully cover it.
> Thanks.
>
>>>>> Anyway thanks again for your series, hopefully this will give a good
>>>>> starting point for upstreaming OV2311 and OV2312 soon.
>>>>>
>>>> I would like to know more about the OV2312 (RGB-Ir) sensor and if it
>>>> has many similarities with OX05B1S.  What hardware are you using to
>>>> test this sensor? And what interface to connect the sensor? We are
>>>> working with MIPI-CSI on most imx boards, and also RPI on imx93.
>>> For OV2312 I have used this FPDLink module [1] with the Arducam V3Link
>>> board [2] that connects to the EVM using 22-pin FFC MIPI-CSI connector
>>> that is pin-compatible with the RPi5 connector.
>>>
>>> [1]: https://leopardimaging.com/wp-content/uploads/2024/03/LI-OV2312-FPDLINKIII_Datasheet.pdf
>>> [2]: https://www.arducam.com/downloads/datasheet/Arducam_V3Link_Datasheet.pdf
>> Thanks, we don't have any ser/des involved in the ox05b1s/os08a20 case, if
>> we will ever get into the position to try ov2312, probably we will look for
>> some solution for imx95-15x15 board, on the RPi connector (22-pin), cannot
>> tell if it will work, one can never tell what may go wrong.
> Makes sense.
>
>>>>    Regards,
>>>>
>>>> Mirela
>>> PS: Your mail client broke the quotations on your reply. I have fixed
>>> those here, but might be a good idea to double-check your client
>>> settings.
>> Sorry about that, I may have edited the draft from both Thunderbird and
>> Outlook. Hope this will be ok, sending from Thunderbird now as plain text
>> only.
>>
> No problem, this mail looks okay.
>
>> Regards,
>>
>> Mirela
>>
> Thanks,
> Jai

Re: [PATCH v4 1/4] dt-bindings: media: i2c: Add OX05B1S sensor

[Sakari Ailus]

Hi Mirela,

On Wed, Mar 05, 2025 at 11:43:56AM +0200, Mirela Rabulea wrote:
> Add bindings for Omnivision OX05B1S sensor.
> Also add compatible for Omnivision OS08A20 sensor.
> 
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> 
> Changes in v4:
> 	Collect Reviewed-by
> 
> Changes in v3:
> 	Use unevaluatedProperties: false and drop orientation/rotation
> 	Drop items and keep alphabetical order in compatible property
> 	Shorten the description for reset_gpio
> 	Make the supplies required.
> 	Use generic node name (camera instead of ox05b1s)
> 
> Changes in v2:
> 	Small updates on description
> 	Update subject, drop "bindings" and "driver"
> 	Just one binding patch (squash os08a20 bindings)
> 	Re-flow to 80 columns.
> 	Drop clock name (not needed in case of single clock)
> 	Make the clock required property, strictly from sensor module point of view, it is mandatory (will use a fixed clock for nxp board)
> 	Add regulators: avdd, dvdd, dovdd
> 	Add $ref: /schemas/media/video-interface-devices.yaml
> 	Drop assigned-clock* properties (defined in clocks.yaml)
> 	Keep "additionalProperties : false" and orientation/rotation (unevaluatedProperties: false was suggested, but only orientation/rotation are needed from video-interface-devices.yaml)
> 	Include assigned-clock* in the example, for completeness sake (although it was also suggested to omit them)
> 
>  .../bindings/media/i2c/ovti,ox05b1s.yaml      | 119 ++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
> new file mode 100644
> index 000000000000..9f35b4e67bea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ox05b1s.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Omnivision OX05B1S Image Sensor
> +
> +maintainers:
> +  - Mirela Rabulea <mirela.rabulea@nxp.com>
> +
> +description:
> +  The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an active
> +  array size of 2592 x 1944. It is programmable through I2C interface.
> +  Image data is available via MIPI CSI-2 serial data output.
> +
> +allOf:
> +  - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ovti,os08a20
> +      - ovti,ox05b1s

The bindings only describe ox05b1s. How are the two different?

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description: Input clock (24 MHz)
> +    maxItems: 1

Is this really a hardware limitation, or what the driver is currently
limited to? In the former case, I'd include the range.

> +
> +  reset-gpios:
> +    description: Active low XSHUTDOWN pin
> +    maxItems: 1
> +
> +  avdd-supply:
> +    description: Power for analog circuit (2.8V)
> +
> +  dovdd-supply:
> +    description: Power for I/O circuit (1.8V)
> +
> +  dvdd-supply:
> +    description: Power for digital circuit (1.2V)
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    additionalProperties: false
> +    description: MIPI CSI-2 transmitter port
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            anyOf:
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +                  - const: 3
> +                  - const: 4
> +        required:
> +          - data-lanes
> +
> +    required:
> +      - endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - port
> +  - avdd-supply
> +  - dovdd-supply
> +  - dvdd-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        camera@36 {
> +            compatible = "ovti,ox05b1s";
> +            reg = <0x36>;
> +            clocks = <&ox05b1s_clk>;
> +
> +            assigned-clocks = <&ox05b1s_clk>;
> +            assigned-clock-parents = <&ox05b1s_clk_parent>;
> +            assigned-clock-rates = <24000000>;
> +
> +            reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
> +
> +            avdd-supply = <&camera_avdd_2v8>;
> +            dovdd-supply = <&camera_dovdd_1v8>;
> +            dvdd-supply = <&camera_dvdd_1v2>;
> +
> +            orientation = <2>;
> +            rotation = <0>;
> +
> +            port {
> +                ox05b1s_mipi_0_ep: endpoint {
> +                    remote-endpoint = <&mipi_csi0_ep>;
> +                    data-lanes = <1 2 3 4>;
> +                };
> +            };
> +        };
> +    };
> +...

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Sakari Ailus]

Hi Mirela,

On 3/5/25 11:43, Mirela Rabulea wrote:
> +struct ox05b1s_reg {
> +	u32 addr;
> +	u32 data;
> +};

Could you use struct reg_sequence instead, please?

-- 
Sakari Ailus

Re: [EXT] Re: [PATCH v4 1/4] dt-bindings: media: i2c: Add OX05B1S sensor

[Mirela Rabulea]

Hi Sakari,

On 6/28/25 21:27, Sakari Ailus wrote:
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
>
>
> Hi Mirela,
>
> On Wed, Mar 05, 2025 at 11:43:56AM +0200, Mirela Rabulea wrote:
>> Add bindings for Omnivision OX05B1S sensor.
>> Also add compatible for Omnivision OS08A20 sensor.
>>
>> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>
>> Changes in v4:
>>        Collect Reviewed-by
>>
>> Changes in v3:
>>        Use unevaluatedProperties: false and drop orientation/rotation
>>        Drop items and keep alphabetical order in compatible property
>>        Shorten the description for reset_gpio
>>        Make the supplies required.
>>        Use generic node name (camera instead of ox05b1s)
>>
>> Changes in v2:
>>        Small updates on description
>>        Update subject, drop "bindings" and "driver"
>>        Just one binding patch (squash os08a20 bindings)
>>        Re-flow to 80 columns.
>>        Drop clock name (not needed in case of single clock)
>>        Make the clock required property, strictly from sensor module point of view, it is mandatory (will use a fixed clock for nxp board)
>>        Add regulators: avdd, dvdd, dovdd
>>        Add $ref: /schemas/media/video-interface-devices.yaml
>>        Drop assigned-clock* properties (defined in clocks.yaml)
>>        Keep "additionalProperties : false" and orientation/rotation (unevaluatedProperties: false was suggested, but only orientation/rotation are needed from video-interface-devices.yaml)
>>        Include assigned-clock* in the example, for completeness sake (although it was also suggested to omit them)
>>
>>   .../bindings/media/i2c/ovti,ox05b1s.yaml      | 119 ++++++++++++++++++
>>   1 file changed, 119 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
>> new file mode 100644
>> index 000000000000..9f35b4e67bea
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.yaml
>> @@ -0,0 +1,119 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2024 NXP
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/i2c/ovti,ox05b1s.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Omnivision OX05B1S Image Sensor
>> +
>> +maintainers:
>> +  - Mirela Rabulea <mirela.rabulea@nxp.com>
>> +
>> +description:
>> +  The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an active
>> +  array size of 2592 x 1944. It is programmable through I2C interface.
>> +  Image data is available via MIPI CSI-2 serial data output.
>> +
>> +allOf:
>> +  - $ref: /schemas/media/video-interface-devices.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ovti,os08a20
>> +      - ovti,ox05b1s
> The bindings only describe ox05b1s. How are the two different?

Indeed, thanks for pointing out. I will update the description in _v5 
like this, to include also os08a20:

The Omnivision OX05B1S is a 1/2.5-Inch CMOS image sensor with an active
  array size of 2592 x 1944 and raw RGB-Ir output. It is programmable 
through
  I2C interface. Image data is available via MIPI CSI-2 serial data output.
  The Omnivision OS08A20 is a 1/1.8-Inch CMOS image sensor with an active
  array size of 3840 x 2160, raw RGB output and 2-exposure staggered HDR
  support.

>
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    description: Input clock (24 MHz)
>> +    maxItems: 1
> Is this really a hardware limitation, or what the driver is currently
> limited to? In the former case, I'd include the range.
No, hardware is not limited to 24 MHz. I will include a range like this:

description: Input clock (6-64 MHz for OX05B1S, 6-27 MHz for OS08A20)


Thanks for feedback,

Mirela

>
>> +
>> +  reset-gpios:
>> +    description: Active low XSHUTDOWN pin
>> +    maxItems: 1
>> +
>> +  avdd-supply:
>> +    description: Power for analog circuit (2.8V)
>> +
>> +  dovdd-supply:
>> +    description: Power for I/O circuit (1.8V)
>> +
>> +  dvdd-supply:
>> +    description: Power for digital circuit (1.2V)
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +    additionalProperties: false
>> +    description: MIPI CSI-2 transmitter port
>> +
>> +    properties:
>> +      endpoint:
>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          data-lanes:
>> +            anyOf:
>> +              - items:
>> +                  - const: 1
>> +                  - const: 2
>> +              - items:
>> +                  - const: 1
>> +                  - const: 2
>> +                  - const: 3
>> +                  - const: 4
>> +        required:
>> +          - data-lanes
>> +
>> +    required:
>> +      - endpoint
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - port
>> +  - avdd-supply
>> +  - dovdd-supply
>> +  - dvdd-supply
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        camera@36 {
>> +            compatible = "ovti,ox05b1s";
>> +            reg = <0x36>;
>> +            clocks = <&ox05b1s_clk>;
>> +
>> +            assigned-clocks = <&ox05b1s_clk>;
>> +            assigned-clock-parents = <&ox05b1s_clk_parent>;
>> +            assigned-clock-rates = <24000000>;
>> +
>> +            reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
>> +
>> +            avdd-supply = <&camera_avdd_2v8>;
>> +            dovdd-supply = <&camera_dovdd_1v8>;
>> +            dvdd-supply = <&camera_dvdd_1v2>;
>> +
>> +            orientation = <2>;
>> +            rotation = <0>;
>> +
>> +            port {
>> +                ox05b1s_mipi_0_ep: endpoint {
>> +                    remote-endpoint = <&mipi_csi0_ep>;
>> +                    data-lanes = <1 2 3 4>;
>> +                };
>> +            };
>> +        };
>> +    };
>> +...
> --
> Kind regards,
>
> Sakari Ailus

Re: [EXT] Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Mirela Rabulea]

Hi Sakari & all,

On 6/29/25 11:30, Sakari Ailus wrote:
> Caution: This is an external email. Please take care when clicking 
> links or opening attachments. When in doubt, report the message using 
> the 'Report this email' button
>
>
> Hi Mirela,
>
> On 3/5/25 11:43, Mirela Rabulea wrote:
>> +struct ox05b1s_reg {
>> +     u32 addr;
>> +     u32 data;
>> +};
>
> Could you use struct reg_sequence instead, please?

Yes, sure, thanks for the hint.

Any other suggestions, anyone, before I send the next version?

Thanks,

Mirela

>
> -- 
> Sakari Ailus

Re: [EXT] Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Sakari Ailus]

Hi Mirela,

On Tue, Jul 08, 2025 at 08:09:25PM +0300, Mirela Rabulea wrote:
> Hi Sakari & all,
> 
> On 6/29/25 11:30, Sakari Ailus wrote:
> > Caution: This is an external email. Please take care when clicking links
> > or opening attachments. When in doubt, report the message using the
> > 'Report this email' button
> > 
> > 
> > Hi Mirela,
> > 
> > On 3/5/25 11:43, Mirela Rabulea wrote:
> > > +struct ox05b1s_reg {
> > > +     u32 addr;
> > > +     u32 data;
> > > +};
> > 
> > Could you use struct reg_sequence instead, please?
> 
> Yes, sure, thanks for the hint.
> 
> Any other suggestions, anyone, before I send the next version?

Do you intend to align this with the common raw sensor model? It may still
take a while until we get those patches merged though, but the upside would
be there's on concern of backwards compatibility. We should discuss how to
implement HDR with that actually (Laurent is cc'd).

-- 
Regards,

Sakari Ailus

Re: [EXT] Re: [PATCH v4 2/4] media: ox05b1s: Add omnivision OX05B1S raw sensor driver

[Mirela Rabulea]

Hi Sakari,

On 7/9/25 02:43, Sakari Ailus wrote:
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
>
>
> Hi Mirela,
>
> On Tue, Jul 08, 2025 at 08:09:25PM +0300, Mirela Rabulea wrote:
>> Hi Sakari & all,
>>
>> On 6/29/25 11:30, Sakari Ailus wrote:
>>> Caution: This is an external email. Please take care when clicking links
>>> or opening attachments. When in doubt, report the message using the
>>> 'Report this email' button
>>>
>>>
>>> Hi Mirela,
>>>
>>> On 3/5/25 11:43, Mirela Rabulea wrote:
>>>> +struct ox05b1s_reg {
>>>> +     u32 addr;
>>>> +     u32 data;
>>>> +};
>>> Could you use struct reg_sequence instead, please?
>> Yes, sure, thanks for the hint.
>>
>> Any other suggestions, anyone, before I send the next version?
> Do you intend to align this with the common raw sensor model? It may still
> take a while until we get those patches merged though, but the upside would
> be there's on concern of backwards compatibility. We should discuss how to
> implement HDR with that actually (Laurent is cc'd).

Yes, I will align with the common raw sensor model. By that I mean I 
will use generic Y formats and the CFA pattern control (already switched 
to Y formats in this  v4, did not experiment yet the new control) and I 
can add streams and routes as soon as internal pads are merged, we 
already experimented with that both for os08a20 HDR and for ox05b1s 
context switching, both using multiple captures on 2 virtual channels. 
I'm also planning to send an RFC for standard controls as arrays for 
exposure and gains, as discussed during the media summit, that's 
currently under internal review.

I'm open for discussions.

For this driver, up until this version, the HDR support is minimal, it 
is just enabled/disabled and tied to the existing control 
V4L2_CID_HDR_SENSOR_MODE, and still only 1 VC is exposed. When we will 
have internal pads and streams and routes, we can toggle the HDR control 
depending on what routes are enabled, already experimented with that 
(the HDR control can in this case be optional).

I saw you sent v10 for generic line based metadata and internal pads, I 
started looking into it, large patch-set ;)

Regards,

Mirela

>
> --
> Regards,
>
> Sakari Ailus