[RFC,v1,02/20] libcamera: v4l2_subdevice: Refactor the model name derivation

28314 diff mbox series

The heuristic that derives a model name in V4L2Subdevice::model()
operates on the media entity name only, but is only reachable through a
V4L2Subdevice instance. Move it to a static modelFromEntityName() helper
and reimplement model() in terms of it, so that a model name can be
derived from an entity name without instantiating a subdevice.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 include/libcamera/internal/v4l2_subdevice.h |  1 +
 src/libcamera/v4l2_subdevice.cpp            | 60 ++++++++++++---------
 2 files changed, 35 insertions(+), 26 deletions(-)

Hi Naush

On Fri, Sep 18, 2026 at 08:59:40AM +0100, Naushir Patuck wrote:
> The heuristic that derives a model name in V4L2Subdevice::model()
> operates on the media entity name only, but is only reachable through a
> V4L2Subdevice instance. Move it to a static modelFromEntityName() helper
> and reimplement model() in terms of it, so that a model name can be

Do we still need it ?

The only left user is CameraLens::init().

> derived from an entity name without instantiating a subdevice.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  include/libcamera/internal/v4l2_subdevice.h |  1 +
>  src/libcamera/v4l2_subdevice.cpp            | 60 ++++++++++++---------
>  2 files changed, 35 insertions(+), 26 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> index c37a82afa881..9593d4f45f64 100644
> --- a/include/libcamera/internal/v4l2_subdevice.h
> +++ b/include/libcamera/internal/v4l2_subdevice.h
> @@ -160,6 +160,7 @@ public:
>  	int setRouting(Routing *routing, Whence whence = ActiveFormat);
>
>  	const std::string &model();
> +	static std::string modelFromEntityName(const std::string &entityName);
>  	const V4L2SubdeviceCapability &caps() const { return caps_; }
>
>  	static std::unique_ptr<V4L2Subdevice>
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index a69de154e8bb..703c82d7f715 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -1702,38 +1702,46 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence)
>   */
>  const std::string &V4L2Subdevice::model()
>  {
> -	if (!model_.empty())
> -		return model_;
> +	if (model_.empty())
> +		model_ = modelFromEntityName(entity_->name());

But I understand removing V4L2Subdevice::model() would require
constructing a new string everytime modelFromEntityName() is called.

What if we keep V4L2Subdevice::model() and construct 'model_' at
V4L2Subdevice creation time and simply return it ?

Users that have not constructed a V4L2Subdevice yet can use
the static modelFromEntityName()

>
> -	/*
> -	 * Extract model name from the media entity name.
> -	 *
> -	 * There is no standardized naming scheme for sensor or other entities
> -	 * in the Linux kernel at the moment.
> -	 *
> -	 * - The most common rule, used by I2C sensors, associates the model
> -	 *   name with the I2C bus number and address (e.g. 'imx219 0-0010').
> -	 *
> -	 * - When the sensor exposes multiple subdevs, the model name is
> -	 *   usually followed by a function name, as in the smiapp driver (e.g.
> -	 *   'jt8ew9 pixel_array 0-0010').
> -	 *
> -	 * - The vimc driver names its sensors 'Sensor A' and 'Sensor B'.
> -	 *
> -	 * Other schemes probably exist. As a best effort heuristic, use the
> -	 * part of the entity name before the first space if the name contains
> -	 * an I2C address, and use the full entity name otherwise.
> -	 */
> -	const std::string &entityName = entity_->name();
> +	return model_;
> +}
> +
> +/**
> + * \brief Derive a model name from a media entity name
> + * \param[in] entityName The name of the media entity
> + *
> + * There is no standardized naming scheme for sensor or other entities in the
> + * Linux kernel at the moment.
> + *
> + * - The most common rule, used by I2C sensors, associates the model name with
> + *   the I2C bus number and address (e.g. 'imx219 0-0010').
> + *
> + * - When the sensor exposes multiple subdevs, the model name is usually
> + *   followed by a function name, as in the smiapp driver (e.g.
> + *   'jt8ew9 pixel_array 0-0010').
> + *
> + * - The vimc driver names its sensors 'Sensor A' and 'Sensor B'.
> + *
> + * Other schemes probably exist. As a best effort heuristic, use the part of
> + * the entity name before the first space if the name contains an I2C address,
> + * and use the full entity name otherwise.
> + *
> + * This function operates on the entity name alone, so that a model name can
> + * be derived without instantiating a subdevice.
> + *
> + * \return The model name
> + */
> +std::string V4L2Subdevice::modelFromEntityName(const std::string &entityName)

Do you think you want the entityName or just the entity ?

Thanks
  j

