Re: [PATCH] drm: bridge: thc63lvd1024: Print error message when DT parsing fails

From: Sui Jingfeng <sui.jingfeng@linux.dev>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: dri-devel@lists.freedesktop.org,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	 Neil Armstrong <neil.armstrong@linaro.org>,
	Robert Foss <rfoss@kernel.org>, Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@gmail.com>
Subject: Re: [PATCH] drm: bridge: thc63lvd1024: Print error message when DT parsing fails
Date: Tue, 19 Mar 2024 02:18:41 +0800	[thread overview]
Message-ID: <f3dc3c95-4346-44dd-9e16-3f07a48d4cda@linux.dev> (raw)
In-Reply-To: <20240318180420.GP13682@pendragon.ideasonboard.com>

Hi,

On 2024/3/19 02:04, Laurent Pinchart wrote:
> Hi Sui,
>
> On Tue, Mar 19, 2024 at 12:42:41AM +0800, Sui Jingfeng wrote:
>> On 2024/3/19 00:06, Laurent Pinchart wrote:
>>> Commit 00084f0c01bf ("drm: bridge: thc63lvd1024: Switch to use
>>> of_graph_get_remote_node()") simplified the thc63lvd1024 driver by
>>> replacing hand-rolled code with a helper function.
>> [...]
>>
>>> While doing so, it
>>> created an error code path at probe time without any error message,
>> If this is a reason or a concern, then every drm bridges drivers will suffer from
>> such a concern. Right?
> Yes, bridge drivers (or any driver, really) should avoid failing probe
> silently.

Yes, I agree with you that bridge drivers should avoid failing probe.

But the real problem that deserve to discuss is that is it really *silently* ?

The of_graph_get_remote_node() function do have debug prints on failure:

   - pr_debug("no valid endpoint (%d, %d) for node %pOF\n", port, endpoint, node);
   - pr_debug("no valid remote node\n");
   - pr_debug("not available for remote node\n");

So it is not really *silently*.