[PATCH v2] media: imx: imx7-media-csi: Fix error handling in imx7_csi_async_register()

Frieder Schrempf posted 1 patch 2 years, 6 months ago
drivers/media/platform/nxp/imx7-media-csi.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
[PATCH v2] media: imx: imx7-media-csi: Fix error handling in imx7_csi_async_register()
Posted by Frieder Schrempf 2 years, 6 months ago
From: Frieder Schrempf <frieder.schrempf@kontron.de>

The CSI requires a connected source subdev to operate. If
fwnode_graph_get_endpoint_by_id() fails and returns NULL, there is no
point in going on. Print an error message and abort instead.

Also we don't need to check for an existing asd. Any failure of
v4l2_async_nf_add_fwnode_remote() should abort the probe.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes for v2:
* Improve commit message
* Use dev_err_probe
---
 drivers/media/platform/nxp/imx7-media-csi.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
index 886374d3a6ff..76ce6ff2c60a 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -2191,19 +2191,18 @@ static int imx7_csi_async_register(struct imx7_csi *csi)
 
 	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi->dev), 0, 0,
 					     FWNODE_GRAPH_ENDPOINT_NEXT);
-	if (ep) {
-		asd = v4l2_async_nf_add_fwnode_remote(&csi->notifier, ep,
-						      struct v4l2_async_subdev);
+	if (!ep)
+		return dev_err_probe(csi->dev, -ENOTCONN,
+				     "Failed to get remote endpoint\n");
 
-		fwnode_handle_put(ep);
+	asd = v4l2_async_nf_add_fwnode_remote(&csi->notifier, ep,
+					      struct v4l2_async_subdev);
 
-		if (IS_ERR(asd)) {
-			ret = PTR_ERR(asd);
-			/* OK if asd already exists */
-			if (ret != -EEXIST)
-				return ret;
-		}
-	}
+	fwnode_handle_put(ep);
+
+	if (IS_ERR(asd))
+		return dev_err_probe(csi->dev, PTR_ERR(asd),
+				     "Failed to add remote subdev to notifier\n");
 
 	csi->notifier.ops = &imx7_csi_notify_ops;
 
-- 
2.39.1
Re: [PATCH v2] media: imx: imx7-media-csi: Fix error handling in imx7_csi_async_register()
Posted by Laurent Pinchart 2 years, 6 months ago
Hi Frieder,

Thank you for the patch.

On Wed, Feb 15, 2023 at 06:16:38PM +0100, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> The CSI requires a connected source subdev to operate. If
> fwnode_graph_get_endpoint_by_id() fails and returns NULL, there is no
> point in going on. Print an error message and abort instead.
> 
> Also we don't need to check for an existing asd. Any failure of
> v4l2_async_nf_add_fwnode_remote() should abort the probe.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

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

> ---
> Changes for v2:
> * Improve commit message
> * Use dev_err_probe
> ---
>  drivers/media/platform/nxp/imx7-media-csi.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> index 886374d3a6ff..76ce6ff2c60a 100644
> --- a/drivers/media/platform/nxp/imx7-media-csi.c
> +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> @@ -2191,19 +2191,18 @@ static int imx7_csi_async_register(struct imx7_csi *csi)
>  
>  	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi->dev), 0, 0,
>  					     FWNODE_GRAPH_ENDPOINT_NEXT);
> -	if (ep) {
> -		asd = v4l2_async_nf_add_fwnode_remote(&csi->notifier, ep,
> -						      struct v4l2_async_subdev);
> +	if (!ep)
> +		return dev_err_probe(csi->dev, -ENOTCONN,
> +				     "Failed to get remote endpoint\n");
>  
> -		fwnode_handle_put(ep);
> +	asd = v4l2_async_nf_add_fwnode_remote(&csi->notifier, ep,
> +					      struct v4l2_async_subdev);
>  
> -		if (IS_ERR(asd)) {
> -			ret = PTR_ERR(asd);
> -			/* OK if asd already exists */
> -			if (ret != -EEXIST)
> -				return ret;
> -		}
> -	}
> +	fwnode_handle_put(ep);
> +
> +	if (IS_ERR(asd))
> +		return dev_err_probe(csi->dev, PTR_ERR(asd),
> +				     "Failed to add remote subdev to notifier\n");
>  
>  	csi->notifier.ops = &imx7_csi_notify_ops;
>  

-- 
Regards,

Laurent Pinchart