[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b3wltiggcukzseiv4vixy3tbhel3c5htzfgr77yjolb3z7wrh7@h746u326byrb>
Date: Wed, 6 Dec 2023 13:59:26 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: "H. Nikolaus Schaller" <hns@...delico.com>,
Tony Lindgren <tony@...mide.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Andrzej Hajda <andrzej.hajda@...el.com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH] drm/mipi-dsi: Fix detach call without attach
Hi,
On Wed, Dec 06, 2023 at 02:48:58PM +0200, Tomi Valkeinen wrote:
> Hi mipi dsi maintainers (I'm not sure who that is =),
>
> On 21/09/2023 13:50, Tomi Valkeinen wrote:
> > 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.
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> > ---
>
> Any comments? I can push this via drm-misc, but I'd like an ack.
An ack is a requirement, it's not optional.
Acked-by: Maxime Ripard <mripard@...nel.org>
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists