diff mbox series

[1/3] ipu3-cio2: Return actual subdev format

Message ID 20201008204747.26320-2-sakari.ailus@linux.intel.com
State Superseded
Headers show
Series [1/3] ipu3-cio2: Return actual subdev format | expand

Commit Message

Sakari Ailus Oct. 8, 2020, 8:47 p.m. UTC
Return actual subdev format on ipu3-cio2 subdev pads. The earlier
implementation was based on an infinite recursion that exhausted the
stack.

Reported-by: Tsuchiya Yuto <kitakar@gmail.com>
Fixes: c2a6a07afe4a ("media: intel-ipu3: cio2: add new MIPI-CSI2 driver")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

Comments

Laurent Pinchart Oct. 9, 2020, 12:39 a.m. UTC | #1
Hi Sakari,

Thank you for the patch.

On Thu, Oct 08, 2020 at 11:47:45PM +0300, Sakari Ailus wrote:
> Return actual subdev format on ipu3-cio2 subdev pads. The earlier

> implementation was based on an infinite recursion that exhausted the

> stack.


A bad idea indeed :-)

> Reported-by: Tsuchiya Yuto <kitakar@gmail.com>

> Fixes: c2a6a07afe4a ("media: intel-ipu3: cio2: add new MIPI-CSI2 driver")

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> ---

>  drivers/media/pci/intel/ipu3/ipu3-cio2.c | 24 +++---------------------

>  1 file changed, 3 insertions(+), 21 deletions(-)

> 

> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c

> index 4e598e937dfe..afa472026ba4 100644

> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c

> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c

> @@ -1232,29 +1232,11 @@ static int cio2_subdev_get_fmt(struct v4l2_subdev *sd,

>  			       struct v4l2_subdev_format *fmt)

>  {

>  	struct cio2_queue *q = container_of(sd, struct cio2_queue, subdev);

> -	struct v4l2_subdev_format format;

> -	int ret;

>  

> -	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {

> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)

>  		fmt->format = *v4l2_subdev_get_try_format(sd, cfg, fmt->pad);

> -		return 0;

> -	}

> -

> -	if (fmt->pad == CIO2_PAD_SINK) {

> -		format.which = V4L2_SUBDEV_FORMAT_ACTIVE;

> -		ret = v4l2_subdev_call(sd, pad, get_fmt, NULL,

> -				       &format);

> -

> -		if (ret)

> -			return ret;

> -		/* update colorspace etc */

> -		q->subdev_fmt.colorspace = format.format.colorspace;

> -		q->subdev_fmt.ycbcr_enc = format.format.ycbcr_enc;

> -		q->subdev_fmt.quantization = format.format.quantization;

> -		q->subdev_fmt.xfer_func = format.format.xfer_func;

> -	}

> -

> -	fmt->format = q->subdev_fmt;


I'm pretty speechless. All I can say is

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


and let's forget this has ever existed :-)

> +	else

> +		fmt->format = q->subdev_fmt;

>  

>  	return 0;

>  }


-- 
Regards,

Laurent Pinchart
diff mbox series

Patch

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 4e598e937dfe..afa472026ba4 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1232,29 +1232,11 @@  static int cio2_subdev_get_fmt(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_format *fmt)
 {
 	struct cio2_queue *q = container_of(sd, struct cio2_queue, subdev);
-	struct v4l2_subdev_format format;
-	int ret;
 
-	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
 		fmt->format = *v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
-		return 0;
-	}
-
-	if (fmt->pad == CIO2_PAD_SINK) {
-		format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-		ret = v4l2_subdev_call(sd, pad, get_fmt, NULL,
-				       &format);
-
-		if (ret)
-			return ret;
-		/* update colorspace etc */
-		q->subdev_fmt.colorspace = format.format.colorspace;
-		q->subdev_fmt.ycbcr_enc = format.format.ycbcr_enc;
-		q->subdev_fmt.quantization = format.format.quantization;
-		q->subdev_fmt.xfer_func = format.format.xfer_func;
-	}
-
-	fmt->format = q->subdev_fmt;
+	else
+		fmt->format = q->subdev_fmt;
 
 	return 0;
 }