| Message ID | 20250226153929.274562-4-mathis.foerst@mt.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | MT9M114 driver bugfix and improvements | expand |
Hi Mathis On Wed, 26 Feb 2025 at 15:45, Mathis Foerst <mathis.foerst@mt.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@mt.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 > >
Hi Mathis, On Wed, Feb 26, 2025 at 04:39:24PM +0100, Mathis Foerst 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 imx driver should really be fixed instead of working around a staging driver issue in sensor drivers.
Hi Sakari, thank you for your input. On Thu, Feb 27, 2025 at 08:53:23AM +0000, Sakari Ailus wrote: > Hi Mathis, > > On Wed, Feb 26, 2025 at 04:39:24PM +0100, Mathis Foerst 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 imx driver should really be fixed instead of working around a staging > driver issue in sensor drivers. Makes sense for me, I implemented a fix in imx-media-csi.c in and moved it to a new patch series as it's no longer related to the MT9M114 driver: https://lore.kernel.org/linux-media/20250305113802.897087-1-mathis.foerst@mt.com/ > > -- > Regards, > > Sakari Ailus Best regards, Mathis Foerst
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; + } 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, };
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 Signed-off-by: Mathis Foerst <mathis.foerst@mt.com> --- drivers/media/i2c/mt9m114.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)