Re: [PATCH v4 10/16] media: dt-bindings: sony,imx678: Add other Starvis 2 family members

[Laurent Pinchart]

On Tue, Oct 20, 2026 at 03:39:52PM +0100, Dave Stevenson wrote:
> On Tue, 20 Oct 2026 at 11:07, Sakari Ailus wrote:
> > On Thu, Oct 15, 2026 at 04:57:36PM +0100, Dave Stevenson wrote:
> > > There are multiple members of the Sony Starvis 2 image sensor family
> > > that have the same hardware requirements as imx678.
> > > Add them as options to the imx678 binding, rather than duplicating it.
> > >
> > > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxxxxx>
> > > ---
> > >  .../devicetree/bindings/media/i2c/sony,imx678.yaml | 38 ++++++++++++++++++----
> > >  1 file changed, 32 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx678.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx678.yaml
> > > index 098f72544216..027872a134f2 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx678.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx678.yaml
> > > @@ -5,7 +5,7 @@
> > >  $id: http://devicetree.org/schemas/media/i2c/sony,imx678.yaml#
> > >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> > >
> > > -title: Sony IMX678 Sensor
> > > +title: Sony IMX678 and Starvis 2 Sensors
> > >
> > >  maintainers:
> > >    - Jai Luthra <jai.luthra@xxxxxxxxxxxxxxxx>
> > > @@ -13,21 +13,47 @@ maintainers:
> > >  description:
> > >    Sony IMX678 diagonal 8.86 mm (Type 1/1.8) CMOS active pixel type solid-state
> > >    image sensor with a square pixel array and 8.40M (3856x2180) effective pixels.
> >
> > You could rewrap this while at it.
> >
> > > +  It is part of the Starvis 2 family of sensors.
> > > +  Other members of the Starvis 2 have the same hardware requirements, and
> > > +  therefore share this binding.
> >
> > This paragraph could be rewrapped, too -- the limit is 80 characters per
> > line.
> 
> OK, I'll rephrase it so that it makes sense as one reflowed paragraph.
> 
> > >
> > >  allOf:
> > >    - $ref: /schemas/media/video-interface-devices.yaml#
> > >
> > >  properties:
> > >    compatible:
> > > -    items:
> > > -      - enum:
> > > -          - sony,imx678-aamr
> > > -          - sony,imx678-aaqr
> > > -      - const: sony,imx678
> > > +    oneOf:
> > > +      - items:
> > > +          - enum:
> > > +              - sony,imx585-aamj
> > > +              - sony,imx585-aaqj
> > > +          - const: sony,imx585
> > > +      - items:
> > > +          - enum:
> > > +              - sony,imx662-aamr
> > > +              - sony,imx662-aaqr
> > > +          - const: sony,imx662
> > > +      - items:
> > > +          - enum:
> > > +              - sony,imx675-aamr
> > > +              - sony,imx675-aaqr
> > > +          - const: sony,imx675
> > > +      - items:
> > > +          - enum:
> > > +              - sony,imx678-aamr
> > > +              - sony,imx678-aaqr
> > > +          - const: sony,imx678
> > > +      - items:
> > > +          - enum:
> > > +              - sony,imx832-amr
> > > +              - sony,imx832-aqr
> > > +          - const: sony,imx832
> >
> > Should we also have a sony,starvis2 compatible? Just wondering.
> 
> I deliberately didn't based on the discussions Jai had with DT
> maintainers when upstreaming the original binding [1], particularly
> Conor's comment:
> "Remember, having distinct multiple compatibles for the same hardware so
> that you can induce different software behaviour is not something we
> typically permit. While we do want to support all use cases, it isn't
> always the case that devicetree is the right place to do it."
> 
> Sorry, but when time is limited, IMHO debating dtbindings just isn't
> worth it. I've therefore taken the simple option.
> 
> I do have patches that implement that compatible and fully autodetect
> the sensor[2], but I'll keep them downstream.
> Doing so means that we have one common overlay for connecting any of
> the supported Starvis2 sensors to a Pi.
> 
> [1] https://patchwork.linuxtv.org/project/linux-media/patch/20260520-imx678-v3-1-8b5f9676486e@xxxxxxxxxxxxxxxx/
> [2] Currently the top 2 commits on
> https://github.com/6by9/linux/tree/mainline_starvis2_v2, but that
> branch will get rebased/reworked.
> 
> > The driver isn't generic still as it requires sensor specific information
> > to be passed to it. CCS solves this with CCS static data (via firmware
> > loading mechanism) so there are no driver changes even if the capability
> > and limit information can't be obtained from device's registers or the
> > device requires MSRs.
> 
> I'm assuming you're referring to the patchset as a whole as I don't
> believe I've used the term generic in this patch. I'm happy to swap to
> "common Starvis 2 driver" where appropriate within the patchset
> instead.
> 
> IMHO Pushing the config to firmware files doesn't help, particularly
> as these sensors don't have a common ID register and so identification
> isn't trivial. It also makes life more involved for a distribution
> maintainer to ensure they're picking up the relevant files, and that's
> assuming that someone is curating them.
> 
> My aim with the series was to reuse as much common code as possible,
> rather than submitting 5 separate drivers which are identical other
> than constants and arrays of registers. Jai now has IMX676 working
> with this driver, so that would make it 6 variants. Refactoring to use
> a firmware file adds complexity for minimal gain, and you'd have to
> special case IMX678 so it worked without a firmware file to avoid
> regressions.
> 
> I guess the common core could be refactored to be a library, so that
> you could minimise module size by having the sensor specific config in
> a module that depends on the library. You're talking of saving a few
> kB uncompressed though, so that would be quite extreme. I'd be more
> tempted to have individual Kconfig entries and use #if
> IS_ENABLED(CONFIG_IMXxxx) to conditionally compile in the config to a
> common module.

Once the raw sensor model gets merged, I want to explore creating
helpers for that. Most of the code in this "starvis2" driver is not
specific to Starvis 2 image sensors, or even to Sony image sensors. I
expect there will be room for refactoring and making writing drivers for
raw sensors much simpler.

In the meantime, a starvis2 driver seems fine to me, even if drawing a
boundary is difficult. Register sets for Sony sensors constantly evolve,
they're not really designed by family as far as I can see.

> > > +
> > >      description:
> > >        The IMX678 sensor exists in a colour variant (IMX678-AAQR) and a mono
> > >        variant (IMX678-AAMR). The variant-specific compatible describes the
> > >        exact sensor variant, with the generic IMX678 compatible as a fallback.
> > > +      The same applies for the other sensors in the family.
> > >
> > >    reg:
> > >      maxItems: 1
> > >

-- 
Regards,

Laurent Pinchart