[PATCH v9 10/15] drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()

[Damon Ding]

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:
>> 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 <damon.ding at rock-chips.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov at oss.qualcomm.com>
>> Tested-by: Marek Szyprowski <m.szyprowski at samsung.com>
>> Tested-by: Heiko Stuebner <heiko at sntech.de> (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).

Will fix in v10.

> 
>> +
>> +		return analogix_dp_aux_done_probing(&dp->aux);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(analogix_dp_finish_probe);
>> +
>>   MODULE_AUTHOR("Jingoo Han <jg1.han at samsung.com>");
>>   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.

>> @@ -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