Re: [PATCH 2/2] media: i2c: Add OmniVision OG0VE1B camera sensor

Hi Tarang.

On 8/21/25 09:52, Tarang Raval wrote:

Hi Vladimir,

quoted

OmniVision OG0VE1B is a monochrome image sensor, which produces frames in
8/10-bit raw output format and supports 640x480, 400x400, 200x200 and
100x100 output image resolution modes.
  
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
  MAINTAINERS                 |   8 +
  drivers/media/i2c/Kconfig   |  10 +
  drivers/media/i2c/Makefile  |   1 +
  drivers/media/i2c/og0ve1b.c | 858 ++++++++++++++++++++++++++++++++++++
  4 files changed, 877 insertions(+)
  create mode 100644 drivers/media/i2c/og0ve1b.c
  

...

quoted

+static const struct cci_reg_sequence og0ve1b_640x480_120fps_mode[] = {
+       { CCI_REG8(0x30a0), 0x02 },
+       { CCI_REG8(0x30a1), 0x00 },
+       { CCI_REG8(0x30a2), 0x48 },
+       { CCI_REG8(0x30a3), 0x34 },
+       { CCI_REG8(0x30a4), 0xf7 },
+       { CCI_REG8(0x30a5), 0x00 },
+       { CCI_REG8(0x3082), 0x32 },
+       { CCI_REG8(0x3083), 0x01 },
+       { CCI_REG8(0x301c), 0xf0 },
+       { CCI_REG8(0x301e), 0x0b },
+       { CCI_REG8(0x3106), 0x10 },
+       { CCI_REG8(0x3708), 0x77 },
+       { CCI_REG8(0x3709), 0xf8 },
+       { CCI_REG8(0x3717), 0x00 },
+       { CCI_REG8(0x3782), 0x00 },
+       { CCI_REG8(0x3783), 0x47 },
+       { CCI_REG8(0x37a2), 0x00 },
+       { CCI_REG8(0x3503), 0x07 },
+       { CCI_REG8(0x3509), 0x10 },
+       { CCI_REG8(0x3600), 0x83 },
+       { CCI_REG8(0x3601), 0x21 },
+       { CCI_REG8(0x3602), 0xf1 },
+       { CCI_REG8(0x360a), 0x18 },
+       { CCI_REG8(0x360e), 0xb3 },
+       { CCI_REG8(0x3613), 0x20 },
+       { CCI_REG8(0x366a), 0x78 },
+       { CCI_REG8(0x3706), 0x63 },
+       { CCI_REG8(0x3713), 0x00 },
+       { CCI_REG8(0x3716), 0xb0 },
+       { CCI_REG8(0x37a1), 0x38 },
+       { CCI_REG8(0x3800), 0x00 },
+       { CCI_REG8(0x3801), 0x04 },
+       { CCI_REG8(0x3802), 0x00 },
+       { CCI_REG8(0x3803), 0x04 },
+       { CCI_REG8(0x3804), 0x02 },
+       { CCI_REG8(0x3805), 0x8b },
+       { CCI_REG8(0x3806), 0x01 },
+       { CCI_REG8(0x3807), 0xeb },
+       { CCI_REG8(0x3808), 0x02 },     /* output width */
+       { CCI_REG8(0x3809), 0x80 },
+       { CCI_REG8(0x380a), 0x01 },     /* output height */
+       { CCI_REG8(0x380b), 0xe0 },
+       { CCI_REG8(0x380c), 0x03 },     /* horizontal timing size */
+       { CCI_REG8(0x380d), 0x18 },
+       { CCI_REG8(0x380e), 0x02 },     /* vertical timing size */

Do you have the datasheet for this sensor?

I don't have the datasheet for this sensor, unfortunately.

If so, it would be better to divide this array into two parts.
Currently, all of these registers are not related to the mode.
One part should be for common registers, and the other for
mode-related registers.

I agree, unfortunatey I have no option even to trace another mode
configuration sequence, of course some of the register values could
be guessed, but on practice it tends to be too tiresome.

Thank yo for review!

-- 
Best wishes,
Vladimir