linux-kernel - Re: [PATCH v2 04/14] drm/bridge: ti-sn65dsi86: Reorder remove()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c70cdb6-7fab-dc53-121e-f355da1ea14f@samsung.com>
Date:   Wed, 31 Mar 2021 11:50:35 +0200
From:   Andrzej Hajda <a.hajda@...sung.com>
To:     Douglas Anderson <dianders@...omium.org>,
        Neil Armstrong <narmstrong@...libre.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Sam Ravnborg <sam@...nborg.org>
Cc:     robdclark@...omium.org, dri-devel@...ts.freedesktop.org,
        David Airlie <airlied@...ux.ie>, linux-arm-msm@...r.kernel.org,
        Steev Klimaszewski <steev@...i.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Stanislav Lisovskiy <stanislav.lisovskiy@...el.com>,
        Robert Foss <robert.foss@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 04/14] drm/bridge: ti-sn65dsi86: Reorder remove()


W dniu 30.03.2021 o 04:53, Douglas Anderson pisze:
> Let's make the remove() function strictly the reverse of the probe()
> function so it's easier to reason about.
>
> NOTES:
> - The MIPI calls probably belong in detach() but will be moved in a
>    separate patch.


The mipi is incorrectly handled already - mipi devices are searched 
after bridge registration - it should be reverse, there is comment in 
the driver that it is due to some dsi hosts, maybe it would be better to 
fix it there instead of conserve this bad design.


> - The cached EDID freeing isn't actually part of probe but needs to be
>    in remove to avoid orphaning memory until better handling of the
>    EDID happens.
> This patch was created by code inspection and should move us closer to
> a proper remove.
>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
>
> (no changes since v1)
>
>   drivers/gpu/drm/bridge/ti-sn65dsi86.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 76f43af6735d..c006678c9921 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -1315,20 +1315,21 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
>   	if (!pdata)
>   		return -EINVAL;
>   
> -	kfree(pdata->edid);
> -	ti_sn_debugfs_remove(pdata);
> -
> -	of_node_put(pdata->host_node);
> -
> -	pm_runtime_disable(pdata->dev);
> -
>   	if (pdata->dsi) {
>   		mipi_dsi_detach(pdata->dsi);
>   		mipi_dsi_device_unregister(pdata->dsi);
>   	}
>   
> +	kfree(pdata->edid);
> +
> +	ti_sn_debugfs_remove(pdata);
> +
>   	drm_bridge_remove(&pdata->bridge);
>   
> +	pm_runtime_disable(pdata->dev);
> +
> +	of_node_put(pdata->host_node);
> +


Looks good.

Reviewed-by: Andrzej Hajda <a.hajda@...sung.com>

Regards
Andrzej


>   	return 0;
>   }
>