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

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

Hi Sergey,

Le 15/09/2026 à 19:36, Sergey Lebedev a écrit :
> 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.

That's fine, I'm glad you're unblocked :)

> 
> 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.

I think the fact that the vgxy61, vd56g3 and vd55g1 already coexists as
separate files in the kernel file structure, all reviewed by Sakari,
could be seen as an implicit agreement. They all share some common IPs
but are pretty different. Sakari, if you have anything to add, please do so.

Sorry for repeating myself but I personally think having everything
separated is better for code clarity. As the register map is
fundamentally different, you will need an indirection map pointing to
each register i2c adress for each sensor, like shown in Peter's serie,
i.e. :

  expoure_register[vd55g1_sensor_type]

While I appreciate the idea, IMHO this boilerplate adds unnecessary
complexity and feels pretty error prone.

As you mentioned both sensors don't offer the same functionalities, so
you will also need some kind of :

  if (vd55g1_sensor_type)
    foorbar();

On top of that, it makes testing a bit more tedious as each change in a
generalised driver might break the other sensor, which you might not
want because, well, let's say you made a change for the vd55g0 and don't
have a vd55g1 to test you didn't break anything on its flow. This issue
disappear if both files are separated.

I'm also a bit concerned about the history, since an heavy refactoring
will alter the git history and make git bisect harder. As we already
have a v55g0 downstream driver, that I think is mature enough. I see it
as a better idea to use it as a starting point.

This are all the downsides I see while typing, of course I might have
forgotten some, but you get the idea.

> 
> 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.

Thanks a ton, I really appreciate the work you and Peter are doing. If
you start this journey please add yourself or/and Peter as MODULE_AUTHOR
and in the maintainers file. Don't hesitate to contact me either.

> 
> Sergey
> 

-- 
Regards,
Benjamin