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

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

[Sergey Lebedev]

Peter,

Tested on a second machine: Surface Pro 11 for Business, Intel Lunar Lake,
IPU7 — same SMO55F0 HID as your Surface Pro 9, one IPU generation later.
Two results worth having before you respin, one good and one blocking.

The good one: the series applies and builds clean, and the sensor works on
IPU7. All seven patches applied in order to an Ubuntu 7.0.0 tree with no
conflicts (its vd55g1.c is 2061 lines, exactly what your diffstat deletes),
and media/i2c, media/pci/intel and int3472 rebuilt without a warning.
With firmware present the driver probes, the bridge finds the sensor and
the subdev registers:

  intel-ipu7: Found supported sensor SMO55F0:00
  intel_ipu7_isys: bind vd55g 1-0060 nlanes is 1 port is 1
  entity 233: vd55g 1-0060 (1 pad, 1 link, 0 routes)

So nothing here is IPU6-specific. That is the part I could add that you
could not.

The blocking one: without that firmware nobody can use VD55G0 at all.

  vd55g i2c-SMO55F0:00: Direct firmware load for vd55g0-cut1.bin failed
                        with error -2
  vd55g i2c-SMO55F0:00: Failed to load required firmware vd55g0-cut1.bin: -2

Probe fails and the camera disappears. The driver has a fallback for
exactly this case, but only the older part gets one:

  static const struct firmware vd55g1_builtin_fw = {
          .data = vd55g1_patch_array, .size = ARRAY_SIZE(vd55g1_patch_array)
  };
  ...
  if (ret == -ENOENT && fw_info->builtin_fw)

vd55g0_fw_maps[] sets fw_name and leaves builtin_fw NULL, and neither
vd55g0-cut1.bin nor vd55g0-cut2.bin is in linux-firmware. So the series
works for you and for nobody else, which I assume is not the intent. Either
a built-in like VD55G1's, or the files submitted to linux-firmware, or at
minimum a note in the cover letter saying where they come from.

