Re: [PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Sergey Lebedev <lsa.uz@pm.me>]

Benjamin,

Yes — it is a fork of yours, and André says so himself in the header he
kept:

  https://github.com/AndreGilerson/linux-surface-kernel
  branch v7.0-surface-devel, drivers/media/i2c/vd55g0.c

  // SPDX-License-Identifier: GPL-2.0
  /*
   * Driver for VD55G0 global shutter sensor family driver
   *
   * Copyright (C) 2024 STMicroelectronics SA
   *
   * Adapter with small changes by André Gilerson
   */

Against your master it is 127 changed lines out of 2114, and all 59
vd55g0_* function names are the same set — nothing added, nothing removed.
vd55g0_patches.h differs in four places: his attribution line, uint ->
unsigned int, and two structs made static. The firmware arrays are
byte-identical, cut1_patch 6868 bytes and cut2_patch 260.

That last point matters for this series. The blob I wrote to
/lib/firmware/vd55g0-cut1.bin to get past the missing-firmware failure was
taken from his header, so it is your firmware, published by you under
GPL-2.0 in your own repository. Peter's driver needs those bytes and
cannot get them from anywhere in-tree; you are the one who can say how
they should reach linux-firmware, or whether VD55G0 should keep a built-in
array the way VD55G1 does.

What the 127 lines actually change, since it is a fair summary of what a
laptop needed that your driver did not have:

  - ACPI. Yours is DT-only; his adds acpi.h and an ACPI match on SMO55F0.
  - Supply names. Yours asks for VCORE, VDDIO and VANA. His asks for avdd
    and dovdd, two rather than three, with the comment that INT3472 on the
    Surface Pro 11 provides GPIO type 0x0B as avdd and 0x10 as dovdd.
  - SGRBG aliases beside the mono formats, commented "allows IPU7 Bayer
    pipeline".
  - illuminator GPIO handling.
  - an unknown revision warns and continues instead of returning -EINVAL.

The second of those explains something I reported yesterday and had not
chased: on this machine Peter's 4/7 leaves `supply vcore not found, using
dummy regulator`. His mapping gives the SMO55F0 GPIOs the names your
driver wants, while the machine's INT3472 appears to describe what André
named avdd and dovdd. One of the two mappings does not match this
hardware, and I have not established which — I can measure it here if that
is useful.

Two things I should retire from my own report, having now seen the
off-list exchange you added back. Peter had already withdrawn the rename
on 4 September, so the libcamera breakage I raised - the sensor-properties
and helper lookups keying on the entity name, which vd55g1 machines have
today - does not arise in a v2 that keeps the name. And the metadata point
you make to him is the same one from the other side: unknown to libcamera,
the sensor streams but on defaults.

What is not retired is the firmware. Nothing in the exchange touches it,
and it is the one thing that stops anyone but Peter using VD55G0 at all.

For contrast rather than contradiction: he reports the sensor is not quite
usable on his Surface Pro 9 because of IPU6 stream problems, short lines
and frames cut off when the CPU idles. I saw none of that on IPU7 - it
probed, bound and registered, and streamed 60 frames at 30 fps. Different
bridge, different result, which may be worth knowing when the two are
compared.

I should be clear that I am not speaking for André. I have written to him
separately about upstreaming his IMX681 driver, he answered that mainline
is his goal, and he is away until 28 September. If you want to reach him
directly his address is in the MODULE_AUTHOR line of that file.

Sergey