Hi Laurent,
On 25-05-15, Laurent Pinchart wrote:
> 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.
Yes, we can drop this patch if container_of() or to_fsl_ldb() is
preferred. I just saw the driver_private field and why not making use of
it since we do that a lot, same is true for container_of :)
Regards,
Marco
> > No functional changes.
> >
> > Signed-off-by: Marco Felsch <m.fel...@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
>
