Hi Luca,

On 3/3/2026 5:54 PM, Luca Ceresoli wrote:

Hello Damon,

On Tue Feb 10, 2026 at 8:12 AM CET, Damon Ding wrote:

quoted

Since the panel/bridge should logically be positioned behind the
Analogix bridge in the display pipeline, it makes sense to handle
the panel/bridge parsing on the Analogix side. Therefore, we add
a new API analogix_dp_finish_probe(), which combines the panel/bridge
parsing with component addition, to do it.

Signed-off-by: Damon Ding <redacted>
Reviewed-by: Dmitry Baryshkov <redacted>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> (on rk3588)

...

quoted

@@ -1581,6 +1583,52 @@ struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp)
  }
  EXPORT_SYMBOL_GPL(analogix_dp_get_aux);

+static int analogix_dp_aux_done_probing(struct drm_dp_aux *aux)
+{
+	struct analogix_dp_device *dp = to_dp(aux);
+	struct analogix_dp_plat_data *plat_data = dp->plat_data;
+	int port = plat_data->dev_type == EXYNOS_DP ? 0 : 1;
+	int ret;
+
+	/*
+	 * If drm_of_find_panel_or_bridge() returns -ENODEV, there may be no valid panel
+	 * or bridge nodes. The driver should go on for the driver-free bridge or the DP
+	 * mode applications.
+	 */
+	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, port, 0,
+					  &plat_data->panel, &plat_data->next_bridge);
+	if (ret && ret != -ENODEV)
+		return ret;
+
+	return component_add(dp->dev, plat_data->ops);
+}
+
+int analogix_dp_finish_probe(struct analogix_dp_device *dp)
+{
+	int ret;
+
+	ret = devm_of_dp_aux_populate_bus(&dp->aux, analogix_dp_aux_done_probing);
+	if (ret) {
+		/*
+		 * If devm_of_dp_aux_populate_bus() returns -ENODEV, the done_probing() will
+		 * not be called because there are no EP devices. Then the callback function
+		 * analogix_dp_aux_done_probing() will be called directly in order to support
+		 * the other valid DT configurations.
+		 *
+		 * NOTE: The devm_of_dp_aux_populate_bus() is allowed to return -EPROBE_DEFER.

Uhm, if it is allowed to return -EPROBE_DEFER...

quoted

+		 */
+		if (ret != -ENODEV) {
+			dev_err(dp->dev, "failed to populate aux bus\n");
+			return ret;
+		}

...then you shouldn't dev_err() when -EPROBE_DEFER is returned.

Either use dev_err_probe() (which would also simplify your code) or check
for if (ret != -ENODEV && ret != -EPROBE_DEFER).

Will fix in v10.

quoted

+
+		return analogix_dp_aux_done_probing(&dp->aux);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_finish_probe);
+
  MODULE_AUTHOR("Jingoo Han [off-list ref]");
  MODULE_DESCRIPTION("Analogix DP Core Driver");
  MODULE_LICENSE("GPL v2");

diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 3428ffff24c5..bae969dec63a 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h

@@ -30,6 +30,7 @@ struct analogix_dp_plat_data {
  	struct drm_bridge *next_bridge;
  	struct drm_encoder *encoder;
  	struct drm_connector *connector;
+	const struct component_ops *ops;

Is adding a new stored field a good idea? Can it be instead passed as an
argument to analogix_dp_finish_probe()?

Note I don't have a strong opinion here, just the added struct field seems
overkill for being used just once.

I agree this is a better approach, since the &component_ops is only used 
during probing.

quoted

@@ -49,5 +50,6 @@ int analogix_dp_stop_crc(struct drm_connector *connector);

  struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux);
  struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp);
+int analogix_dp_finish_probe(struct analogix_dp_device *dp);

  #endif /* _ANALOGIX_DP_H_ */

Best regards,
Damon