Re: [PATCH] [RFT] drm: adv7511/33: fix adv7511_cec_init() failure handling

From: John Stultz <john.stultz@linaro.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Archit Taneja <architt@codeaurora.org>,
	Xinliang Liu <xinliang.liu@linaro.org>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Sean Paul <seanpaul@chromium.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	David Airlie <airlied@linux.ie>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Bhumika Goyal <bhumirks@gmail.com>,
	Inki Dae <inki.dae@samsung.com>,
	dri-devel@lists.freedesktop.org,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] [RFT] drm: adv7511/33: fix adv7511_cec_init() failure handling
Date: Thu, 16 Nov 2017 13:50:22 -0800	[thread overview]
Message-ID: <CALAqxLWstbhPJVbR1hT1AGKMdpZ-GdMyjmDmqE78B0jr0D+-Sw@mail.gmail.com> (raw)
In-Reply-To: <20171115123823.1895515-1-arnd@arndb.de>

On Wed, Nov 15, 2017 at 4:37 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> An otherwise correct cleanup patch from Dan Carpenter turned a broken
> failure handling from a feature patch by Hans Verkuil into a kernel
> Oops, so bisection points to commit 7af35b0addbc ("drm/kirin: Checking
> for IS_ERR() instead of NULL") rather than 3b1b975003e4 ("drm:
> adv7511/33: add HDMI CEC support").
>
> I've managed to piece together several partial problems, though
> I'm still struggling with the bigger picture:
>
> adv7511_probe() registers a drm_bridge structure that was allocated
> with devm_kzalloc(). It calls adv7511_cec_init(), which fails for an
> unknown reason, which in turn triggers the registered structure to be
> removed.
>
> Elsewhere, kirin_drm_platform_probe() gets called, which calls
> of_graph_get_remote_node(), and that returns NULL. Before Dan's
> patch we would go on with a NULL pointer here and register that,
> now kirin_drm_platform_probe() fails with -ENODEV.
>
> In a third driver, dsi_parse_dt() calls drm_of_find_panel_or_bridge(),
> which after not finding a panel goes on to call of_drm_find_bridge(),
> and that crashes due to the earlier list corruption.
>
> This addresses the first issue by making sure that adv7511_probe()
> does not leave behind any corrupted list entries. This should
> get the system back to boot but needs testing. From my understanding,
> there is at least one more bug that needs to be resolved to actually
> get everything to work again.

So I've started hitting the issue this patch tries to address (now
that the related code landed in Linus' tree). The only issue is that
with this fix, I don't see graphics initializing properly, so I
suspect something is still wrong with the error handling (though what
exactly I'm not sure).

If you have a new version of the patch with Hans' feedback in it, I'd
be happy to re-test.

thanks
-john