linux-kernel - Re: [PATCH v1 3/8] MT9M114: Add get_mbus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPY8ntA2eedTY-0hY5ThkiKiKYKpSAij+bbeBWvWAgSeMEzvsQ@mail.gmail.com>
Date: Wed, 26 Feb 2025 17:13:38 +0000
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Mathis Foerst <mathis.foerst@...com>
Cc: linux-kernel@...r.kernel.org, 
	Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-media@...r.kernel.org, 
	devicetree@...r.kernel.org, manuel.traut@...com
Subject: Re: [PATCH v1 3/8] MT9M114: Add get_mbus_config

Hi Mathis

On Wed, 26 Feb 2025 at 15:45, Mathis Foerst <mathis.foerst@...com> wrote:
>
> The i.MX6 camera frameworks requires get_mbus_config to be implemented.
> See [0].
>
> [0] drivers/staging/media/imx/imx-media-csi.c - line 211..216

The docs for get_mbus_config say
 * @get_mbus_config: get the media bus configuration of a remote sub-device.
 *             The media bus configuration is usually retrieved from the
 *             firmware interface at sub-device probe time, immediately
 *             applied to the hardware and eventually adjusted by the
 *             driver.
https://github.com/torvalds/linux/blob/master/include/media/v4l2-subdev.h#L814

All other receiver drivers (including imx6-mipi-csi2.c) that call
get_mbus_config handle it returning -ENOIOCTLCMD by reverting to the
static configuration or the receiver node from device tree / ACPI.

I may be missing something, but as imx-media-csi.c appears to be the
exception, isn't it better to fix that up rather than having to fix up
all the sensor drivers that ever might get attached to it?

> Signed-off-by: Mathis Foerst <mathis.foerst@...com>
> ---
>  drivers/media/i2c/mt9m114.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
> index 5f0b0ad8f885..fa64d6d315a1 100644
> --- a/drivers/media/i2c/mt9m114.c
> +++ b/drivers/media/i2c/mt9m114.c
> @@ -1977,6 +1977,24 @@ static int mt9m114_ifp_registered(struct v4l2_subdev *sd)
>                 v4l2_device_unregister_subdev(&sensor->pa.sd);
>                 return ret;
>         }
> +       return 0;
> +}
> +
> +static int mt9m114_ifp_get_mbus_config(struct v4l2_subdev *sd,
> +                                  unsigned int pad,
> +                                  struct v4l2_mbus_config *cfg)
> +{
> +       struct mt9m114 *sensor = ifp_to_mt9m114(sd);
> +
> +       if (sensor->bus_cfg.bus_type == V4L2_MBUS_CSI2_DPHY) {
> +               cfg->type = V4L2_MBUS_CSI2_DPHY;

Not setting cfg->bus.mipi_csi2.num_data_lanes is going to cause some
confusion. What does an assumed 0 data lanes mean?

Likewise it would be sensible to set cfg->bus.mipi_csi2.flags so as to
avoid any ambiguities (did the caller memset all fields before
calling?)

  Dave

> +       } else {
> +               cfg->type = V4L2_MBUS_PARALLEL;
> +               cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> +                                         V4L2_MBUS_PCLK_SAMPLE_RISING |
> +                                         V4L2_MBUS_DATA_ACTIVE_HIGH;
> +               cfg->bus.parallel.bus_width = 8;
> +       }
>
>         return 0;
>  }
> @@ -1993,6 +2011,7 @@ static const struct v4l2_subdev_pad_ops mt9m114_ifp_pad_ops = {
>         .set_fmt = mt9m114_ifp_set_fmt,
>         .get_selection = mt9m114_ifp_get_selection,
>         .set_selection = mt9m114_ifp_set_selection,
> +       .get_mbus_config = mt9m114_ifp_get_mbus_config,
>         .get_frame_interval = mt9m114_ifp_get_frame_interval,
>         .set_frame_interval = mt9m114_ifp_set_frame_interval,
>  };
> --
> 2.34.1
>
>