Message-ID: <CAD=FV=XyM0WHB8kpirqfnV2CY2KTrHd14pi5qeS+Sq7mjPphsA@mail.gmail.com>
Date: Tue, 2 Sep 2025 10:26:48 -0700
From: Doug Anderson <dianders@...omium.org>
To: John Ripple <john.ripple@...sight.com>
Cc: Laurent.pinchart@...asonboard.com, airlied@...il.com, 
	andrzej.hajda@...el.com, dri-devel@...ts.freedesktop.org, 
	jernej.skrabec@...il.com, jonas@...boo.se, linux-kernel@...r.kernel.org, 
	maarten.lankhorst@...ux.intel.com, mripard@...nel.org, 
	neil.armstrong@...aro.org, rfoss@...nel.org, simona@...ll.ch, 
	tzimmermann@...e.de
Subject: Re: [PATCH 2/2] drm/bridge: ti-sn65dsi86: break probe dependency loop

Hi,

On Tue, Sep 2, 2025 at 9:23 AM John Ripple <john.ripple@...sight.com> wrote:
>
> Hi,
>
> >Which i2c bridge are you talking about? You mean the one created by
> >i2c_add_adapter() in drm_dp_aux_register()? I guess I'm confused about
> >why the DSI probe routine would even be looking for that adapter.
>
> The i2c bridge I was seeing was created by the drm_bridge_add() function
> in the ti_sn_bridge_probe() function. Without moving the ti_sn_attach_host()
> function out of the ti_sn_bridge_probe() function I kept getting stuck in a
> loop during boot where the bridge would never come up. It's possible this
> could be a unique interaction with the hardware I'm using and the nwl-dsi
> driver.

Sorry, I still don't really know what i2c bridge you're talking about
here. At this point there are a number of different MIPI hosts that
are using ti-sn65dsi86 and they don't seem to run into this, so
probably digging into your MIPI host to see exactly what it's doing
makes sense. Where exactly is the nwl-dsi driver trying to acquire
this bridge and failing?


> >In any case, I don't _think_ your patch is valid. Specifically, if you
> >notice ti_sn_attach_host() can return "-EPROBE_DEFER". That's a valid
> >error code to return from a probe routine but I don't think it's a
> >valid error code to return from a bridge attach function, is it?
>
> What error code would you suggest?

You can't just change the error code. The problem here is that, in
general, there is no guarantee of the order that devices are probed in
Linux. The general solution for this in Linux is for drivers to find
all the devices that they depend on during their probe routine. If any
are missing then they return -EPROBE_DEFER and the system will try
again once more things are loaded. In the case of ti-sn65dsi86 we need
the MIPI host device so we find it at probe time. If it's not there
then we want to try again later.

The whole "try again" logic for -EPROBE_DEFER is only guaranteed in
certain contexts. Generally it's reserved for probe. ...but that logic
_could_ be extended to other contexts. It's possible it could be
extended to bridge attach, but one would have to make sure it actually
is (I haven't checked) and, as Maxime says, it should be documented.

I suppose it's also possible that when ti_sn_bridge_attach() is
called, it's guaranteed that of_find_mipi_dsi_host_by_node() won't
return NULL. If you can prove this by looking through the DRM code
_then_ you could probably make your change and just change the error
code.


To sum it up

1. Ideally you can fix the nwl-dsi driver to work however everyone
else is working.

2. If you can't then your commit message needs to prove that it's safe
to move the code to the "attach" routine. You either need to prove
that it's guaranteed that of_find_mipi_dsi_host_by_node() won't return
NULL when called from ti_sn_bridge_attach() or you need to prove that
returning -EPROBE_DEFER in this case is safe.

3. In either case, updating the docs that Maxime pointed to would be useful.


-Doug

Powered by blists - more mailing lists