diff mbox

[39/40] drm/rockchip: analogix_dp: Fix invalid implementation of unbind

Message ID 20180115171614.14474-40-thierry.escande@collabora.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Escande Jan. 15, 2018, 5:16 p.m. UTC 
From: Tomasz Figa <tfiga@chromium.org>

Current implementation of unbind dereferences the drvdata pointer
assuming that it's its own data, however the ownership belongs to the
analogix code, which means that the pointer is dereferenced with wrong
type. Fix this by using the recently added platform data .cleanup()
callback to do Rockchip-specific things at unbind.

Cc: Kristian H. Kristensen <hoegsberg@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
[seanpaul fixed conflict keeping rockchip_dp_unbind() preserving clk_unprepare]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Tomasz Figa Jan. 16, 2018, 2:10 a.m. UTC | #1 
Hi Thierry,

On Tue, Jan 16, 2018 at 2:16 AM, Thierry Escande
<thierry.escande@collabora.com> wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> Current implementation of unbind dereferences the drvdata pointer
> assuming that it's its own data, however the ownership belongs to the
> analogix code, which means that the pointer is dereferenced with wrong
> type. Fix this by using the recently added platform data .cleanup()
> callback to do Rockchip-specific things at unbind.

First of all, thanks a lot for taking care of these patches. Much appreciated.

One thing to note, though, is that the four patches (31, 32, 33 and 39
in this series), which we initially made for fixing the problems
related, were later reverted and replaced by a cleaner solution.
Please take a look at this series on our gerrit:

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/442390/
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/442392/
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/442391/

Perhaps we could skip this intermediate solution and just go with the
above 3 patches?

Best regards,
Tomasz
diff mbox

Patch

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index a6a3d6f1f127..eaa025730c28 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -161,6 +161,13 @@  static int rockchip_dp_get_modes(struct analogix_dp_plat_data *plat_data,
 	return 0;
 }
 
+static void rockchip_dp_cleanup(struct analogix_dp_plat_data *plat_data)
+{
+	struct rockchip_dp_device *dp = to_dp(plat_data);
+
+	rockchip_drm_psr_unregister(&dp->encoder);
+}
+
 static bool
 rockchip_dp_drm_encoder_mode_fixup(struct drm_encoder *encoder,
 				   const struct drm_display_mode *mode,
@@ -333,6 +340,7 @@  static int rockchip_dp_bind(struct device *dev, struct device *master,
 	dp->plat_data.power_on = rockchip_dp_poweron;
 	dp->plat_data.power_off = rockchip_dp_powerdown;
 	dp->plat_data.get_modes = rockchip_dp_get_modes;
+	dp->plat_data.cleanup = rockchip_dp_cleanup;
 
 	ret = rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set);
 	if (ret < 0)
@@ -358,7 +366,6 @@  static void rockchip_dp_unbind(struct device *dev, struct device *master,
 	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
 
 	analogix_dp_unbind(dp->adp);
-	rockchip_drm_psr_unregister(&dp->encoder);
 	dp->encoder.funcs->destroy(&dp->encoder);
 }