Re: [PATCH v4 2/3] media: i2c: Add driver for OmniVision OV32C4

[Robert Bozik]

Hi Sakari,

Thank you for the review.

On Mon, Oct 05, 2026 at 12:23:20PM +0300, Sakari Ailus wrote:
> > +	{ 0x6ad0, 0x00 },
> > +	{ 0x6ad1, 0x75 },
> 
> Are there any gaps in this deluge of 0x0 and 0x75? If not, could you write
> it programmatically rather than using a huge array?

Two contiguous ranges, 0x6ad0-0x6bef and 0x6c00-0x707f, with one gap of
16 bytes between them; every even address holds 0x00 and the odd one
0x75, so it is 720 16-bit registers set to 0x0075. v5 writes them from a
loop and the table shrinks to 345 entries. I'll verify the stream and
the image after the change, since it alters the bus transactions.

> > +	ret = acpi_dev_get_resources(adev, &resources, ov32c4_i2c_res_cb, &ctx);
> 
> The presence of additional chips like VCM is very much dependent on the
> module, and I think we should have parsing of the I²C address outside the
> sensor driver.
> 
> One option could be to stuff it into the reg property in the ipu-bridge,
> that way it'd work the same way for the driver on both DT and ACPI. In the
> ipu-bridge, I'd use a static value and provide the reg property for this
> sensor only (based on _HID). i2c_new_ancillary_device() will only use OF so
> the driver will need to dig the address manually still.

Agreed, that is better. For v5 I'd give ipu-bridge a small table of
sensors whose second I2C address belongs to the sensor itself,
{ "OVTI32C4", 0x3e }; for a sensor in it the bridge adds reg = <main,
aon> to the sensor's software node and does not instantiate the VCM
from SSDB - so the no-VCM exception of patch 3 folds into the same
table. The driver reads reg with device_property_read_u32_array() on
both DT and ACPI and the _CRS walk goes away with its CONFIG_ACPI
guard. The property fits into the existing dev_properties slot that
lens-focus uses when there is a VCM, so no change to the header.

The rest will be in v5 as suggested - the register names, the RGB/IR
comments, the function name, the error label and the comments; the HTS
define was unused and goes too.

Thanks,
Robert