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] [day] [month] [year] [list]
Message-id: <22343b9b-f9fb-200c-45e4-fd3380470864@samsung.com>
Date:   Wed, 18 Oct 2017 15:22:13 +0200
From:   Andrzej Hajda <a.hajda@...sung.com>
To:     Jeffy Chen <jeffy.chen@...k-chips.com>,
        linux-kernel@...r.kernel.org
Cc:     briannorris@...omium.org, seanpaul@...omium.org,
        dianders@...omium.org, heiko@...ech.de, tfiga@...omium.org,
        dri-devel@...ts.freedesktop.org, Caesar Wang <wxt@...k-chips.com>,
        David Airlie <airlied@...ux.ie>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        linux-samsung-soc@...r.kernel.org,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Seung-Woo Kim <sw0312.kim@...sung.com>,
        Inki Dae <inki.dae@...sung.com>,
        linux-rockchip@...ts.infradead.org,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        linux-arm-kernel@...ts.infradead.org,
        Kukjin Kim <kgene@...nel.org>,
        Tomeu Vizoso <tomeu.vizoso@...labora.com>,
        zain wang <wzz@...k-chips.com>,
        Archit Taneja <architt@...eaurora.org>,
        Joonyoung Shim <jy0922.shim@...sung.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Mark Yao <mark.yao@...k-chips.com>,
        Jingoo Han <jingoohan1@...il.com>
Subject: Re: [PATCH v5 4/9] drm/bridge: analogix_dp: Fix connector & encoder
 cleanup

On 18.10.2017 14:09, Jeffy Chen wrote:
> Since we are initing connector in the core driver and encoder in the
> plat driver, let's clean them up in the right places.
>
> Signed-off-by: Jeffy Chen <jeffy.chen@...k-chips.com>
> ---
>
> Changes in v5: None
>
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  2 --
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  7 +++++--
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    | 15 ++++++---------
>  3 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 74d274b6d31d..3f910ab36ff6 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1409,7 +1409,6 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
>  	ret = analogix_dp_create_bridge(drm_dev, dp);
>  	if (ret) {
>  		DRM_ERROR("failed to create bridge (%d)\n", ret);
> -		drm_encoder_cleanup(dp->encoder);
>  		goto err_disable_pm_runtime;
>  	}
>  
> @@ -1432,7 +1431,6 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
>  {
>  	analogix_dp_bridge_disable(dp->bridge);
>  	dp->connector.funcs->destroy(&dp->connector);
> -	dp->encoder->funcs->destroy(dp->encoder);
>  
>  	if (dp->plat_data->panel) {
>  		if (drm_panel_unprepare(dp->plat_data->panel))
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> index f7e5b2c405ed..33319a858f3a 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -185,8 +185,10 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>  	dp->plat_data.encoder = encoder;
>  
>  	dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
> -	if (IS_ERR(dp->adp))
> +	if (IS_ERR(dp->adp)) {
> +		dp->encoder.funcs->destroy(&dp->encoder);
>  		return PTR_ERR(dp->adp);
> +	}
>  
>  	return 0;
>  }
> @@ -196,7 +198,8 @@ static void exynos_dp_unbind(struct device *dev, struct device *master,
>  {
>  	struct exynos_dp_device *dp = dev_get_drvdata(dev);
>  
> -	return analogix_dp_unbind(dp->adp);
> +	analogix_dp_unbind(dp->adp);
> +	dp->encoder.funcs->destroy(&dp->encoder);
>  }
>  
>  static const struct component_ops exynos_dp_ops = {
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index fa0365de31d2..c0fb3f3748f4 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -261,13 +261,8 @@ static struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = {
>  	.atomic_check = rockchip_dp_drm_encoder_atomic_check,
>  };
>  
> -static void rockchip_dp_drm_encoder_destroy(struct drm_encoder *encoder)
> -{
> -	drm_encoder_cleanup(encoder);
> -}
> -
>  static struct drm_encoder_funcs rockchip_dp_encoder_funcs = {
> -	.destroy = rockchip_dp_drm_encoder_destroy,
> +	.destroy = drm_encoder_cleanup,
>  };
>  
>  static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
> @@ -361,12 +356,13 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
>  	dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
>  	INIT_WORK(&dp->psr_work, analogix_dp_psr_work);
>  
> -	rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set);
> -
>  	dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
> -	if (IS_ERR(dp->adp))
> +	if (IS_ERR(dp->adp)) {
> +		dp->encoder.funcs->destroy(&dp->encoder);
>  		return PTR_ERR(dp->adp);
> +	}
>  
> +	rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set);

You are changing here order of calls: psr_reg after bind, it does not
seem to be related to patch subject.
Anyway psr_register can fail and its result is not checked, but it can
be addressed in separate patch.
So maybe it would be better to leave the order as is, unless there is
reason for change it in one patch, in such case please explain it in
commit message.
Beside this:
Reviewed-by: Andrzej Hajda <a.hajda@...sung.com>

 --
Regards
Andrzej

>  	return 0;
>  }
>  
> @@ -377,6 +373,7 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master,
>  
>  	rockchip_drm_psr_unregister(&dp->encoder);
>  	analogix_dp_unbind(dp->adp);
> +	dp->encoder.funcs->destroy(&dp->encoder);
>  }
>  
>  static const struct component_ops rockchip_dp_component_ops = {


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