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@xxxxxxxxxx>
> Suggested-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Signed-off-by: Adrien Grassein <adrien.grassein@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxx>
Regards
Andrzej
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
