Hi,

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?


potentially causing probe issues that get annoying to debug.

Sorry, let's keep it fair enough, it creates nothing annoyed.

If there is a probe issues, then, it is caused by ill-behavioral DT.
*NOT* my patch. And should be found during review stage.

If the of_graph_get_remote_node() function is not good enough,
I suggest to improve the of_graph_get_remote_node() function,
then all callers of it will benefits.

Well, the strong word here just terrifying new programmers to call
core function helpers. Please use more *soft* description in the
commit message.


Fix it by
adding an error message.

Fixes: 00084f0c01bf ("drm: bridge: thc63lvd1024: Switch to use 
of_graph_get_remote_node()")


Please drop the fixes tag at here, append the tag to a real bug-fix patch will 
make more sense imo.
I suggest to improve the of_graph_get_remote_node() function, then all callers 
of it will benefits.
NOT a single implement like this.

Signed-off-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com>
---
  drivers/gpu/drm/bridge/thc63lvd1024.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c 
b/drivers/gpu/drm/bridge/thc63lvd1024.c
index 5f99f9724081..674efc489e3a 100644
--- a/drivers/gpu/drm/bridge/thc63lvd1024.c
+++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
@@ -125,8 +125,11 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
remote = of_graph_get_remote_node(thc63->dev->of_node,
                                          THC63_RGB_OUT0, -1);
-       if (!remote)
+       if (!remote) {
+               dev_err(thc63->dev, "No remote endpoint for port@%u\n",
+                       THC63_RGB_OUT0);
                return -ENODEV;
+       }
thc63->next = of_drm_find_bridge(remote);
        of_node_put(remote);

base-commit: 00084f0c01bf3a2591d007010b196e048281c455

--
Best regards,
Sui

Reply via email to