Skip to content

Commit

Permalink
kernel: fix crash for late mipi dsi start
Browse files Browse the repository at this point in the history
  • Loading branch information
facchinm committed Jun 13, 2024
1 parent ca996bd commit 0fbfbf9
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
From 847e95abdf801b28496e7ab064ca968db3ca6876 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <[email protected]>
Date: Thu, 21 Sep 2023 13:50:32 +0300
Subject: [PATCH] drm/mipi-dsi: Fix detach call without attach

It's been reported that DSI host driver's detach can be called without
the attach ever happening:

https://lore.kernel.org/all/[email protected]/

After reading the code, I think this is what happens:

We have a DSI host defined in the device tree and a DSI peripheral under
that host (i.e. an i2c device using the DSI as data bus doesn't exhibit
this behavior).

The host driver calls mipi_dsi_host_register(), which causes (via a few
functions) mipi_dsi_device_add() to be called for the DSI peripheral. So
now we have a DSI device under the host, but attach hasn't been called.

Normally the probing of the devices continues, and eventually the DSI
peripheral's driver will call mipi_dsi_attach(), attaching the
peripheral.

However, if the host driver's probe encounters an error after calling
mipi_dsi_host_register(), and before the peripheral has called
mipi_dsi_attach(), the host driver will do cleanups and return an error
from its probe function. The cleanups include calling
mipi_dsi_host_unregister().

mipi_dsi_host_unregister() will call two functions for all its DSI
peripheral devices: mipi_dsi_detach() and mipi_dsi_device_unregister().
The latter makes sense, as the device exists, but the former may be
wrong as attach has not necessarily been done.

To fix this, track the attached state of the peripheral, and only detach
from mipi_dsi_host_unregister() if the peripheral was attached.

Note that I have only tested this with a board with an i2c DSI
peripheral, not with a "pure" DSI peripheral.

However, slightly related, the unregister machinery still seems broken.
E.g. if the DSI host driver is unbound, it'll detach and unregister the
DSI peripherals. After that, when the DSI peripheral driver unbound
it'll call detach either directly or using the devm variant, leading to
a crash. And probably the driver will crash if it happens, for some
reason, to try to send a message via the DSI bus.

But that's another topic.

Tested-by: H. Nikolaus Schaller <[email protected]>
Acked-by: Maxime Ripard <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
Tested-by: Tony Lindgren <[email protected]>
Signed-off-by: Tomi Valkeinen <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
---
drivers/gpu/drm/drm_mipi_dsi.c | 17 +++++++++++++++--
include/drm/drm_mipi_dsi.h | 2 ++
2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index f25ddfe37498..39eb1df76b1b 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -346,7 +346,8 @@ static int mipi_dsi_remove_device_fn(struct device *dev, void *priv)
{
struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);

- mipi_dsi_detach(dsi);
+ if (dsi->attached)
+ mipi_dsi_detach(dsi);
mipi_dsi_device_unregister(dsi);

return 0;
@@ -369,11 +370,18 @@ EXPORT_SYMBOL(mipi_dsi_host_unregister);
int mipi_dsi_attach(struct mipi_dsi_device *dsi)
{
const struct mipi_dsi_host_ops *ops = dsi->host->ops;
+ int ret;

if (!ops || !ops->attach)
return -ENOSYS;

- return ops->attach(dsi->host, dsi);
+ ret = ops->attach(dsi->host, dsi);
+ if (ret)
+ return ret;
+
+ dsi->attached = true;
+
+ return 0;
}
EXPORT_SYMBOL(mipi_dsi_attach);

@@ -385,9 +393,14 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
{
const struct mipi_dsi_host_ops *ops = dsi->host->ops;

+ if (WARN_ON(!dsi->attached))
+ return -EINVAL;
+
if (!ops || !ops->detach)
return -ENOSYS;

+ dsi->attached = false;
+
return ops->detach(dsi->host, dsi);
}
EXPORT_SYMBOL(mipi_dsi_detach);
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 9054a5185e1a..31171914990a 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -168,6 +168,7 @@ struct mipi_dsi_device_info {
* struct mipi_dsi_device - DSI peripheral device
* @host: DSI host for this peripheral
* @dev: driver model device node for this peripheral
+ * @attached: the DSI device has been successfully attached
* @name: DSI peripheral chip type
* @channel: virtual channel assigned to the peripheral
* @format: pixel format for video mode
@@ -184,6 +185,7 @@ struct mipi_dsi_device_info {
struct mipi_dsi_device {
struct mipi_dsi_host *host;
struct device dev;
+ bool attached;

char name[DSI_DEV_NAME_SIZE];
unsigned int channel;
--
2.25.1

1 change: 1 addition & 0 deletions recipes-kernel/linux/linux-lmp-fslc-imx_%.bbappend
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ SRC_URI:append:portenta-x8 = " \
file://QUECTEL-0001-support-ec200a.patch \
file://DVFS-0001-Add_400MHz_200MHz_100MHz.patch \
file://0001-spi-test.patch \
file://0001-drm-mipi-dsi-Fix-detach-call-without-attach.patch \
"

0 comments on commit 0fbfbf9

Please sign in to comment.