Re: [PATCH 3/4] media: imx: imx-mipi-csis: Implement the .get_frame_desc() operation

[Rui Miguel Silva <rmfrfs@gmail.com>]

Hi Laurent,
On Thu, Jun 16, 2022 at 02:59:32AM +0300, Laurent Pinchart wrote:
> Hi Rui,
> 
> On Thu, Jun 16, 2022 at 12:53:14AM +0100, Rui Miguel Silva wrote:
> > On Wed, Jun 15, 2022 at 10:26:01PM +0300, Laurent Pinchart wrote:
> > > The CSIS is connected to its sink through an SoC-specific gasket that
> > > needs to be configured. Depending on the platform, the gasket
> > > configuration requires knowing the CSI-2 DT. To provide the needed
> > > information, implement the .get_frame_desc() operation.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  drivers/media/platform/nxp/imx-mipi-csis.c | 27 ++++++++++++++++++++++
> > >  1 file changed, 27 insertions(+)
> > > 
> > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > index 8674aaad5fa0..905072871ed2 100644
> > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > @@ -1160,6 +1160,32 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> > >  	return 0;
> > >  }
> > >  
> > > +static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> > > +				    struct v4l2_mbus_frame_desc *fd)
> > > +{
> > > +	struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > > +	struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[0];
> > 
> > Think that you should check *fd before use here, other than that.
> 
> fd isn't supposed to be null, the same way sd isn't, or the various
> format pointers passed to other subdev operations aren't. We don't check
> those either, that's why I haven't checked fd here.

Yeah, agree. Thanks.

Cheers,
   Rui

> 
> > Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>
> > 
> > > +
> > > +	if (pad != CSIS_PAD_SOURCE)
> > > +		return -EINVAL;
> > > +
> > > +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL;
> > > +	fd->num_entries = 1;
> > > +
> > > +	memset(entry, 0, sizeof(*entry));
> > > +
> > > +	mutex_lock(&csis->lock);
> > > +
> > > +	entry->flags = 0;
> > > +	entry->pixelcode = csis->csis_fmt->code;
> > > +	entry->bus.csi2.vc = 0;
> > > +	entry->bus.csi2.dt = csis->csis_fmt->data_type;
> > > +
> > > +	mutex_unlock(&csis->lock);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int mipi_csis_log_status(struct v4l2_subdev *sd)
> > >  {
> > >  	struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > > @@ -1184,6 +1210,7 @@ static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
> > >  	.enum_mbus_code		= mipi_csis_enum_mbus_code,
> > >  	.get_fmt		= mipi_csis_get_fmt,
> > >  	.set_fmt		= mipi_csis_set_fmt,
> > > +	.get_frame_desc		= mipi_csis_get_frame_desc,
> > >  };
> > >  
> > >  static const struct v4l2_subdev_ops mipi_csis_subdev_ops = {
> 
> -- 
> Regards,
> 
> Laurent Pinchart