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 DC0FB4F1F9; Mon, 22 Jan 2024 14:58:01 +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=1705935482; cv=none; b=e+2s4Ed8w6DaUT/r1A5BI+jZI1OOtWhAwckAEUP4cEWLbIvB/5Hkh8srQTGs//w7AukVSwp9a1vZY/bnGLnQb2XQsPnp8b+iASolQ6o381neiit5v2XtD/1yDwOT7llbffAR1k+GsyJT2FpY8Gs8nIAkBf10cpwE3U6Za4fYm44= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705935482; c=relaxed/simple; bh=LcM1J8JXu/WkuyLUAnmbSDWLN8uNldFny21YGjP87KM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=bwK/G8wSf+qwtX9lKa2hTQhyu6+uxCClolrBHt+0gQ2HuNpfhkqpyWJ1s6XbZ4ULfLg7EujWI62PaGiq9G6hIIdgDcBVqgzPCVbTCaRE3XjMPstk14hqc0kYMAqK41OnkTw/PyqkzKiVZuOVW42Y9IF64yLTtpdQyyXPffO/yIc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=FJKUF2EO; 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="FJKUF2EO" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DD556C433F1; Mon, 22 Jan 2024 14:57:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705935481; bh=LcM1J8JXu/WkuyLUAnmbSDWLN8uNldFny21YGjP87KM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FJKUF2EOpsmgOnafLiR9zHbmSaJUZJ3bHirmmtTqtOm3n411+ruuB1350r5x6UW0P X+38AXt7FdxjN7bwKSeuAIoJTaj0+AOcoOzFfi2NVIQblQW1nJAiid6HDVhTeoIqfx IefFAOdyh3dnOf8L79mwGgamNEH6nQYXWqzT/SdSPNHOI3wffsfo8wD3c/bRF9hMKo MzIu7ZjkdCU4Dp7CkKIG6XKjl0HLHukJZsgswMPr6VQJ6oaKVZiwLNcRNMVYKQnAdL +DuJLNlnhmXkt+U7f7NwNrbZneUTie1jD/zKa22QjhvgCf/oIf8poTcw3Z45daQqBm jrInyeIVJweJA== 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 6.7 26/88] drm/mipi-dsi: Fix detach call without attach Date: Mon, 22 Jan 2024 09:50:59 -0500 Message-ID: <20240122145608.990137-26-sashal@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240122145608.990137-1-sashal@kernel.org> References: <20240122145608.990137-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 6.7.1 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 14201f73aab1..843a6dbda93a 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -347,7 +347,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; @@ -370,11 +371,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); @@ -386,9 +394,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 c9df0407980c..c0aec0d4d664 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.43.0