> +{
>  	static const std::regex i2cRegex{ " [0-9]+-[0-9a-f]{4}" };
>  	std::smatch match;
>
>  	if (std::regex_search(entityName, match, i2cRegex))
> -		model_ = entityName.substr(0, entityName.find(' '));
> -	else
> -		model_ = entityName;
> +		return entityName.substr(0, entityName.find(' '));
>
> -	return model_;
> +	return entityName;
>  }
>
>  /**
> --
> 2.53.0
>

Hi Jacopo,

On Fri, 16 Oct 2026 at 11:33, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Naush
>
> On Fri, Sep 18, 2026 at 08:59:40AM +0100, Naushir Patuck wrote:
> > The heuristic that derives a model name in V4L2Subdevice::model()
> > operates on the media entity name only, but is only reachable through a
> > V4L2Subdevice instance. Move it to a static modelFromEntityName() helper
> > and reimplement model() in terms of it, so that a model name can be
>
> Do we still need it ?

There are still callers in CameraSensorLegacy and CameraSensorRaw if
I'm not mistaken.

>
> The only left user is CameraLens::init().
>
> > derived from an entity name without instantiating a subdevice.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  include/libcamera/internal/v4l2_subdevice.h |  1 +
> >  src/libcamera/v4l2_subdevice.cpp            | 60 ++++++++++++---------
> >  2 files changed, 35 insertions(+), 26 deletions(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> > index c37a82afa881..9593d4f45f64 100644
> > --- a/include/libcamera/internal/v4l2_subdevice.h
> > +++ b/include/libcamera/internal/v4l2_subdevice.h
> > @@ -160,6 +160,7 @@ public:
> >       int setRouting(Routing *routing, Whence whence = ActiveFormat);
> >
> >       const std::string &model();
> > +     static std::string modelFromEntityName(const std::string &entityName);
> >       const V4L2SubdeviceCapability &caps() const { return caps_; }
> >
> >       static std::unique_ptr<V4L2Subdevice>
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index a69de154e8bb..703c82d7f715 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -1702,38 +1702,46 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence)
> >   */
> >  const std::string &V4L2Subdevice::model()
> >  {
> > -     if (!model_.empty())
> > -             return model_;
> > +     if (model_.empty())
> > +             model_ = modelFromEntityName(entity_->name());
>
> But I understand removing V4L2Subdevice::model() would require
> constructing a new string everytime modelFromEntityName() is called.
>
> What if we keep V4L2Subdevice::model() and construct 'model_' at
> V4L2Subdevice creation time and simply return it ?
>
> Users that have not constructed a V4L2Subdevice yet can use
> the static modelFromEntityName()

Yes, that makes sense and I'll change it for v2.

>
> >
> > -     /*
> > -      * Extract model name from the media entity name.
> > -      *
> > -      * There is no standardized naming scheme for sensor or other entities
> > -      * in the Linux kernel at the moment.
> > -      *
> > -      * - The most common rule, used by I2C sensors, associates the model
> > -      *   name with the I2C bus number and address (e.g. 'imx219 0-0010').
> > -      *
> > -      * - When the sensor exposes multiple subdevs, the model name is
> > -      *   usually followed by a function name, as in the smiapp driver (e.g.
> > -      *   'jt8ew9 pixel_array 0-0010').
> > -      *
> > -      * - The vimc driver names its sensors 'Sensor A' and 'Sensor B'.
> > -      *
> > -      * Other schemes probably exist. As a best effort heuristic, use the
> > -      * part of the entity name before the first space if the name contains
> > -      * an I2C address, and use the full entity name otherwise.
> > -      */
> > -     const std::string &entityName = entity_->name();
> > +     return model_;
> > +}
> > +
> > +/**
> > + * \brief Derive a model name from a media entity name
> > + * \param[in] entityName The name of the media entity
> > + *
> > + * There is no standardized naming scheme for sensor or other entities in the
> > + * Linux kernel at the moment.
> > + *
> > + * - The most common rule, used by I2C sensors, associates the model name with
> > + *   the I2C bus number and address (e.g. 'imx219 0-0010').
> > + *
> > + * - When the sensor exposes multiple subdevs, the model name is usually
> > + *   followed by a function name, as in the smiapp driver (e.g.
> > + *   'jt8ew9 pixel_array 0-0010').
> > + *
> > + * - The vimc driver names its sensors 'Sensor A' and 'Sensor B'.
> > + *
> > + * Other schemes probably exist. As a best effort heuristic, use the part of
> > + * the entity name before the first space if the name contains an I2C address,
> > + * and use the full entity name otherwise.
> > + *
> > + * This function operates on the entity name alone, so that a model name can
> > + * be derived without instantiating a subdevice.
> > + *
> > + * \return The model name
> > + */
> > +std::string V4L2Subdevice::modelFromEntityName(const std::string &entityName)
>
> Do you think you want the entityName or just the entity ?

I think just entity ought to be fine here.

Regards,
Naush


>
> Thanks
>   j
>
> > +{
> >       static const std::regex i2cRegex{ " [0-9]+-[0-9a-f]{4}" };
> >       std::smatch match;
> >
> >       if (std::regex_search(entityName, match, i2cRegex))
> > -             model_ = entityName.substr(0, entityName.find(' '));
> > -     else
> > -             model_ = entityName;
> > +             return entityName.substr(0, entityName.find(' '));
> >
> > -     return model_;
> > +     return entityName;
> >  }
> >
> >  /**
> > --
> > 2.53.0
> >