Re: [PATCH v9 06/10] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge

[Neil Armstrong <neil.armstrong@linaro.org>]

On 11/11/25 11:50, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge
> device.
> 
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> ---
> 
> (no changes since v7)
> 
> Changes in v6:
> - Fix depend in Kconfig.
> 
>   drivers/phy/rockchip/Kconfig              |  2 +
>   drivers/phy/rockchip/phy-rockchip-typec.c | 52 +++++++++++++++++++++++
>   2 files changed, 54 insertions(+)
> 
> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> index db4adc7c53da..bcb5476222fc 100644
> --- a/drivers/phy/rockchip/Kconfig
> +++ b/drivers/phy/rockchip/Kconfig
> @@ -120,6 +120,8 @@ config PHY_ROCKCHIP_TYPEC
>   	tristate "Rockchip TYPEC PHY Driver"
>   	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
>   	depends on TYPEC || TYPEC=n
> +	depends on DRM || DRM=n
> +	select DRM_AUX_BRIDGE if DRM_BRIDGE
>   	select EXTCON
>   	select GENERIC_PHY
>   	select RESET_CONTROLLER
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index 1f5b4142cbe4..748a6eb8ad95 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -36,6 +36,7 @@
>    * orientation, false is normal orientation.
>    */
>   
> +#include <linux/auxiliary_bus.h>
>   #include <linux/clk.h>
>   #include <linux/clk-provider.h>
>   #include <linux/delay.h>
> @@ -56,6 +57,7 @@
>   #include <linux/phy/phy.h>
>   #include <linux/usb/typec_dp.h>
>   #include <linux/usb/typec_mux.h>
> +#include <drm/bridge/aux-bridge.h>
>   
>   #define CMN_SSM_BANDGAP			(0x21 << 2)
>   #define CMN_SSM_BIAS			(0x22 << 2)
> @@ -415,6 +417,7 @@ struct rockchip_usb3phy_port_cfg {
>   
>   struct rockchip_typec_phy {
>   	struct device *dev;
> +	struct auxiliary_device dp_port_dev;
>   	void __iomem *base;
>   	struct extcon_dev *extcon;
>   	struct typec_mux_dev *mux;
> @@ -1299,6 +1302,51 @@ static void tcphy_typec_mux_unregister(void *data)
>   	typec_mux_unregister(tcphy->mux);
>   }
>   
> +static void tcphy_dp_port_dev_release(struct device *dev)
> +{
> +	struct auxiliary_device *adev = to_auxiliary_dev(dev);
> +
> +	of_node_put(adev->dev.of_node);
> +}
> +
> +static void tcphy_dp_port_unregister_adev(void *_adev)
> +{
> +	struct auxiliary_device *adev = _adev;
> +
> +	auxiliary_device_delete(adev);
> +	auxiliary_device_uninit(adev);
> +}
> +
> +static int tcphy_aux_bridge_register(struct rockchip_typec_phy *tcphy, struct device_node *np)
> +{
> +	struct auxiliary_device *adev = &tcphy->dp_port_dev;
> +	int ret;
> +
> +	adev->name = "dp_port";
> +	adev->dev.parent = tcphy->dev;
> +	adev->dev.of_node = of_node_get(np);
> +	adev->dev.release = tcphy_dp_port_dev_release;
> +
> +	ret = auxiliary_device_init(adev);
> +

Drop this empty line.

> +	if (ret) {
> +		of_node_put(adev->dev.of_node);
> +		return ret;
> +	}
> +
> +	ret = auxiliary_device_add(adev);
> +	if (ret) {
> +		auxiliary_device_uninit(adev);
> +		return ret;
> +	}
> +
> +	devm_add_action_or_reset(tcphy->dev, tcphy_dp_port_unregister_adev, adev);
> +
> +	ret = drm_aux_bridge_register(&adev->dev);

Adding an aux device to an aux device looks quite overengineered to me !

If it's a matter of using the proper of_node, you may instead create a separate
drm_aux_bridge_register() like drm_aux_bridge_register_from_node() instead.

Neil

> +
> +	return 0;
> +}
> +
>   static int tcphy_setup_typec_mux(struct rockchip_typec_phy *tcphy)
>   {
>   	struct typec_mux_desc mux_desc = {};
> @@ -1312,6 +1360,10 @@ static int tcphy_setup_typec_mux(struct rockchip_typec_phy *tcphy)
>   	if (!of_property_read_bool(np, "mode-switch"))
>   		goto put_np;
>   
> +	ret = tcphy_aux_bridge_register(tcphy, np);
> +	if (ret)
> +		goto put_np;
> +
>   	mux_desc.drvdata = tcphy;
>   	mux_desc.fwnode = device_get_named_child_node(tcphy->dev, "dp-port");
>   	mux_desc.set = tcphy_typec_mux_set;