Re: [PATCH v7 2/9] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: Chaoyi Chen <hidden>
Date: 2025-10-23 12:10:36
Also in:
dri-devel, linux-devicetree, linux-phy, linux-rockchip, linux-usb, lkml
Hi Heikki, On 10/23/2025 8:03 PM, Heikki Krogerus wrote:
quoted hunk ↗ jump to hunk
quoted
quoted
quoted
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 245e8a27e3fc..e91736829167 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile@@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += aux-hpd-bridge.o +obj-$(CONFIG_DRM_AUX_TYPEC_DP_HPD_BRIDGE) += aux-hpd-typec-dp-bridge.oInstead, why not just make that a part of aux-hpd-bridge conditionally: ifneq ($(CONFIG_TYPEC),) aux-hpd-bridge-y += aux-hpd-typec-dp-bridge.o endifOh, I did consider that! But I noticed that aux-hpd-bridge.c contains the following statement module_auxiliary_driver(drm_aux_hpd_bridge_drv), which already includes a module_init. In the newly added file, in order to call the register function, another module_init was also added. If the two files are each made into a module separately, would there be a problem?You would not call module_init() from the new file. Instead you would call drm_aux_hpd_typec_dp_bridge_init() and what ever directly from aux-hpd-bridge.c:diff --git a/drivers/gpu/drm/bridge/aux-bridge.h b/drivers/gpu/drm/bridge/aux-bridge.h new file mode 100644 index 000000000000..ae689a7778fa --- /dev/null +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.h@@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef AUX_HPD_BRIDGE_H +#define AUX_HPD_BRIDGE_H + +#if IS_ENABLED(CONFIG_TYPEC) +int drm_aux_hpd_typec_dp_bridge_init(void); +void drm_aux_hpd_typec_dp_bridge_exit(void); +#else +static inline int drm_aux_hpd_typec_dp_bridge_init(void) { return 0; } +static inline void drm_aux_hpd_typec_dp_bridge_exit(void) { } +#endif /* IS_ENABLED(CONFIG_TYPEC) */ + +#endif /* AUX_HPD_BRIDGE_H */diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-bridge.c index 2e9c702c7087..3578df1df78a 100644 --- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c@@ -12,6 +12,8 @@ #include <drm/drm_bridge.h> #include <drm/bridge/aux-bridge.h> +#include "aux-hpd-bridge.h" + static DEFINE_IDA(drm_aux_hpd_bridge_ida); struct drm_aux_hpd_bridge_data {@@ -190,9 +192,16 @@ static int drm_aux_hpd_bridge_probe(struct auxiliary_device *auxdev, auxiliary_set_drvdata(auxdev, data); + drm_aux_hpd_typec_dp_bridge_init(); + return devm_drm_bridge_add(data->dev, &data->bridge); } +static void drm_aux_hpd_bridge_remove(struct auxiliary_device *auxdev) +{ + drm_aux_hpd_typec_dp_bridge_exit(); +} + static const struct auxiliary_device_id drm_aux_hpd_bridge_table[] = { { .name = KBUILD_MODNAME ".dp_hpd_bridge", .driver_data = DRM_MODE_CONNECTOR_DisplayPort, }, {},@@ -203,6 +212,7 @@ static struct auxiliary_driver drm_aux_hpd_bridge_drv = { .name = "aux_hpd_bridge", .id_table = drm_aux_hpd_bridge_table, .probe = drm_aux_hpd_bridge_probe, + .remove = drm_aux_hpd_bridge_remove, }; module_auxiliary_driver(drm_aux_hpd_bridge_drv);
Yes, if we don't distinguish them through Kconfig, we need to use the IS_ENABLED macro in the code. Thanks again for you code. Another thing is that CONFIG_DRM_AUX_HPD_BRIDGE originally needed to be selected by other modules. With this change, we also need to expose it in Kconfig.
-- Best, Chaoyi