* [PATCH v4 0/1] Fixes 30e2ae943c26 "drm/bridge: Introduce LT8912B DSI to HDMI" @ 2021-03-31 11:49 Adrien Grassein 2021-03-31 11:49 ` [PATCH v4 1/1] drm/bridge: lt8912b: Fix issues found during static analysis Adrien Grassein 0 siblings, 1 reply; 5+ messages in thread From: Adrien Grassein @ 2021-03-31 11:49 UTC (permalink / raw) Cc: dri-devel, dan.carpenter, Adrien Grassein Hi, This patch fixes issues found by a static checker. Thanks, Adrien Grassein (1): drm/bridge: lt8912b: Fix issues found during static analysis drivers/gpu/drm/bridge/lontium-lt8912b.c | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) -- 2.25.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 1/1] drm/bridge: lt8912b: Fix issues found during static analysis 2021-03-31 11:49 [PATCH v4 0/1] Fixes 30e2ae943c26 "drm/bridge: Introduce LT8912B DSI to HDMI" Adrien Grassein @ 2021-03-31 11:49 ` Adrien Grassein 2021-03-31 12:14 ` Dan Carpenter 2021-03-31 12:57 ` Andrzej Hajda 0 siblings, 2 replies; 5+ messages in thread From: Adrien Grassein @ 2021-03-31 11:49 UTC (permalink / raw) Cc: dri-devel, dan.carpenter, Adrien Grassein Some issues where found during static analysis of this driver. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Suggested-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> --- drivers/gpu/drm/bridge/lontium-lt8912b.c | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c index 61491615bad0..4c8d79142262 100644 --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c @@ -621,7 +621,8 @@ static int lt8912_parse_dt(struct lt8912 *lt) { struct gpio_desc *gp_reset; struct device *dev = lt->dev; - int ret = 0; + int ret; + int data_lanes; struct device_node *port_node; struct device_node *endpoint; @@ -635,13 +636,16 @@ static int lt8912_parse_dt(struct lt8912 *lt) lt->gp_reset = gp_reset; endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); - if (IS_ERR(endpoint)) { - ret = PTR_ERR(endpoint); - goto end; - } + if (!endpoint) + return -ENODEV; - lt->data_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); + data_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); of_node_put(endpoint); + if (data_lanes < 0) { + dev_err(lt->dev, "%s: Bad data-lanes property\n", __func__); + return data_lanes; + } + lt->data_lanes = data_lanes; lt->host_node = of_graph_get_remote_node(dev->of_node, 0, -1); if (!lt->host_node) { @@ -658,19 +662,22 @@ static int lt8912_parse_dt(struct lt8912 *lt) } lt->hdmi_port = of_drm_find_bridge(port_node); - if (IS_ERR(lt->hdmi_port)) { + if (!lt->hdmi_port) { dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); - ret = PTR_ERR(lt->hdmi_port); - of_node_put(lt->host_node); - goto end; + ret = -ENODEV; + of_node_put(port_node); + goto err_free_host_node; } if (!of_device_is_compatible(port_node, "hdmi-connector")) { dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); + of_node_put(port_node); ret = -EINVAL; + goto err_free_host_node; } of_node_put(port_node); + return 0; end: return ret; -- 2.25.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/1] drm/bridge: lt8912b: Fix issues found during static analysis 2021-03-31 11:49 ` [PATCH v4 1/1] drm/bridge: lt8912b: Fix issues found during static analysis Adrien Grassein @ 2021-03-31 12:14 ` Dan Carpenter 2021-03-31 12:57 ` Andrzej Hajda 1 sibling, 0 replies; 5+ messages in thread From: Dan Carpenter @ 2021-03-31 12:14 UTC (permalink / raw) To: Adrien Grassein; +Cc: dri-devel Thanks! regards, dan carpenter _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/1] drm/bridge: lt8912b: Fix issues found during static analysis 2021-03-31 11:49 ` [PATCH v4 1/1] drm/bridge: lt8912b: Fix issues found during static analysis Adrien Grassein 2021-03-31 12:14 ` Dan Carpenter @ 2021-03-31 12:57 ` Andrzej Hajda 2021-03-31 14:39 ` Dan Carpenter 1 sibling, 1 reply; 5+ messages in thread From: Andrzej Hajda @ 2021-03-31 12:57 UTC (permalink / raw) To: Adrien Grassein; +Cc: dan.carpenter, dri-devel W dniu 31.03.2021 o 13:49, Adrien Grassein pisze: > Some issues where found during static analysis of this driver. Subject should describe what has been fixed, description why. If there is multiple different issues maybe patch split would be better. > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Suggested-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> > --- > drivers/gpu/drm/bridge/lontium-lt8912b.c | 27 +++++++++++++++--------- > 1 file changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c > index 61491615bad0..4c8d79142262 100644 > --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c > +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c > @@ -621,7 +621,8 @@ static int lt8912_parse_dt(struct lt8912 *lt) > { > struct gpio_desc *gp_reset; > struct device *dev = lt->dev; > - int ret = 0; > + int ret; > + int data_lanes; > struct device_node *port_node; > struct device_node *endpoint; > > @@ -635,13 +636,16 @@ static int lt8912_parse_dt(struct lt8912 *lt) > lt->gp_reset = gp_reset; > > endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); > - if (IS_ERR(endpoint)) { > - ret = PTR_ERR(endpoint); > - goto end; > - } > + if (!endpoint) > + return -ENODEV; > > - lt->data_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); > + data_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); > of_node_put(endpoint); > + if (data_lanes < 0) { > + dev_err(lt->dev, "%s: Bad data-lanes property\n", __func__); > + return data_lanes; > + } > + lt->data_lanes = data_lanes; > > lt->host_node = of_graph_get_remote_node(dev->of_node, 0, -1); > if (!lt->host_node) { > @@ -658,19 +662,22 @@ static int lt8912_parse_dt(struct lt8912 *lt) > } > > lt->hdmi_port = of_drm_find_bridge(port_node); > - if (IS_ERR(lt->hdmi_port)) { > + if (!lt->hdmi_port) { > dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > - ret = PTR_ERR(lt->hdmi_port); > - of_node_put(lt->host_node); > - goto end; > + ret = -ENODEV; > + of_node_put(port_node); > + goto err_free_host_node; > } > > if (!of_device_is_compatible(port_node, "hdmi-connector")) { > dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > + of_node_put(port_node); > ret = -EINVAL; > + goto err_free_host_node; Maybe better would be to put of_node_put(port_node) after err_free_host_node label - of_node_put(NULL) does nothing. > } > > of_node_put(port_node); > + return 0; > > end: > return ret; This label and code can be removed, am I right? After reading the body I know what the patch does, so I can propose the subject, what about "fix incorrect handling of of_* return values". And please make description more descriptive :) With that fixed you can add my: Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> Regards Andrzej _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/1] drm/bridge: lt8912b: Fix issues found during static analysis 2021-03-31 12:57 ` Andrzej Hajda @ 2021-03-31 14:39 ` Dan Carpenter 0 siblings, 0 replies; 5+ messages in thread From: Dan Carpenter @ 2021-03-31 14:39 UTC (permalink / raw) To: Andrzej Hajda; +Cc: Adrien Grassein, dri-devel On Wed, Mar 31, 2021 at 02:57:31PM +0200, Andrzej Hajda wrote: > > > > if (!of_device_is_compatible(port_node, "hdmi-connector")) { > > dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > > + of_node_put(port_node); > > ret = -EINVAL; > > + goto err_free_host_node; > > Maybe better would be to put of_node_put(port_node) after > err_free_host_node label - of_node_put(NULL) does nothing. > I prefer this style for several reasons: 1) I kind of hate no-op puts 2) The port_node is not part of the wind up in the sense that we don't leave hold the reference on the success path. It's the same with locking, I prefer if unlock happens before the goto. Sometimes people do an unlock as part of the unwind but this style only works for the first label in the unwind path. 3) I like when you can copy and paste the unwind code to create the release function, lt8912_put_dt() in this example. Normally you have to add one more release function because most times the last failure path is an allocation but in this case the release is just of_node_put(lt->host_node); But, yeah, putting the of_node_put() after the label isn't bug so whatever... regards, dan carpenter _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-31 14:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-31 11:49 [PATCH v4 0/1] Fixes 30e2ae943c26 "drm/bridge: Introduce LT8912B DSI to HDMI" Adrien Grassein 2021-03-31 11:49 ` [PATCH v4 1/1] drm/bridge: lt8912b: Fix issues found during static analysis Adrien Grassein 2021-03-31 12:14 ` Dan Carpenter 2021-03-31 12:57 ` Andrzej Hajda 2021-03-31 14:39 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox