Hello Damon,
On Tue Feb 10, 2026 at 8:12 AM CET, Damon Ding wrote:
> 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 <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Tested-by: Marek Szyprowski <[email protected]>
> Tested-by: Heiko Stuebner <[email protected]> (on rk3588)
...
> @@ -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...
> + */
> + 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).
> +
> + return analogix_dp_aux_done_probing(&dp->aux);
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_finish_probe);
> +
> MODULE_AUTHOR("Jingoo Han <[email protected]>");
> 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.
> @@ -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_ */
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
