On Mon, Oct 27, 2025 at 03:44:49AM +0200, Laurent Pinchart wrote:
> On Thu, Aug 21, 2025 at 04:15:47PM -0400, Frank Li wrote:
> > Move imx6's specific sd's owner, name, csi2_internal_ops, grp_id into
>
> grp_id needs to go. It's specific to the i.MX6 IPUv3 framework, and is
> the main part hindering splitting the CSI-2 receiver driver from the
> rest of the code.
>
> > imx6-sci2.c.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > drivers/staging/media/imx/imx6-csi2.c | 23 +++++++++++++++++++++++
> > drivers/staging/media/imx/imx6-mipi-csi2.c | 29 ++++++-----------------------
> > include/media/dw-mipi-csi2.h | 4 ++++
> > 3 files changed, 33 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/staging/media/imx/imx6-csi2.c b/drivers/staging/media/imx/imx6-csi2.c
> > index 72eff685fad99b3def46cf2866565191b7de9a8b..66274d8d73b67b35682bb82a9eb745bb24da7ae4 100644
> > --- a/drivers/staging/media/imx/imx6-csi2.c
> > +++ b/drivers/staging/media/imx/imx6-csi2.c
> > @@ -19,6 +19,29 @@ struct imx6_csi2 {
> > struct dw_mipi_csi2_dev dw;
> > };
> >
> > +static int csi2_registered(struct v4l2_subdev *sd)
> > +{
> > + struct dw_mipi_csi2_dev *csi2 = sd_to_dw_mipi_csi2_dev(sd);
> > +
> > + /* set a default mbus format */
> > + return imx_media_init_mbus_fmt(&csi2->format_mbus,
> > + IMX_MEDIA_DEF_PIX_WIDTH,
> > + IMX_MEDIA_DEF_PIX_HEIGHT, 0,
> > + V4L2_FIELD_NONE, NULL);
> > +}
> > +
> > +static const struct v4l2_subdev_internal_ops csi2_internal_ops = {
> > + .init_state = imx_media_init_state,
> > + .registered = csi2_registered,
Also, these two operations should be handled by the new driver. The imx6
media framework has a set of helper functions to handle initialization
of formats, with data structures shared between drivers. It's fine
duplicating some of the code, with a copy of the required data in the
new driver. It will be a good opportunity to both avoid historical
mistake with the new code (for instance, imx_media_init_state() should
really not be implemented as a wrapper around get subdev get_fmt
operation), and to remove code from the shared helpers that is
CSI-2-specific, if any.
> > +};
> > +
> > +static const struct dw_mipi_csi2_config imx6_config = {
> > + .module = THIS_MODULE,
> > + .name = "imx6-mipi-csi2",
> > + .internal_ops = &csi2_internal_ops,
> > + .grp_id = IMX_MEDIA_GRP_ID_CSI2,
> > +};
> > +
> > static int csi2_probe(struct platform_device *pdev)
> > {
> > struct imx6_csi2 *csi2;
> > diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > index b5911a37972be868e10c115ada1910fa04b2765c..ff06a739b957b305625d145c04c06999d0c13cae 100644
> > --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> > +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > @@ -15,11 +15,11 @@
> > #include <linux/platform_device.h>
> > #include <media/dw-mipi-csi2.h>
> > #include <media/v4l2-common.h>
> > +#include <media/v4l2-ctrls.h>
> > #include <media/v4l2-device.h>
> > #include <media/v4l2-fwnode.h>
> > #include <media/v4l2-mc.h>
> > #include <media/v4l2-subdev.h>
> > -#include "imx-media.h"
> >
> > /*
> > * The default maximum bit-rate per lane in Mbps, if the
> > @@ -44,8 +44,6 @@ struct dw_csi2_regs {
> > u32 phy_tst_ctrl1;
> > };
> >
> > -#define DEVICE_NAME "imx6-mipi-csi2"
> > -
> > /* Help check wrong access unexisted register at difference IP version */
> > #define DW_REG_EXIST 0x80000000
> > #define DW_REG(x) (DW_REG_EXIST | (x))
> > @@ -544,17 +542,6 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
> > return 0;
> > }
> >
> > -static int csi2_registered(struct v4l2_subdev *sd)
> > -{
> > - struct dw_mipi_csi2_dev *csi2 = sd_to_dev(sd);
> > -
> > - /* set a default mbus format */
> > - return imx_media_init_mbus_fmt(&csi2->format_mbus,
> > - IMX_MEDIA_DEF_PIX_WIDTH,
> > - IMX_MEDIA_DEF_PIX_HEIGHT, 0,
> > - V4L2_FIELD_NONE, NULL);
> > -}
> > -
> > /* --------------- CORE OPS --------------- */
> >
> > static int csi2_log_status(struct v4l2_subdev *sd)
> > @@ -605,11 +592,6 @@ static const struct v4l2_subdev_ops csi2_subdev_ops = {
> > .pad = &csi2_pad_ops,
> > };
> >
> > -static const struct v4l2_subdev_internal_ops csi2_internal_ops = {
> > - .init_state = imx_media_init_state,
> > - .registered = csi2_registered,
> > -};
> > -
> > static int csi2_notify_bound(struct v4l2_async_notifier *notifier,
> > struct v4l2_subdev *sd,
> > struct v4l2_async_connection *asd)
> > @@ -709,14 +691,15 @@ int dw_mipi_csi2_init(struct platform_device *pdev, struct dw_mipi_csi2_dev *csi
> > csi2->regs = &dw_csi2_v0;
> > v4l2_subdev_init(&csi2->sd, &csi2_subdev_ops);
> > v4l2_set_subdevdata(&csi2->sd, &pdev->dev);
> > - csi2->sd.internal_ops = &csi2_internal_ops;
> > csi2->sd.entity.ops = &csi2_entity_ops;
> > csi2->sd.dev = &pdev->dev;
> > - csi2->sd.owner = THIS_MODULE;
> > csi2->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> > - strscpy(csi2->sd.name, DEVICE_NAME, sizeof(csi2->sd.name));
> > csi2->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> > - csi2->sd.grp_id = IMX_MEDIA_GRP_ID_CSI2;
> > +
> > + csi2->sd.owner = config->module;
> > + strscpy(csi2->sd.name, config->name, sizeof(csi2->sd.name));
> > + csi2->sd.internal_ops = config->internal_ops;
> > + csi2->sd.grp_id = config->grp_id;
> >
> > for (i = 0; i < CSI2_NUM_PADS; i++) {
> > csi2->pad[i].flags = (i == CSI2_SINK_PAD) ?
> > diff --git a/include/media/dw-mipi-csi2.h b/include/media/dw-mipi-csi2.h
> > index 4e656f09c81db9dc9f232ed061f70803a5478c85..dfd25e23ea93fd53b064471b6a9557ef9c070fe7 100644
> > --- a/include/media/dw-mipi-csi2.h
> > +++ b/include/media/dw-mipi-csi2.h
> > @@ -10,6 +10,10 @@
> > #include <media/v4l2-subdev.h>
> >
> > struct dw_mipi_csi2_config {
> > + struct module *module;
> > + const char *name;
> > + int grp_id;
> > + const struct v4l2_subdev_internal_ops *internal_ops;
> > };
> >
> > /*
--
Regards,
Laurent Pinchart