Re: [PATCH 21/28] media: ti-vpe: cal: fix cal_ctx_v4l2_register error handling

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

On Mon, Apr 12, 2021 at 02:34:50PM +0300, Tomi Valkeinen wrote:
> cal_ctx_v4l2_register() returns an error code, but the returned error
> code is not handled anywhere. However, we can't really even handle the
> error in any proper way, so lets just drop the return value and make
> sure all error paths have an error print.

Ouch. Doesn't this call for registering the video node earlier, at probe
time, instead of in the async notifier complete callback ?

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/platform/ti-vpe/cal-video.c | 16 ++++++++--------
>  drivers/media/platform/ti-vpe/cal.h       |  2 +-
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal-video.c b/drivers/media/platform/ti-vpe/cal-video.c
> index 064efdc31b28..ea9b13c16a06 100644
> --- a/drivers/media/platform/ti-vpe/cal-video.c
> +++ b/drivers/media/platform/ti-vpe/cal-video.c
> @@ -864,14 +864,16 @@ static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
>  	return 0;
>  }
>  
> -int cal_ctx_v4l2_register(struct cal_ctx *ctx)
> +void cal_ctx_v4l2_register(struct cal_ctx *ctx)
>  {
>  	struct video_device *vfd = &ctx->vdev;
>  	int ret;
>  
>  	ret = cal_ctx_v4l2_init_formats(ctx);
> -	if (ret)
> -		return ret;
> +	if (ret) {
> +		ctx_err(ctx, "Failed to init formats: %d\n", ret);
> +		return;
> +	}
>  
>  	if (!cal_mc_api) {
>  		struct v4l2_ctrl_handler *hdl = &ctx->ctrl_handler;
> @@ -880,14 +882,14 @@ int cal_ctx_v4l2_register(struct cal_ctx *ctx)
>  					    NULL, true);
>  		if (ret < 0) {
>  			ctx_err(ctx, "Failed to add source ctrl handler\n");
> -			return ret;
> +			return;
>  		}
>  	}
>  
>  	ret = video_register_device(vfd, VFL_TYPE_VIDEO, cal_video_nr);
>  	if (ret < 0) {
>  		ctx_err(ctx, "Failed to register video device\n");
> -		return ret;
> +		return;
>  	}
>  
>  	ret = media_create_pad_link(&ctx->phy->subdev.entity,
> @@ -899,13 +901,11 @@ int cal_ctx_v4l2_register(struct cal_ctx *ctx)
>  		ctx_err(ctx, "Failed to create media link for context %u\n",
>  			ctx->dma_ctx);
>  		video_unregister_device(vfd);
> -		return ret;
> +		return;
>  	}
>  
>  	ctx_info(ctx, "V4L2 device registered as %s\n",
>  		 video_device_node_name(vfd));
> -
> -	return 0;
>  }
>  
>  void cal_ctx_v4l2_unregister(struct cal_ctx *ctx)
> diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
> index 8aa93c92193a..ad7d26c803eb 100644
> --- a/drivers/media/platform/ti-vpe/cal.h
> +++ b/drivers/media/platform/ti-vpe/cal.h
> @@ -310,7 +310,7 @@ void cal_ctx_set_dma_addr(struct cal_ctx *ctx, dma_addr_t addr);
>  void cal_ctx_start(struct cal_ctx *ctx);
>  void cal_ctx_stop(struct cal_ctx *ctx);
>  
> -int cal_ctx_v4l2_register(struct cal_ctx *ctx);
> +void cal_ctx_v4l2_register(struct cal_ctx *ctx);
>  void cal_ctx_v4l2_unregister(struct cal_ctx *ctx);
>  int cal_ctx_v4l2_init(struct cal_ctx *ctx);
>  void cal_ctx_v4l2_cleanup(struct cal_ctx *ctx);

-- 
Regards,

Laurent Pinchart