Re: [PATCH v2] media: ipu-bridge: add the OV13858 rear sensor

[Fernando Rimoli <fernandorimoli11@gmail.com>]

Hi Sergey,

On Sun, Sep 13, 2026 at 02:20:40PM +0000, Sergey Lebedev wrote:
> +	/* Omnivision OV13858 */
> +	IPU_SENSOR_CONFIG("OVTID858", 2, 540000000, 270000000),

I have the same sensor on the other side of the IPU generation gap, a
Surface Pro 9, IPU6 Alder Lake-P, with OVTID858 as the rear sensor and
OVTI5693 as the front one. Your entry works there too:

Tested-by: Fernando Rimoli <fernandorimoli11@gmail.com> # Surface Pro 9, IPU6 Alder Lake-P (8086:465d)

Method, since this machine does not boot media/next as it stands: I took
the tree it does boot and replaced my own OVTID858 entry with yours, so 
your line is the only difference from a known-good build. Your
patch also applies clean to media/next on its own. Only ipu-bridge needed
rebuilding, as the entry does not touch the header, so the exported CRCs
are unchanged and the installed intel-ipu6 and intel-ipu6-isys load against
it untouched.

Cold boot:

  intel-ipu6 0000:00:05.0: Found supported sensor OVTI5693:00
  intel-ipu6 0000:00:05.0: Found supported sensor OVTID858:00
  intel-ipu6 0000:00:05.0: Found supported sensor SMO55F0:00
  intel-ipu6 0000:00:05.0: Connected 3 cameras

Rear sensor at its native 4224x3136 SGRBG10 off the ISYS node, 29.95 fps
over 60 frames, and the operating point is identical to my ascending
ordering, which is the part worth checking:

  link_frequency  value=0 (540000000)
  pixel_rate      432000000

To show the path carries pixels and not just buffers I used the sensor's
own vertical colour bar: stddev 479.5 on a 0-1023 range, min/max 64/1023,
bar edge where it belongs, and no CSI-2 errors on that run with the log
cleared first. Front and IR cameras unaffected.

One thing that may save you a review round is that ov13858 never reads the
link-frequencies property at all. It calls v4l2_fwnode_device_parse() for
the device properties and never v4l2_fwnode_endpoint_alloc_parse(), so the
array the bridge publishes is not consumed by this driver. It exposes its
own menu instead, and the receiver takes V4L2_CID_LINK_FREQ from that:

  link_frequency  0x009f0901 (intmenu): min=0 max=1 value=0 (540000000)
  pixel_rate      0x009f0902 (int64)  : value=432000000

That reads 540 MHz here even though my local entry happens to list 270
first, which is how I know the order is inert. Useful if anyone asks why
you ordered them that way, the other multi-frequency entries in that table
happen to be ascending.

What I am mainly writing about is your remark that the sensor also needs
power sequencing the in-tree driver does not do, and that it is a separate
patch. I have an ov13858 patch parked that touches the same probe path, and
I would rather we did not send two overlapping fixes into the same
function.

Mine retries the chip-id read in ov13858_identify_module(), five attempts
5 ms apart. Without it the rear camera fails probe with -EIO on every boot
here. The cause I identified is not a missing power sequence: on this
machine OVTID858 and the IR sensor sit on the same I2C controller
(ov13858 2-0010 and vd55g 2-0060), and the first chip-id read gets
corrupted by bus and power activity from the neighbour, so it returns
garbage rather than timing out. Reverting the retry on its own reproduces
the failure.

So, what does your power-sequencing patch do, and against which symptom?
If it orders or delays the sensor's own power-up then the two are probably
independent and both wanted, in which case I should say so in my commit
message so that it does not read as a duplicate of yours. If it subsumes
what I am seeing, I would rather drop mine and test yours. Either answer
suits me, I would just like to know before either of us sends.

I had held mine back because the failure was not reproducible on mainline,
there being no bridge entry to enumerate the sensor in the first place.
Your patch removes that objection, which is great.

On your rotation patch, separately: the SP9 rear sensor is mounted inverted
as well and its SSDB reports 0, so that machine wants the same treatment. I
will send the SP9 entries myself. Your DMI_SYS_VENDOR match is right for
Surface, for what it is worth: this SP9 reports "Microsoft Corporation"
with no leading space, so the exact match holds across two Surface
generations.

Thanks for doing this.

Fernando