Re: [PATCH v5 08/21] drm/bridge: inno-hdmi: Split probe out of bind

["Heiko Stübner" <heiko@sntech.de>]

Am Dienstag, 29. September 2026, 12:31:05 Mitteleuropäische Sommerzeit schrieb Michal Wilczynski:
> inno_hdmi_bind() both sets up the bridge and attaches it to a DRM
> encoder. A platform whose HDMI controller is a child of a larger device
> needs the first half without the second, since it registers as its own
> platform driver and lets the DRM core bind the bridge later.
> 
> Move the setup into a new exported inno_hdmi_probe(), and reduce
> inno_hdmi_bind() to a wrapper around it.
> 
> No functional change intended.
> 
> Reviewed-by: Joshua Peisach <jpeisach@ubuntu.com>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  drivers/gpu/drm/bridge/inno-hdmi.c | 49 +++++++++++++++++++++++++++++++++-----
>  include/drm/bridge/inno_hdmi.h     |  4 ++++
>  2 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/inno-hdmi.c b/drivers/gpu/drm/bridge/inno-hdmi.c
> index d7c33b92e2ddf4ced84d17304463f44be52270b8..7a9f54dce2647f0320d77629a28154eade14385f 100644
> --- a/drivers/gpu/drm/bridge/inno-hdmi.c
> +++ b/drivers/gpu/drm/bridge/inno-hdmi.c
> @@ -929,7 +929,14 @@ static irqreturn_t inno_hdmi_irq(int irq, void *dev_id)
>  {
>  	struct inno_hdmi *hdmi = dev_id;
>  
> -	drm_helper_hpd_irq_event(hdmi->bridge.dev);
> +	/*
> +	 * The interrupt is requested in probe, but bridge.dev is only set once
> +	 * the DRM master binds and attaches the bridge, which may never happen.
> +	 * Drop hotplug events that arrive before then rather than dereference a
> +	 * NULL drm_device.
> +	 */
> +	if (hdmi->bridge.dev)
> +		drm_helper_hpd_irq_event(hdmi->bridge.dev);

This is not part of the commit description and while true, should likely
be its own patch with its own description.


>  
>  	return IRQ_HANDLED;
>  }
> @@ -1061,11 +1068,24 @@ static struct i2c_adapter *inno_hdmi_i2c_adapter(struct inno_hdmi *hdmi)
>  	return adap;
>  }
>  
> -struct inno_hdmi *inno_hdmi_bind(struct device *dev,
> -				 struct drm_encoder *encoder,
> -				 const struct inno_hdmi_plat_data *plat_data)
> +/**
> + * inno_hdmi_probe - Internal helper to perform common setup
> + * @pdev: platform device
> + * @plat_data: SoC-specific platform data
> + *
> + * This function handles all the common hardware setup: allocating the main
> + * struct, mapping registers, getting clocks, initializing the hardware,
> + * setting up the IRQ, and initializing the DDC adapter and bridge struct.
> + * It returns a pointer to the inno_hdmi struct on success, or an ERR_PTR
> + * on failure.
> + *
> + * This function is used by modern, decoupled MFD/glue drivers. It registers
> + * the bridge but does not attach it.
> + */
> +struct inno_hdmi *inno_hdmi_probe(struct platform_device *pdev,
> +				  const struct inno_hdmi_plat_data *plat_data)
>  {
> -	struct platform_device *pdev = to_platform_device(dev);
> +	struct device *dev = &pdev->dev;
>  	struct inno_hdmi *hdmi;
>  	int irq;
>  	int ret;
> @@ -1128,7 +1148,24 @@ struct inno_hdmi *inno_hdmi_bind(struct device *dev,
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> -	ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> +	return hdmi;
> +}
> +EXPORT_SYMBOL_GPL(inno_hdmi_probe);
> +
> +struct inno_hdmi *inno_hdmi_bind(struct device *dev,
> +				 struct drm_encoder *encoder,
> +				 const struct inno_hdmi_plat_data *plat_data)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct inno_hdmi *hdmi;
> +	int ret;
> +
> +	hdmi = inno_hdmi_probe(pdev, plat_data);
> +	if (IS_ERR(hdmi))
> +		return hdmi;
> +
> +	ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL,
> +				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>  	if (ret)
>  		return ERR_PTR(ret);

This clutters up the structure even more than it is right now. From "bind"
back to "probe" and "dev" back to "pdev" and calling a function called
"probe" from the bind callback.

You are right, that all the resource allocation can (and probably should)
live in the drivers probe-path, but then please call that probe function
from the actual probe path.

For example you could look at how all the Synopsys bridges do that.


Thanks
Heiko