Hi,
On Thu, Apr 21, 2022 at 9:00 AM Sankeerth Billakanti (QUIC) quic_sbillaka@quicinc.com wrote:
}/** External bridges are mandatory for eDP interfaces: one has to* provide at least an eDP panel (which gets wrapped into panel-bridge).
** For DisplayPort interfaces external bridges are optional, so* silently ignore an error if one is not present (-ENODEV).*/rc = dp_parser_find_next_bridge(dp_priv->parser);if (rc && dp->is_edp) {DRM_ERROR("eDP: cannot find the next bridge, rc = %d\n", rc);goto edp_error;} else if (rc && rc != -ENODEV) {DRM_ERROR("DP: cannot find the next bridge, rc = %d\n", rc);goto error;}The above wouldn't be my favorite way of doing this. Instead, I would have written:
if (rc) { DRM_ERROR("Cannot find the next bridge, rc = %d\n", rc); goto err; } ...
err: if (dp->is_edp) { disable_irq(...); dp_display_host_phy_exit(...); dp_display_host_deinit(...); } return rc;
If rc is ENODEV for DP, then we need to return 0. Shall I add like below ?
err: if (dp->is_edp) { disable_irq(...); dp_display_host_phy_exit(...); dp_display_host_deinit(...); } else If (rc == -ENODEV) rc = 0; return rc;
I wouldn't. Then you're essentially going to "err" for a case that you don't consider an error. I would instead have just handled it right away.
rc = dp_parser_find_next_bridge(dp_priv->parser); if (!dp->is_edp && rc == -ENODEV) return 0;
This also is better IMO because it means you aren't assuming that `dp_priv->parser->next_bridge` is "valid" (or at least NULL) after dp_parser_find_next_bridge() returned an error.
-Doug