From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6BA2C150984; Mon, 22 Jan 2024 15:18:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705936721; cv=none; b=DNWHDmdfjzZSw7MT5RZElVEVD3sL5kgeQsVSplN8E28nbVAg6xWiEuKdWuJvfGN7RrAo/iqewyqvZOsS5zksyl3nE4UH9+Fu9QoroKcDawsnBpoSwRvfAag7oOhfHbMNK57Q1s2dR+qC/jA1mD9PZUv2edRiFBGwaH5F09ZLimA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705936721; c=relaxed/simple; bh=tKbUPlybQDC0bQoV5Q3ibiocWcAvTi6dQJjdsKwUH/g=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ZGBdTZS920kwdlw4b4Vnd8b4+rLQns+gmnT11JBzXlbm5p32O7FA7ixqVPhLHY8Vvj/J8POCGoOEwA9jNT1rDtpVKGYV1IbKxPg1MJuDFSPiCHCYwa1ybHelmdY9ht3MwAqOqw/ELLZYcyYNIsjksa2dg4TDO9PtrBJBocXhWGw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=XJpsqXdl; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XJpsqXdl" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B1113C433C7; Mon, 22 Jan 2024 15:18:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705936721; bh=tKbUPlybQDC0bQoV5Q3ibiocWcAvTi6dQJjdsKwUH/g=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XJpsqXdlhG6DL1mRDlAIV2R28WmT+wNNpSN0knKC7qH1eVfr/8oyXV2PLGOLsLu1y QwPsuBYYHeXNGYHctpCw+AyGG6iz9Pm2xI0jmCiCmeC4xCFjRai7RsC7IXhl4k9mIR DNe98ZUTZj2a99y6vOsNGo1hD/722yxEKPcZut7aVVh0Plul+9TDTdjBC2QAoRDdZP GTNyr6m0wEEA1jXIjdCyd/r4gkdiKdN9q5EIvtizBHkal1DwhZs4xEmjkOxeuzDARZ PEfdlTw7Ia5RpDsYFGRU3PqUB0cnV51kJswd3XaEkolUwv5OkkMMx67Jfneg2vXrKn eQVq3ye+0esqA== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Tomi Valkeinen , "H . Nikolaus Schaller" , Maxime Ripard , Sebastian Reichel , Tony Lindgren , Sasha Levin , maarten.lankhorst@linux.intel.com, tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch, dri-devel@lists.freedesktop.org Subject: [PATCH AUTOSEL 4.19 08/23] drm/mipi-dsi: Fix detach call without attach Date: Mon, 22 Jan 2024 10:17:48 -0500 Message-ID: <20240122151823.997644-8-sashal@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240122151823.997644-1-sashal@kernel.org> References: <20240122151823.997644-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 4.19.305 Content-Transfer-Encoding: 8bit From: Tomi Valkeinen [ Upstream commit 90d50b8d85834e73536fdccd5aa913b30494fef0 ] It's been reported that DSI host driver's detach can be called without the attach ever happening: https://lore.kernel.org/all/20230412073954.20601-1-tony@atomide.com/ 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 Acked-by: Maxime Ripard Reviewed-by: Sebastian Reichel Tested-by: Tony Lindgren Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/20230921-dsi-detach-fix-v1-1-d0de2d1621d9@ideasonboard.com Signed-off-by: Sasha Levin --- 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 81923442b42d..2bd4e768b129 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -305,7 +305,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; @@ -328,11 +329,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); @@ -344,9 +352,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 689f615471ab..a059f1d968b7 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -163,6 +163,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 @@ -172,6 +173,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.43.0