Re: [PATCH 1/1] media: dt-bindings: Add bindings for camera modules

[Sakari Ailus <sakari.ailus@iki.fi>]

Hi Jacopo,

On Fri, May 09, 2025 at 10:24:09PM +0200, Jacopo Mondi wrote:
> Hi Sakari
> 
> On Fri, May 09, 2025 at 08:12:47PM +0000, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Thu, May 08, 2025 at 07:00:34PM +0200, Jacopo Mondi wrote:
> > > Hi Sakari,
> > >   thanks a lot for the proposal, it will be useful for next week discussion
> >
> > Thank you for the review!
> >
> > >
> > > On Wed, May 07, 2025 at 11:13:38AM +0300, Sakari Ailus wrote:
> > > > Add bindings for camera modules to allow telling especially the user space
> > > > which module is found in the system. Camera modules do not have a device
> > > > node so this is a property for the camera sensor device node. This allows
> > > > describing modules that contain a single camera sensor.
> > > >
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > > Hi all,
> > > >
> > > > Here's the patch to give some advance warning for the camera module
> > > > discussion. The good thing is that it's quite short.
> > > >
> > > > The intent indeed is to address the regular use case where we have a
> > > > single sensor in a camera module. For cases where we have more, we'll need
> > > > something else, not based on individual properties. I believe this is
> > > > still the way to go, to address current issues and for a couple of
> > > > additional reasons:
> > > >
> > > > - Cameras with more than one sensor tend to be collections of camera
> > > >   modules so this is still relevant in most cases.
> > > >
> > > > - It's much simpler to have a single property than begin having new nodes
> > > >   in DT. In practice such nodes would be a poor fit for DT generally as
> > > >   they have (few or) no functions.
> > > >
> > > > The biggest difficulty is still in module identification. These components
> > > > tend to be often ignored and the best we have for a module name in that
> > > > case is random-looking string if even that. Besides DT bindings, we need
> > > > an additional (git?) tree to describe the modules that have no proper
> > > > names but it could be also useful for those that do, for instance to
> > > > include information on lens, field of view, IR filter, photos of the
> > > > module etc. There is some overlap with what libcamera needs, too.
> > > >
> > > > - Sakari
> > > >
> > > >  .../bindings/media/camera-module.yaml         | 52 +++++++++++++++++++
> > > >  .../media/video-interface-devices.yaml        |  3 ++
> > > >  2 files changed, 55 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/camera-module.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/camera-module.yaml b/Documentation/devicetree/bindings/media/camera-module.yaml
> > > > new file mode 100644
> > > > index 000000000000..31b898c8c334
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/camera-module.yaml
> > > > @@ -0,0 +1,52 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +# Copyright (C) 2025 Intel Corporation
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/camera-module.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Camera modules
> > > > +
> > > > +maintainers:
> > > > +  - Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > +
> > > > +description: |
> > > > +  Camera modules are devices that embed one or more active devices, including
> > > > +  Camera Sensors, Voice Coil Motor (VCM) and possibly a flash LED as well as
> > > > +  other passive devices such as lenses and Ultra-Violet (UV) filters. While the
> > > > +  camera modules themselves have no OF nodes and have generally no module
> > > > +  specific functions, it still does matter for the software stack as a whole
> > > > +  which module the devices are a part of.
> > > > +
> > > > +  Two properties are used for this, depending on what is known of the module:
> > >
> > > I might have missed a point here.
> > >
> > >
> > > > +
> > > > +  1. The model of the module is known. In this case the name of the module uses
> > > > +  the format "vendor,model[,version]" where "vendor" is the manufacturer of the
> > > > +  module and "model" the name of the model. The version part is optional. In
> > > > +  such cases the property "camera-module-canonical" will be used. If the vendor
> > > > +  is not known, the "gpio" vendor prefix is used.
> > >
> > > So if the module is "known" it will be described using the above
> > > specified triplet
> > >
> > > (also, why "gpio" ?)
> >
> > "gpio" is reserved. Another option would be to reserve a name for an
> > unknown vendor. I believe DT maintainers will have an opinion on this.
> >
> 
> I see
> 
> > >
> > > > +
> > > > +  2. The model of the module is unknown. In this case, the module has an
> > > > +  identifier only, and will be described in detail in the camera module
> > > > +  database. The property "camera-module-casual" is used to denote such modules.
> > >
> > > If the module is "unknown" it will be identified by a numerical id that
> > > points to the camera module database where it is "described in
> > > detail". But if an entry is present in the camera module database, then it's not
> > > really "unkown", right ?
> > >
> > > What is the actual difference between an "unknown" and a "known"
> > > module then ?
> >
> > We could use different terms certainly. I wanted to differentiate here
> 
> It's certainly not my intention to bikeshed on naming (yet :)
> 
> > modules the name of which, and hopefully also the manufacturer of which, is
> > known. Otherwise we can just describe it by various other means that are
> > all sub-par compared to having a model and the name of the vendor written
> > on the side of the module.
> >
> > >
> > >
> > > > +
> > > > +  Before including in this binding documentation, all modules shall also be
> > > > +  documented in add-URL-here.
> > >
> > > If an entry in the camera module database is a requirement can't we
> > > simply point to that entry using a numerical id like you proposed for
> > > the "camera-module-casual" property ?
> >
> > That exactly is the idea.
> 
> Ok, so the two properties are not mutually exclusive ?

Yes, they are. A module can not both have a known name and not have one at
the same time.

> 
> Because, and that's the thing I'm missing, if you have an entry in a
> database, that entry will almost certainly contain the vendor and the
> model name (which in my understanding means that if an entry exists
> the module is always "known").

This should actually probably be changed in such a way that the module
always needs to be added to the database. There will be more than how to
identify such a module there.

> >
> > >
> > > Thanks
> > >   j
> > >
> > > > +
> > > > +  All camera modules listed below shall have the name of the sensor as well as
> > > > +  other devices included in the module as DT compatible string mentioned in a
> > > > +  comment after the enumeration, separated by a whitespace (" ").
> > > > +
> > > > +  Always keep the enumeration alphabetically (1) or numerically (2) sorted.
> > > > +
> > > > +properties:
> > > > +  camera-module-canonical:
> > > > +    $ref: /schemas/types.yaml#/definitions/string
> > > > +    enum:
> > > > +      - "dell,0BF122N3" # onnn,ov01a10
> > > > +  camera-module-casual:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum:
> > > > +      - 1 # st,vs6555
> > > > +
> > > > +additionalProperties: true
> > > > diff --git a/Documentation/devicetree/bindings/media/video-interface-devices.yaml b/Documentation/devicetree/bindings/media/video-interface-devices.yaml
> > > > index cf7712ad297c..27fa6711367e 100644
> > > > --- a/Documentation/devicetree/bindings/media/video-interface-devices.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/video-interface-devices.yaml
> > > > @@ -10,6 +10,9 @@ maintainers:
> > > >    - Jacopo Mondi <jacopo@jmondi.org>
> > > >    - Sakari Ailus <sakari.ailus@linux.intel.com>
> > > >
> > > > +allOf:
> > > > +  - $ref: /schemas/media/camera-module.yaml#
> > > > +
> > > >  properties:
> > > >    flash-leds:
> > > >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >

-- 
Regards,

Sakari Ailus