To get past it I used the same ST patch arrays that André Gilerson's
out-of-tree VD55G0 driver compiles in (vd55g0_patches.h, "Copyright (C)
2024 STMicroelectronics SA"), written out as a file. cut1 is 6868 bytes and
its header parses correctly against struct vd55g_patch_header — 0x0681,
version 2.11 — so the format is right. That is how I tested, not a
distribution path I am proposing.

Second, and this one reaches further than this series. Benjamin's warning
about libcamera is concrete and it is a regression for machines that work
today. libcamera keys the sensor-properties database, the soft-ISP helper
and the tuning file on the media entity name, which is the driver name:

  WARN  No static properties available for 'vd55g'
  WARN  'vd55g 1-0060': No sensor delays found in static properties
  WARN  Configuration file 'vd55g.yaml' not found for IPA module 'simple'
  WARN  IPASoft: Failed to create camera sensor helper for vd55g

and the installed libcamera 0.7.0 does carry a vd55g1 entry — `strings
libcamera.so.0.7 | grep -x vd55g1` finds it. So renaming the driver does
not only fail to help VD55G0; it silently takes the properties and the
helper away from VD55G1 users who have them now. Whatever is decided about
one driver or two, that rename needs libcamera patches landing alongside,
or the entity name keeping its old form.

Two smaller things, neither chased:

  - `supply vcore not found, using dummy regulator` persists here despite
    4/7's SMO55F0 mapping. It was not the blocker, so I did not dig.
  - 3/7 advertises 380 MHz for SMO55F0. This machine has been running 402 MHz
    ("MIPI output freq is 804MHz / 2") from the out-of-tree driver. Your
    driver takes the endpoint's value rather than validating against a list,
    so it accepted 380 without complaint, and I have not established which is
    correct for either model. Worth knowing that two people measured
    different numbers for the same HID on different Surfaces.

Happy to run v2 here when you have one, and to report anything specific you
would like measured on IPU7.

Sergey

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

[Benjamin Mugnier]

Hi Sergey,

Le 07/09/2026 à 22:59, Sergey Lebedev a écrit :
> Peter,
> 
> Tested on a second machine: Surface Pro 11 for Business, Intel Lunar Lake,
> IPU7 — same SMO55F0 HID as your Surface Pro 9, one IPU generation later.
> Two results worth having before you respin, one good and one blocking.
> 
> The good one: the series applies and builds clean, and the sensor works on
> IPU7. All seven patches applied in order to an Ubuntu 7.0.0 tree with no
> conflicts (its vd55g1.c is 2061 lines, exactly what your diffstat deletes),
> and media/i2c, media/pci/intel and int3472 rebuilt without a warning.
> With firmware present the driver probes, the bridge finds the sensor and
> the subdev registers:
> 
>   intel-ipu7: Found supported sensor SMO55F0:00
>   intel_ipu7_isys: bind vd55g 1-0060 nlanes is 1 port is 1
>   entity 233: vd55g 1-0060 (1 pad, 1 link, 0 routes)
> 
> So nothing here is IPU6-specific. That is the part I could add that you
> could not.
> 
> The blocking one: without that firmware nobody can use VD55G0 at all.
> 
>   vd55g i2c-SMO55F0:00: Direct firmware load for vd55g0-cut1.bin failed
>                         with error -2
>   vd55g i2c-SMO55F0:00: Failed to load required firmware vd55g0-cut1.bin: -2
> 
> Probe fails and the camera disappears. The driver has a fallback for
> exactly this case, but only the older part gets one:
> 
>   static const struct firmware vd55g1_builtin_fw = {
>           .data = vd55g1_patch_array, .size = ARRAY_SIZE(vd55g1_patch_array)
>   };
>   ...
>   if (ret == -ENOENT && fw_info->builtin_fw)
> 
> vd55g0_fw_maps[] sets fw_name and leaves builtin_fw NULL, and neither
> vd55g0-cut1.bin nor vd55g0-cut2.bin is in linux-firmware. So the series
> works for you and for nobody else, which I assume is not the intent. Either
> a built-in like VD55G1's, or the files submitted to linux-firmware, or at
> minimum a note in the cover letter saying where they come from.
> 
> To get past it I used the same ST patch arrays that André Gilerson's
> out-of-tree VD55G0 driver compiles in (vd55g0_patches.h, "Copyright (C)
> 2024 STMicroelectronics SA"), written out as a file. cut1 is 6868 bytes and

I'm not aware of André's work. Could you link it to me ?

The only vd55g0 out of tree driver I know is our own version on
Github[1]. Is it a fork of it ?

[1] https://github.com/STMicroelectronics/vd55g0-linux-driver

> its header parses correctly against struct vd55g_patch_header — 0x0681,
> version 2.11 — so the format is right. That is how I tested, not a
> distribution path I am proposing.
> 
> Second, and this one reaches further than this series. Benjamin's warning
> about libcamera is concrete and it is a regression for machines that work
> today. libcamera keys the sensor-properties database, the soft-ISP helper
> and the tuning file on the media entity name, which is the driver name:
> 
>   WARN  No static properties available for 'vd55g'
>   WARN  'vd55g 1-0060': No sensor delays found in static properties
>   WARN  Configuration file 'vd55g.yaml' not found for IPA module 'simple'
>   WARN  IPASoft: Failed to create camera sensor helper for vd55g
> 
> and the installed libcamera 0.7.0 does carry a vd55g1 entry — `strings
> libcamera.so.0.7 | grep -x vd55g1` finds it. So renaming the driver does
> not only fail to help VD55G0; it silently takes the properties and the
> helper away from VD55G1 users who have them now. Whatever is decided about
> one driver or two, that rename needs libcamera patches landing alongside,
> or the entity name keeping its old form.
> 
> Two smaller things, neither chased:
> 
>   - `supply vcore not found, using dummy regulator` persists here despite
>     4/7's SMO55F0 mapping. It was not the blocker, so I did not dig.
>   - 3/7 advertises 380 MHz for SMO55F0. This machine has been running 402 MHz
>     ("MIPI output freq is 804MHz / 2") from the out-of-tree driver. Your
>     driver takes the endpoint's value rather than validating against a list,
>     so it accepted 380 without complaint, and I have not established which is
>     correct for either model. Worth knowing that two people measured
>     different numbers for the same HID on different Surfaces.
> 
> Happy to run v2 here when you have one, and to report anything specific you
> would like measured on IPU7.
> 
> Sergey
> 

-- 
Regards,
Benjamin

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

[Sergey Lebedev]

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

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

[Sergey Lebedev]

Following up on the firmware blocker I raised on 7 September, since it is
still the thing that stops anyone but the author from using VD55G0. Relayed
with Peter Marshall's agreement.

Peter sourced the firmware the same way I did for testing: the version 2.11
patch array in Andre Gilerson's out-of-tree VD55G0 driver, which he reports is
identical to the one in ST's standalone driver. He has published the extracted
blobs for testing at

  https://github.com/petm5/vd55g-firmware

and would prefer submission to linux-firmware over a built-in array, so that
firmware can be updated independently of the kernel version. I agree that is
the better end state.

Worth adding, because it may lower the bar for getting there: the built-in
arrangement is already in the tree, from ST, for the sibling part.
drivers/media/i2c/vd55g1.c is Copyright (C) 2025 STMicroelectronics SA under
SPDX-License-Identifier: GPL-2.0, and it carries vd55g1_patch_array[] - 3512
bytes, firmware revision 2.9 by its own defines. That file has no
request_firmware path at all; the built-in array is its only mechanism.

Different sensor and a different binary from the VD55G0 one, which measures
6868 bytes at revision 2.11, so it is precedent for the arrangement rather
than for the blob. But it does mean a built-in for VD55G0 would not be asking
ST for something they have not already done one part number over, whereas
linux-firmware needs an explicit redistribution grant in WHENCE from the
copyright holder.

So the two routes are not equally hard today, and they are not exclusive: a
built-in now and linux-firmware when ST are ready is not a wasted step.

Whether GPL-2.0 on the surrounding code carries the blob with it is a licence
question and not one I will answer.

Benjamin, Sylvain - this is the one blocker on the series that nobody outside
ST can clear.

Sergey

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

[Benjamin Mugnier]

Hi Sergey,

Le 10/09/2026 à 23:33, Sergey Lebedev a écrit :
> Following up on the firmware blocker I raised on 7 September, since it is
> still the thing that stops anyone but the author from using VD55G0. Relayed
> with Peter Marshall's agreement.
> 
> Peter sourced the firmware the same way I did for testing: the version 2.11
> patch array in Andre Gilerson's out-of-tree VD55G0 driver, which he reports is
> identical to the one in ST's standalone driver. He has published the extracted
> blobs for testing at
> 
>   https://github.com/petm5/vd55g-firmware
> 
> and would prefer submission to linux-firmware over a built-in array, so that
> firmware can be updated independently of the kernel version. I agree that is
> the better end state.
> 
> Worth adding, because it may lower the bar for getting there: the built-in
> arrangement is already in the tree, from ST, for the sibling part.
> drivers/media/i2c/vd55g1.c is Copyright (C) 2025 STMicroelectronics SA under
> SPDX-License-Identifier: GPL-2.0, and it carries vd55g1_patch_array[] - 3512
> bytes, firmware revision 2.9 by its own defines. That file has no
> request_firmware path at all; the built-in array is its only mechanism.

We initially thought of using linux-firmware, and we didn't for several
reasons :
- The firmware is reasonably small.
- The driver source code is directly dependant on the firmware, as some
behaviors may be altered by the firmware. Keeping it in the source code
allows easy synchronisation.
- A lot of mainline sensor do the same thing, with exception for only 4
drivers (ccs included) that use request-firmware.

It has been discussed with maintainers back to the vgxy61.

Unless I'm proven moving to the request_firmware API allows more upsides
than the above points, unfortunately I will not ack a serie that moves
from the builtin patch to the request_firmware API.

> 
> Different sensor and a different binary from the VD55G0 one, which measures
> 6868 bytes at revision 2.11, so it is precedent for the arrangement rather
> than for the blob. But it does mean a built-in for VD55G0 would not be asking
> ST for something they have not already done one part number over, whereas
> linux-firmware needs an explicit redistribution grant in WHENCE from the
> copyright holder.

I'm not sure I get you. Is this a licence problem of some sort ? Could
you rephrase ?

Anyway if we don't move to request_firmware I guess this is a non issue ?

By the way we didn't upstream the vd55g0 because it requires a bit of
cleaning, but this is something that could also be done.

> 
> So the two routes are not equally hard today, and they are not exclusive: a
> built-in now and linux-firmware when ST are ready is not a wasted step.
> 
> Whether GPL-2.0 on the surrounding code carries the blob with it is a licence
> question and not one I will answer.
> 
> Benjamin, Sylvain - this is the one blocker on the series that nobody outside
> ST can clear.
> 
> Sergey
> 

-- 
Regards,
Benjamin

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

[Sergey Lebedev]

Benjamin,

First, an apology: I wrote that badly. Reading it back I can see it lands as
a request to move vd55g1 to request_firmware, and it was never meant as one.
You have had to defend a position I was not attacking, and that is my fault
rather than yours.

What I meant, and should have written: the VD55G0 firmware has to reach users
somehow, and I do not mind which of the two mechanisms carries it. Your three
reasons settle the mechanism for me - built in, and no more about it. Peter
preferred linux-firmware and I relayed that with his agreement; whether your
answer changes his view is his to say, not mine.

> I'm not sure I get you. Is this a licence problem of some sort ? Could
> you rephrase ?

Yes, and it is not about the API at all. It is about who may put ST's
firmware into the kernel tree.

vd55g1.c raises no question. ST holds the copyright and ST put its own array
in its own file, GPL-2.0, 3512 bytes, no request_firmware path anywhere.
Nothing had to be granted, by anyone, to anyone.

VD55G0 is not in that position today. The bytes this series would carry were
extracted from a third-party out-of-tree driver, by people who are not ST.
That is the same question whether they land as a C array or as a file in
linux-firmware - the mechanism does not change who owns them. linux-firmware
only makes it audible, because WHENCE asks for the grant out loud, where a
built-in array lets the same question pass without being asked.

So there are three routes, and only one of them is clean:

  built in, sent by someone outside ST - which is the series as it stands.
  Works technically. Leaves the provenance question unasked rather than
  answered.

  linux-firmware - needs an explicit grant from ST in WHENCE, and you have
  now ruled out the API change it would require. So: closed.

  ST upstreams VD55G0 itself, with its own array, exactly as you did for
  vd55g1. Nobody outside ST has to ask for anything, because nothing is being
  redistributed by anyone who does not own it.

> By the way we didn't upstream the vd55g0 because it requires a bit of
> cleaning, but this is something that could also be done.

That is the one I would hope for, and it is worth more than getting my own
machine working. It is the only route that ends the question rather than
moving it. And it puts VD55G0 on the same footing as its sibling, rather than
leaving it the part with an awkward history.

If the cleaning is what stands in the way, say what would help and I will do
what I can. I cannot clean code I do not have. I can carry the mechanical
half of it - checkpatch and sparse, builds across configurations, dt-binding
checks, a review pass. And I can test on hardware you may not have: a Surface
Pro 11 where the VD55G0 is the face-unlock sensor, so it is exercised by
something real rather than by a capture tool. Peter's series in this thread
already does a good deal of the genericising, if any of it is useful as a
starting point.

Whether GPL-2.0 on the surrounding code carries a blob with it is still a
licence question, and still not one I will answer.

Sergey

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

[Benjamin Mugnier]

Hi Sergey,

Le 15/09/2026 à 12:07, Sergey Lebedev a écrit :
> Benjamin,
> 
> First, an apology: I wrote that badly. Reading it back I can see it lands as
> a request to move vd55g1 to request_firmware, and it was never meant as one.
> You have had to defend a position I was not attacking, and that is my fault
> rather than yours.

No worries, It didn't feel like an attack of any sort ;)

> 
> What I meant, and should have written: the VD55G0 firmware has to reach users
> somehow, and I do not mind which of the two mechanisms carries it. Your three
> reasons settle the mechanism for me - built in, and no more about it. Peter
> preferred linux-firmware and I relayed that with his agreement; whether your
> answer changes his view is his to say, not mine.
> 
>> I'm not sure I get you. Is this a licence problem of some sort ? Could
>> you rephrase ?
> 
> Yes, and it is not about the API at all. It is about who may put ST's
> firmware into the kernel tree.

Ah I got you. I'm not a licence expert in any way but since we already
disclosed the firmware patch in our out of tree driver as GPL2 [1], then
I guess it's free to use it as long as you respect GPL2, whether you're
from ST or not.

[1]
https://github.com/STMicroelectronics/vd55g0-linux-driver/blob/master/vd55g0_patches.h

> 
> vd55g1.c raises no question. ST holds the copyright and ST put its own array
> in its own file, GPL-2.0, 3512 bytes, no request_firmware path anywhere.
> Nothing had to be granted, by anyone, to anyone.
> 
> VD55G0 is not in that position today. The bytes this series would carry were
> extracted from a third-party out-of-tree driver, by people who are not ST.
> That is the same question whether they land as a C array or as a file in
> linux-firmware - the mechanism does not change who owns them. linux-firmware
> only makes it audible, because WHENCE asks for the grant out loud, where a
> built-in array lets the same question pass without being asked.
> 
> So there are three routes, and only one of them is clean:
> 
>   built in, sent by someone outside ST - which is the series as it stands.
>   Works technically. Leaves the provenance question unasked rather than
>   answered.
> 
>   linux-firmware - needs an explicit grant from ST in WHENCE, and you have
>   now ruled out the API change it would require. So: closed.
> 
>   ST upstreams VD55G0 itself, with its own array, exactly as you did for
>   vd55g1. Nobody outside ST has to ask for anything, because nothing is being
>   redistributed by anyone who does not own it.
> 
>> By the way we didn't upstream the vd55g0 because it requires a bit of
>> cleaning, but this is something that could also be done.
> 
> That is the one I would hope for, and it is worth more than getting my own
> machine working. It is the only route that ends the question rather than
> moving it. And it puts VD55G0 on the same footing as its sibling, rather than
> leaving it the part with an awkward history.
> 
> If the cleaning is what stands in the way, say what would help and I will do
> what I can. I cannot clean code I do not have. I can carry the mechanical
> half of it - checkpatch and sparse, builds across configurations, dt-binding
> checks, a review pass. And I can test on hardware you may not have: a Surface
> Pro 11 where the VD55G0 is the face-unlock sensor, so it is exercised by
> something real rather than by a capture tool. Peter's series in this thread
> already does a good deal of the genericising, if any of it is useful as a
> starting point.

I still stand against the refactoring and generalisation. I can
elaborate on that point if needed.

Now about mainlining vd55g0 out of tree module, my current priorities
won't allow me to do it anytime soon, but if you want to start from the
out of tree module and clean the driver to submit it I'll be happy to
assist. On top of my head we need to remove the KERNEL_VERSION guards
and move to streams api. Having the vd55g1 as reference is also a good
idea. I can test on device trees platforms.

> 
> Whether GPL-2.0 on the surrounding code carries a blob with it is still a
> licence question, and still not one I will answer.
> 
> Sergey
> 

-- 
Regards,
Benjamin

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

[Sergey Lebedev]

Benjamin,

Thank you - for the explanation, for the link, and for saying where you
stand. All three are worth more than an ack would have been.

Sorry for the slow reply. I spent the hours reading rather than writing: the
repository you pointed at, the two drivers side by side, and the in-tree
binding. The checking changed what I have to say, so it seemed better than
answering quickly. Three things below, separated so you can take whichever is
worth your time.

1. The licence question, and my mistake
---------------------------------------

Settled, by your link. vd55g0_patches.h carries
"SPDX-License-Identifier: GPL-2.0" and "Copyright (C) 2024 STMicroelectronics
SA", with cut1 and cut2 arrays. ST published the firmware itself, two years
ago, under a licence that answers the question I was asking.

There is nothing for anyone to grant, and the right source to take those
bytes from is yours rather than any third-party copy of them.

I should have found that before raising the point twice. Your standalone
driver was named to me in this thread and I did not open it. The blocker I
reported on 7 September is not a blocker and never was.

2. What the work would actually be
----------------------------------

I measured it rather than guessed, so that we are talking about the same
thing:

  - twenty-five LINUX_VERSION_CODE guards, all of the form
    "#if KERNEL_VERSION(x,y,z) > LINUX_VERSION_CODE", about 180 lines to drop
  - s_stream to enable_streams and disable_streams, with vd55g1 in-tree as a
    line-by-line reference written by you
  - st,vd55g0.yaml, adapted from the 3.3 KB st,vd55g1.yaml
  - Kconfig, Makefile, MAINTAINERS
  - checkpatch, sparse, builds across configurations, and testing here

That should not take as long as I expected.

3. The question I cannot answer on your behalf
----------------------------------------------

A 2100-line driver arriving beside a 2100-line sibling by the same author
invites "why is this not an extension of vd55g1". If I answer that badly,
the third version is a rewrite in the direction you have already rejected.

Reading the two files I can see arguments for your position. vd55g0 is
monochrome-first, Y8_1X8 and Y10_1X10, where vd55g1 carries the Bayer codes.
vd55g0 has real strobe and flash handling that vd55g1 barely touches. The
register namespaces are essentially disjoint.

But those are my inferences from one reading. What I would be repeating to a
maintainer is your engineering judgement, and I would rather have it from
you.

So: you offered to elaborate. Please do, if you have the time. Not to
convince me - I have no stake in either shape. It is so that when Sakari or
Hans asks why VD55G0 is not folded into vd55g1, the answer comes from the
person who wrote both parts.

If a separate driver is right, I will start and send you something to look
at. If the two really should converge, that is worth knowing now too, and
Peter's series already points that way.

Sergey

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

[Peter Marshall]

This patch series refactors the STMicroelectronics VD55G1 and VD65G4 image
sensor driver (vd55g1) into a generic VD55G family driver (vd55g) and adds
support for the VD55G0 monochrome sensor.

The VD55G0 sensor is found in devices such as the Microsoft Surface Pro 9 under
ACPI HID `SMO55F0`. It features a native resolution of 644 x 604 pixels and
differs from later variants in the family in several ways:

* It uses a different native resolution and register addresses mapping,
  requiring specification of generational feature differences.
* It lacks support for spatial and temporal HDR.
* It requires an exposure duty cycle limit to keep the illuminator current safe.

Tested on a Microsoft Surface Pro 9 (an Intel IPU6 platform with ACPI
HID "SMO55F0") running libcamera.

Peter Marshall (7):
  media: i2c: vd55g: move patch array to a separate firmware header
  dt-bindings: media: i2c: st,vd55g: rename and add VD55G0 variant
  media: ipu-bridge: Add VD55G0 to the list of supported sensors
  platform/x86: int3472: Add VD55G0 supply GPIO mapping
  media: i2c: st-vd55g: Add generic driver and VD55G0 support
  media: i2c: st-vd55g: Remove legacy vd55g1 remnants
  MAINTAINERS: Update ST VD55G camera driver entry

 .../i2c/{st,vd55g1.yaml => st,vd55g.yaml}     |   30 +-
 MAINTAINERS                                   |    7 +-
 drivers/media/i2c/Kconfig                     |   11 +
 drivers/media/i2c/Makefile                    |    2 +-
 drivers/media/i2c/vd55g-fw.h                  |  305 +++
 drivers/media/i2c/vd55g.c                     | 2166 +++++++++++++++++
 drivers/media/i2c/vd55g1.c                    | 2061 ----------------
 drivers/media/pci/intel/ipu-bridge.c          |    2 +
 drivers/platform/x86/intel/int3472/discrete.c |   18 +
 9 files changed, 2531 insertions(+), 2071 deletions(-)
 rename Documentation/devicetree/bindings/media/i2c/{st,vd55g1.yaml => st,vd55g.yaml} (76%)
 create mode 100644 drivers/media/i2c/vd55g-fw.h
 create mode 100644 drivers/media/i2c/vd55g.c
 delete mode 100644 drivers/media/i2c/vd55g1.c

-- 
2.55.0

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

[Benjamin Mugnier]

Hi Peter,

Le 02/09/2026 à 22:45, Peter Marshall a écrit :
> This patch series refactors the STMicroelectronics VD55G1 and VD65G4 image
> sensor driver (vd55g1) into a generic VD55G family driver (vd55g) and adds
> support for the VD55G0 monochrome sensor.
> 
> The VD55G0 sensor is found in devices such as the Microsoft Surface Pro 9 under
> ACPI HID `SMO55F0`. It features a native resolution of 644 x 604 pixels and
> differs from later variants in the family in several ways:
> 
> * It uses a different native resolution and register addresses mapping,
>   requiring specification of generational feature differences.
> * It lacks support for spatial and temporal HDR.
> * It requires an exposure duty cycle limit to keep the illuminator current safe.
> 
> Tested on a Microsoft Surface Pro 9 (an Intel IPU6 platform with ACPI
> HID "SMO55F0") running libcamera.

I'm sorry I haven't read through your whole serie yet.

First thing first what is the point in refactoring both drivers into the
same one ? I guess it should be addressed in your commit messages. To be
fair it even looks to add complexity because even if the sensors share
some IPs you might have notice they are in fact very different. I could
even argue that the vd56g3 is closer to the vd55g1 than the vd55g0 is.

Plus having your commits merged will also require changes in libcamera.

Otherwise I'm interested in your patches 3 and 4. If I read it correctly
they allow the Microsoft Surface Pro 9 to use the embedded vd55g0 right ?

> 
> Peter Marshall (7):
>   media: i2c: vd55g: move patch array to a separate firmware header
>   dt-bindings: media: i2c: st,vd55g: rename and add VD55G0 variant
>   media: ipu-bridge: Add VD55G0 to the list of supported sensors
>   platform/x86: int3472: Add VD55G0 supply GPIO mapping
>   media: i2c: st-vd55g: Add generic driver and VD55G0 support
>   media: i2c: st-vd55g: Remove legacy vd55g1 remnants
>   MAINTAINERS: Update ST VD55G camera driver entry
> 
>  .../i2c/{st,vd55g1.yaml => st,vd55g.yaml}     |   30 +-
>  MAINTAINERS                                   |    7 +-
>  drivers/media/i2c/Kconfig                     |   11 +
>  drivers/media/i2c/Makefile                    |    2 +-
>  drivers/media/i2c/vd55g-fw.h                  |  305 +++
>  drivers/media/i2c/vd55g.c                     | 2166 +++++++++++++++++
>  drivers/media/i2c/vd55g1.c                    | 2061 ----------------
>  drivers/media/pci/intel/ipu-bridge.c          |    2 +
>  drivers/platform/x86/intel/int3472/discrete.c |   18 +
>  9 files changed, 2531 insertions(+), 2071 deletions(-)
>  rename Documentation/devicetree/bindings/media/i2c/{st,vd55g1.yaml => st,vd55g.yaml} (76%)
>  create mode 100644 drivers/media/i2c/vd55g-fw.h
>  create mode 100644 drivers/media/i2c/vd55g.c
>  delete mode 100644 drivers/media/i2c/vd55g1.c
> 

-- 
Regards,
Benjamin

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

[Benjamin Mugnier]

Hi Peter,

Adding back everyone to the CC list so they can see our discussion ;)

Le 04/09/2026 à 22:51, Peter Marshall a écrit :
> Hi Benjamin,
> 
> Thanks for your initial review.
> 
> I chose to merge the new hardware support into the existing driver to avoid code
> duplication and improve long-term maintainability. It looks like mainline is
> moving toward generic interfaces, and away from the "copy-and-modify" status quo
> common in vendor kernel trees.

I'm not aware of such thing in the mainline tree, we can take for
example imx sensors and every sensor tends to have it's own driver, but
I may be wrong.

> 
> The vd55g1 appears to be a generational successor of the vd55g0 and the two are
> similar enough that the driver only needs a few feature flags passed through
> the ACPI/OF match table to support both of them. The added abstraction does not
> obscure operational logic, although it might make it harder to verify according
> to the datasheets.

I tend to disagree, I still think it adds complexity and slowdown code
iteration as we will have to test both drivers at the same time to see
any regressions, which is not ideal.

I'd like to have an advice from Sakari and or Laurent on this matter,
what do you think ?

> 
> Patches 3 and 4 add the required identifiers for the Microsoft Surface Pro 9 to
> use its embedded vd55g0. The sensor isn't quite usable yet because of remaining
> issues in the IPU6 subsystem. Most notably the stream fails to start if the
> sensor sends less than a certain number of bytes per line (384 bytes or less
> on my hardware), and the frame gets cut off causing flickering when the CPU is
> idle. The latter issue was already documented by Hans de Goede on linux-media
> (Oct 2025).

Gotcha.

> 
> libcamera comes with generic defaults for unknown sensors and doesn't require
> any modifications to stream successfully.

Well yeah, the sensor will stream but will lack its medata to make it
stream cleanly. Please note that for Raspberry Pi this is a hard
requirement and it won't stream at without a cam_helper. Honestly that's
not that big of a deal and I consider it a side effect, we could fix
that easilly. I'm more concerned about the huge refactoring part.

> 
> Unfortunately, patch 5 in my series is not split into logical steps and does
> way too much at once. The renaming from vd55g1 to vd55g was unnecessary, and the
> following patches 6 and 7 should have been squashed into it as they represent
> cleanup steps instead of isolated logical changes. That makes this series
> completely unbisectable and not suitable for merging. The next version will

Indeed.

> focus on abstracting vd55g1.c in logical steps before supporting
> the vd55g0 revision.

Thank you for your work. I'm eager to see this.

> 
> Best regards,
> Peter Marshall

-- 
Regards,
Benjamin