Re: [PATCH 1/3] drm/bridge: fsl-ldb: make use of driver_private

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Marco,

Thank you for the patch.

On Thu, May 15, 2025 at 12:24:51AM +0200, Marco Felsch wrote:
> Make use of the drm_bridge::driver_private data instead of
> container_of() wrapper.

I suppose this is a personal preference, but I like usage of
container_of() better. In my opinion it conveys better that struct
fsl_ldb "unherits" from struct drm_bridge.

> No functional changes.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/gpu/drm/bridge/fsl-ldb.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index 0fc8a14fd800..fa29f2bf4031 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -99,11 +99,6 @@ static bool fsl_ldb_is_dual(const struct fsl_ldb *fsl_ldb)
>  	return (fsl_ldb->ch0_enabled && fsl_ldb->ch1_enabled);
>  }
>  
> -static inline struct fsl_ldb *to_fsl_ldb(struct drm_bridge *bridge)
> -{
> -	return container_of(bridge, struct fsl_ldb, bridge);
> -}
> -
>  static unsigned long fsl_ldb_link_frequency(struct fsl_ldb *fsl_ldb, int clock)
>  {
>  	if (fsl_ldb_is_dual(fsl_ldb))
> @@ -115,7 +110,7 @@ static unsigned long fsl_ldb_link_frequency(struct fsl_ldb *fsl_ldb, int clock)
>  static int fsl_ldb_attach(struct drm_bridge *bridge,
>  			  enum drm_bridge_attach_flags flags)
>  {
> -	struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge);
> +	struct fsl_ldb *fsl_ldb = bridge->driver_private;
>  
>  	return drm_bridge_attach(bridge->encoder, fsl_ldb->panel_bridge,
>  				 bridge, flags);
> @@ -124,7 +119,7 @@ static int fsl_ldb_attach(struct drm_bridge *bridge,
>  static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
>  				  struct drm_bridge_state *old_bridge_state)
>  {
> -	struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge);
> +	struct fsl_ldb *fsl_ldb = bridge->driver_private;
>  	struct drm_atomic_state *state = old_bridge_state->base.state;
>  	const struct drm_bridge_state *bridge_state;
>  	const struct drm_crtc_state *crtc_state;
> @@ -226,7 +221,7 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
>  static void fsl_ldb_atomic_disable(struct drm_bridge *bridge,
>  				   struct drm_bridge_state *old_bridge_state)
>  {
> -	struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge);
> +	struct fsl_ldb *fsl_ldb = bridge->driver_private;
>  
>  	/* Stop channel(s). */
>  	if (fsl_ldb->devdata->lvds_en_bit)
> @@ -270,7 +265,7 @@ fsl_ldb_mode_valid(struct drm_bridge *bridge,
>  		   const struct drm_display_info *info,
>  		   const struct drm_display_mode *mode)
>  {
> -	struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge);
> +	struct fsl_ldb *fsl_ldb = bridge->driver_private;
>  
>  	if (mode->clock > (fsl_ldb_is_dual(fsl_ldb) ? 160000 : 80000))
>  		return MODE_CLOCK_HIGH;
> @@ -309,6 +304,7 @@ static int fsl_ldb_probe(struct platform_device *pdev)
>  	fsl_ldb->dev = &pdev->dev;
>  	fsl_ldb->bridge.funcs = &funcs;
>  	fsl_ldb->bridge.of_node = dev->of_node;
> +	fsl_ldb->bridge.driver_private = fsl_ldb;
>  
>  	fsl_ldb->clk = devm_clk_get(dev, "ldb");
>  	if (IS_ERR(fsl_ldb->clk))

-- 
Regards,

Laurent Pinchart