All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Bozik <robertbozik@gmail.com>
To: sakari.ailus@linux.intel.com
Cc: linux-media@vger.kernel.org, mchehab@kernel.org, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	antti.laakso@linux.intel.com
Subject: Re: [PATCH v4 2/3] media: i2c: Add driver for OmniVision OV32C4
Date: Mon,  5 Oct 2026 12:14:46 +0200	[thread overview]
Message-ID: <20261005101447.15264-1-robertbozik@gmail.com> (raw)
In-Reply-To: <asNsiKal6GgcqUSK@kekkonen.localdomain>

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

  reply	other threads:[~2026-10-05 10:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-05  7:10 [PATCH v4 0/3] media: Add OmniVision OV32C4 sensor driver Robert Bozik
2026-10-05  7:10 ` [PATCH v4 1/3] dt-bindings: media: i2c: Add OmniVision OV32C4 Robert Bozik
2026-10-05  8:11   ` Sakari Ailus
2026-10-05  8:36     ` Robert Bozik
2026-10-05 11:02     ` Conor Dooley
2026-10-05 12:01       ` Robert Bozik
2026-10-05  7:10 ` [PATCH v4 2/3] media: i2c: Add driver for " Robert Bozik
2026-10-05  9:23   ` Sakari Ailus
2026-10-05 10:14     ` Robert Bozik [this message]
2026-10-05 10:36       ` Sakari Ailus
2026-10-05 10:46         ` Robert Bozik
2026-10-05  7:10 ` [PATCH v4 3/3] media: ipu-bridge: Add " Robert Bozik
2026-10-05  8:04 ` [PATCH v4 0/3] media: Add OmniVision OV32C4 sensor driver Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261005101447.15264-1-robertbozik@gmail.com \
    --to=robertbozik@gmail.com \
    --cc=antti.laakso@linux.intel.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=robh@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.