| Message ID | 20230214163200.1448837-1-frieder@fris.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: imx7-media-csi: Fix error handling in imx7_csi_async_register() | expand |
Hi Frieder, Thank you for the patch. On Tue, Feb 14, 2023 at 05:31:56PM +0100, Frieder Schrempf wrote: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > If fwnode_graph_get_endpoint_by_id() fails and returns NULL, there is > no point in going on. Print an error message a abort instead. s/a abort/and abort/ Explaining why as a reference to future readers would be nice: 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> > --- > drivers/media/platform/nxp/imx7-media-csi.c | 22 +++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c > index 886374d3a6ff..a7db310624e2 100644 > --- a/drivers/media/platform/nxp/imx7-media-csi.c > +++ b/drivers/media/platform/nxp/imx7-media-csi.c > @@ -2191,18 +2191,20 @@ 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) { > + dev_err(csi->dev, "Failed to get remote endpoint\n"); Let's use dev_err_probe() as this is called from the probe function: dev_err_probe(csi->dev, -ENOTCONN, "Failed to get remote endpoint\n"); > + return -ENOTCONN; > + } > > - 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)) { > + ret = PTR_ERR(asd); > + dev_err(csi->dev, "Failed to add remote subdev to notifier: %d\n", ret); Same here: dev_err_probe(csi->dev, ret, "Failed to add remote subdev to notifier\n"); > + return ret; > } > > csi->notifier.ops = &imx7_csi_notify_ops;
diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c index 886374d3a6ff..a7db310624e2 100644 --- a/drivers/media/platform/nxp/imx7-media-csi.c +++ b/drivers/media/platform/nxp/imx7-media-csi.c @@ -2191,18 +2191,20 @@ 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) { + dev_err(csi->dev, "Failed to get remote endpoint\n"); + return -ENOTCONN; + } - 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)) { + ret = PTR_ERR(asd); + dev_err(csi->dev, "Failed to add remote subdev to notifier: %d\n", ret); + return ret; } csi->notifier.ops = &imx7_csi_notify_ops;