Hi Alexander and Dorota, Thank you for the patch. On Fri, Feb 11, 2022 at 03:27:44PM +0100, Alexander Stein wrote: > From: Dorota Czaplejewicz <dorota.czaplejewicz@xxxxxxx> > > The driver covers i.MX5/6, as well as i.MX7/8 hardware. > Those implementations differ, e.g. in the sizes of buffers they accept. > > Some functionality should be abstracted, and storing type achieves that. As much as I'd love it one of you would pick https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/csi-bridge/destage and bring it to completion, I won't ask for yak shaving. Could you however mention in the commit message that this is a temporary solution ? Maybe as follows: This is a temporary solution until the imx7-media-csi driver gets decoupled from the helpers shared with the i.MX6 implementation, and should be reverted once this happens. With that, Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@xxxxxxx> > Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx> > --- > Changes in v2: > * Switch back to using enum > > drivers/staging/media/imx/imx-ic-prpencvf.c | 3 ++- > drivers/staging/media/imx/imx-media-capture.c | 5 ++++- > drivers/staging/media/imx/imx-media-csi.c | 3 ++- > drivers/staging/media/imx/imx-media.h | 8 +++++++- > drivers/staging/media/imx/imx7-media-csi.c | 3 ++- > 5 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c > index 9b81cfbcd777..671bb9a681aa 100644 > --- a/drivers/staging/media/imx/imx-ic-prpencvf.c > +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c > @@ -1266,7 +1266,8 @@ static int prp_registered(struct v4l2_subdev *sd) > > priv->vdev = imx_media_capture_device_init(ic_priv->ipu_dev, > &ic_priv->sd, > - PRPENCVF_SRC_PAD, true); > + PRPENCVF_SRC_PAD, true, > + DEVICE_TYPE_IMX56); > if (IS_ERR(priv->vdev)) > return PTR_ERR(priv->vdev); > > diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c > index 93ba09236010..65dc95a48ecc 100644 > --- a/drivers/staging/media/imx/imx-media-capture.c > +++ b/drivers/staging/media/imx/imx-media-capture.c > @@ -34,6 +34,7 @@ struct capture_priv { > > struct imx_media_video_dev vdev; /* Video device */ > struct media_pad vdev_pad; /* Video device pad */ > + enum imx_media_device_type type; /* Type of hardware implementation */ > > struct v4l2_subdev *src_sd; /* Source subdev */ > int src_sd_pad; /* Source subdev pad */ > @@ -957,7 +958,8 @@ EXPORT_SYMBOL_GPL(imx_media_capture_device_unregister); > > struct imx_media_video_dev * > imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd, > - int pad, bool legacy_api) > + int pad, bool legacy_api, > + enum imx_media_device_type type) > { > struct capture_priv *priv; > struct video_device *vfd; > @@ -972,6 +974,7 @@ imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd, > priv->src_sd_pad = pad; > priv->dev = dev; > priv->legacy_api = legacy_api; > + priv->type = type; > > mutex_init(&priv->mutex); > INIT_LIST_HEAD(&priv->ready_q); > diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c > index bd7f156f2d52..d5557bb4913d 100644 > --- a/drivers/staging/media/imx/imx-media-csi.c > +++ b/drivers/staging/media/imx/imx-media-csi.c > @@ -1803,7 +1803,8 @@ static int csi_registered(struct v4l2_subdev *sd) > } > > priv->vdev = imx_media_capture_device_init(priv->sd.dev, &priv->sd, > - CSI_SRC_PAD_IDMAC, true); > + CSI_SRC_PAD_IDMAC, true, > + DEVICE_TYPE_IMX56); > if (IS_ERR(priv->vdev)) { > ret = PTR_ERR(priv->vdev); > goto free_fim; > diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h > index f263fc3adbb9..e4c22b3ccd57 100644 > --- a/drivers/staging/media/imx/imx-media.h > +++ b/drivers/staging/media/imx/imx-media.h > @@ -96,6 +96,11 @@ enum imx_pixfmt_sel { > PIXFMT_SEL_ANY = PIXFMT_SEL_YUV | PIXFMT_SEL_RGB | PIXFMT_SEL_BAYER, > }; > > +enum imx_media_device_type { > + DEVICE_TYPE_IMX56, > + DEVICE_TYPE_IMX78, > +}; > + > struct imx_media_buffer { > struct vb2_v4l2_buffer vbuf; /* v4l buffer must be first */ > struct list_head list; > @@ -282,7 +287,8 @@ int imx_media_ic_unregister(struct v4l2_subdev *sd); > /* imx-media-capture.c */ > struct imx_media_video_dev * > imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd, > - int pad, bool legacy_api); > + int pad, bool legacy_api, > + enum imx_media_device_type type); > void imx_media_capture_device_remove(struct imx_media_video_dev *vdev); > int imx_media_capture_device_register(struct imx_media_video_dev *vdev, > u32 link_flags); > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c > index 32311fc0e2a4..173dd014c2d6 100644 > --- a/drivers/staging/media/imx/imx7-media-csi.c > +++ b/drivers/staging/media/imx/imx7-media-csi.c > @@ -1039,7 +1039,8 @@ static int imx7_csi_registered(struct v4l2_subdev *sd) > } > > csi->vdev = imx_media_capture_device_init(csi->sd.dev, &csi->sd, > - IMX7_CSI_PAD_SRC, false); > + IMX7_CSI_PAD_SRC, false, > + DEVICE_TYPE_IMX78); > if (IS_ERR(csi->vdev)) > return PTR_ERR(csi->vdev); > -- Regards, Laurent Pinchart
