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

[Benjamin Mugnier <benjamin.mugnier@foss.st.com>]

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