linux-kernel - Re: [PATCH] samsung-dsim: move drm_bridge

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250728-diligent-brainy-hyena-109dde@houat>
Date: Mon, 28 Jul 2025 10:10:38 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: Inki Dae <inki.dae@...sung.com>, 
	Jagan Teki <jagan@...rulasolutions.com>, Marek Szyprowski <m.szyprowski@...sung.com>, 
	Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>, 
	Robert Foss <rfoss@...nel.org>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>, 
	Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Hui Pu <Hui.Pu@...ealthcare.com>, Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] samsung-dsim: move drm_bridge_add() call to probe

Hi,

On Fri, Jul 25, 2025 at 05:28:03PM +0200, Luca Ceresoli wrote:
> This bridge driver calls drm_bridge_add() in the DSI host .attach callback
> instead of in the probe function. This looks strange, even though
> apparently not a problem for currently supported use cases.
> 
> However it is a problem for supporting hotplug of DRM bridges, which is in
> the works [0][1][2]. The problematic case is when this DSI host is always
> present while its DSI device is hot-pluggable. In such case with the
> current code the DRM card will not be populated until after the DSI device
> attaches to the host, and which could happen a very long time after
> booting, or even not happen at all.
> 
> Supporting hotplug in the latest public draft is based on an ugly
> workaround in the hotplug-bridge driver code. This is visible in the
> hotplug_bridge_dsi_attach implementation and documentation in [3] (but
> keeping in mind that workaround is complicated as it is also circumventing
> another problem: updating the DSI host format when the DSI device gets
> connected).
> 
> As a preliminary step to supporting hotplug in a proper way, and also make
> this driver cleaner, move drm_bridge_add() at probe time, so that the
> bridge is available during boot.
> 
> However simply moving drm_bridge_add() prevents populating the whole card
> when the hot-pluggable addon is not present at boot, for another
> reason. The reason is:
> 
>  * now the encoder driver finds this bridge instead of getting
>    -EPROBE_DEFER as before
>  * but it cannot attach it because the bridge attach function in turn tries
>    to attach to the following bridge, which has not yet been hot-plugged
> 
> This needs to be fixed in the bridge attach function by simply returning
> -EPROBE_DEFER ifs the following bridge (i.e. the DSI device) is not yet
> present.
> 
> [0] https://lpc.events/event/18/contributions/1750/
> [1] https://lore.kernel.org/lkml/20240924174254.711c7138@booty/
> [2] https://lore.kernel.org/lkml/20250723-drm-bridge-alloc-getput-for_each_bridge-v1-0-be8f4ae006e9@bootlin.com/
> [3] https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-4-bc4dfee61be6@bootlin.com/
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>

There's many things lacking from that commit log to evaluate whether
it's a good solution or not:

- What is the typical sequence of probe / attach on your board?

- Why moving the call to drm_bridge_attach helps?

- What is the next bridge in your case? Did you try with a device
  controlled through DCS, or with a bridge connected through I2C/SPI
  that would typically have a lifetime disconnected from the DSI host.

- If you think it's a pattern that is generic enough, we must document
  it. If you don't, we must find something else.

- Why returning EPROBE_DEFER from the attach callback helps? Also, this
  is an undocumented behaviour, so if it does, we must document that
  it's acceptable.

Without that, unfortunately, we can't really review that patch.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)