Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <9daacd2f-0dbf-dddc-9403-b802447896a2@ideasonboard.com>
Date:   Wed, 13 Sep 2023 14:59:48 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.0
Subject: Re: [PATCH] drm/omap: dsi: Fix deferred probe warnings
Content-Language: en-US
To:     Tony Lindgren <tony@atomide.com>
Cc:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Sebastian Reichel <sre@kernel.org>,
        dri-devel@lists.freedesktop.org, linux-omap@vger.kernel.org,
        linux-kernel@vger.kernel.org, Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@gmail.com>
References: <20230412073954.20601-1-tony@atomide.com>
From:   Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
In-Reply-To: <20230412073954.20601-1-tony@atomide.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/04/2023 10:39, Tony Lindgren wrote:
> We may not have dsi->dsidev initialized during probe, and that can
> lead into various dsi related warnings as omap_dsi_host_detach() gets
> called with dsi->dsidev set to NULL.
> 
> The warnings can be "Fixed dependency cycle(s)" followed by a
> WARNING: CPU: 0 PID: 787 at drivers/gpu/drm/omapdrm/dss/dsi.c:4414.
> 
> Let's fix the warnings by checking for a valid dsi->dsidev.
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>   drivers/gpu/drm/omapdrm/dss/dsi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -4411,7 +4411,7 @@ static int omap_dsi_host_detach(struct mipi_dsi_host *host,
>   {
>   	struct dsi_data *dsi = host_to_omap(host);
>   
> -	if (WARN_ON(dsi->dsidev != client))
> +	if (dsi->dsidev && WARN_ON(dsi->dsidev != client))
>   		return -EINVAL;
>   
>   	cancel_delayed_work_sync(&dsi->dsi_disable_work);

Shouldn't this rather be

if (!dsi->dsidev)
	return 0;

before the if (WARN_ON(dsi->dsidev != client)) line?

If dsi->dsidev is NULL, then attach hasn't been called, and we shouldn't 
do anything in the detach callback either.

With your change we'll end up doing all the work in the detach callback, 
without ever doing their counterpart in the attach side.

  Tomi