| Message ID | 1527285240-12762-1-git-send-email-khoroshilov@ispras.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, May 26, 2018 at 12:54:00AM +0300, Alexey Khoroshilov wrote: > of_graph_get_next_endpoint() returns device_node with refcnt increased, > but these is no of_node_put() for it. I think this is correct - but would it not be simpler to do endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep)); of_node_put(ep); if (IS_ERR(endpoint)) { .... As the of_node_put(np) actually is unconditional anyway I think this should be semantically equivalent. > > The patch adds one on error and normal paths. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> Reviewed-by: Nicholas Mc Guire <der.herr@hofr.at> > --- > drivers/media/i2c/tc358743.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c > index 393bbbbbaad7..44c41933415a 100644 > --- a/drivers/media/i2c/tc358743.c > +++ b/drivers/media/i2c/tc358743.c > @@ -1918,7 +1918,8 @@ static int tc358743_probe_of(struct tc358743_state *state) > endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep)); > if (IS_ERR(endpoint)) { > dev_err(dev, "failed to parse endpoint\n"); > - return PTR_ERR(endpoint); > + ret = PTR_ERR(endpoint); > + goto put_node; > } > > if (endpoint->bus_type != V4L2_MBUS_CSI2 || > @@ -2013,6 +2014,8 @@ static int tc358743_probe_of(struct tc358743_state *state) > clk_disable_unprepare(refclk); > free_endpoint: > v4l2_fwnode_endpoint_free(endpoint); > +put_node: > + of_node_put(ep); > return ret; > } > #else > -- > 2.7.4 > > _______________________________________________ > SIL2review mailing list > SIL2review@lists.osadl.org > https://lists.osadl.org/mailman/listinfo/sil2review
On 26.05.2018 17:38, Nicholas Mc Guire wrote: > On Sat, May 26, 2018 at 12:54:00AM +0300, Alexey Khoroshilov wrote: >> of_graph_get_next_endpoint() returns device_node with refcnt increased, >> but these is no of_node_put() for it. > > I think this is correct - but would it not be simpler to do > > endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep)); > of_node_put(ep); > if (IS_ERR(endpoint)) { > .... > > As the of_node_put(np) actually is unconditional anyway I think this > should be semantically equivalent. You are right. But the same is true for v4l2_fwnode_endpoint_free(endpoint); that is already correctly handled by the driver. So, I have preferred to follow the same pattern. > >> >> The patch adds one on error and normal paths. >> >> Found by Linux Driver Verification project (linuxtesting.org). >> >> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> > Reviewed-by: Nicholas Mc Guire <der.herr@hofr.at> Thank you, Alexey
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c index 393bbbbbaad7..44c41933415a 100644 --- a/drivers/media/i2c/tc358743.c +++ b/drivers/media/i2c/tc358743.c @@ -1918,7 +1918,8 @@ static int tc358743_probe_of(struct tc358743_state *state) endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep)); if (IS_ERR(endpoint)) { dev_err(dev, "failed to parse endpoint\n"); - return PTR_ERR(endpoint); + ret = PTR_ERR(endpoint); + goto put_node; } if (endpoint->bus_type != V4L2_MBUS_CSI2 || @@ -2013,6 +2014,8 @@ static int tc358743_probe_of(struct tc358743_state *state) clk_disable_unprepare(refclk); free_endpoint: v4l2_fwnode_endpoint_free(endpoint); +put_node: + of_node_put(ep); return ret; } #else
of_graph_get_next_endpoint() returns device_node with refcnt increased, but these is no of_node_put() for it. The patch adds one on error and normal paths. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> --- drivers/media/i2c/tc358743.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)