On 10/17/2025, Marek Vasut wrote: > The DT binding for this bridge describe register offsets for the LDB,
s/describe/describes/ > parse the register offsets from DT instead of hard-coding them in the > driver. No functional change. > > Signed-off-by: Marek Vasut <[email protected]> > --- > Cc: Abel Vesa <[email protected]> > Cc: Conor Dooley <[email protected]> > Cc: Fabio Estevam <[email protected]> > Cc: Krzysztof Kozlowski <[email protected]> > Cc: Laurent Pinchart <[email protected]> > Cc: Liu Ying <[email protected]> > Cc: Lucas Stach <[email protected]> > Cc: Peng Fan <[email protected]> > Cc: Pengutronix Kernel Team <[email protected]> > Cc: Rob Herring <[email protected]> > Cc: Shawn Guo <[email protected]> > Cc: Thomas Zimmermann <[email protected]> > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > --- > drivers/gpu/drm/bridge/fsl-ldb.c | 42 ++++++++++++++++++++------------ > 1 file changed, 26 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c > b/drivers/gpu/drm/bridge/fsl-ldb.c > index 5c3cf37200bce..c54caea0b63fc 100644 > --- a/drivers/gpu/drm/bridge/fsl-ldb.c > +++ b/drivers/gpu/drm/bridge/fsl-ldb.c > @@ -61,24 +61,16 @@ enum fsl_ldb_devtype { > }; > > struct fsl_ldb_devdata { > - u32 ldb_ctrl; > - u32 lvds_ctrl; > bool lvds_en_bit; > bool single_ctrl_reg; single_ctrl_reg can be dropped then, as it can be expressed by failing to get the second register. Furthermore, with this done, lvds_en_bit is the only member left and hence struct fsl_ldb_devdata can also be dropped, as IIRC there is no need to use a structure for device data with only a flag. > }; > > static const struct fsl_ldb_devdata fsl_ldb_devdata[] = { > [IMX6SX_LDB] = { > - .ldb_ctrl = 0x18, > .single_ctrl_reg = true, > }, > - [IMX8MP_LDB] = { > - .ldb_ctrl = 0x5c, > - .lvds_ctrl = 0x128, > - }, > + [IMX8MP_LDB] = { }, > [IMX93_LDB] = { > - .ldb_ctrl = 0x20, > - .lvds_ctrl = 0x24, > .lvds_en_bit = true, > }, > }; > @@ -90,6 +82,8 @@ struct fsl_ldb { > struct clk *clk; > struct regmap *regmap; > const struct fsl_ldb_devdata *devdata; > + u32 ldb_ctrl; > + u32 lvds_ctrl; > bool ch0_enabled; > bool ch1_enabled; > }; > @@ -204,7 +198,7 @@ static void fsl_ldb_atomic_enable(struct drm_bridge > *bridge, > reg |= (fsl_ldb->ch0_enabled ? LDB_CTRL_DI0_VSYNC_POLARITY : > 0) | > (fsl_ldb->ch1_enabled ? LDB_CTRL_DI1_VSYNC_POLARITY : > 0); > > - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, reg); > + regmap_write(fsl_ldb->regmap, fsl_ldb->ldb_ctrl, reg); > > if (fsl_ldb->devdata->single_ctrl_reg) > return; > @@ -212,7 +206,7 @@ static void fsl_ldb_atomic_enable(struct drm_bridge > *bridge, > /* Program LVDS_CTRL */ > reg = LVDS_CTRL_CC_ADJ(2) | LVDS_CTRL_PRE_EMPH_EN | > LVDS_CTRL_PRE_EMPH_ADJ(3) | LVDS_CTRL_VBG_EN; > - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg); > + regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, reg); > > /* Wait for VBG to stabilize. */ > usleep_range(15, 20); > @@ -220,7 +214,7 @@ static void fsl_ldb_atomic_enable(struct drm_bridge > *bridge, > reg |= (fsl_ldb->ch0_enabled ? LVDS_CTRL_CH0_EN : 0) | > (fsl_ldb->ch1_enabled ? LVDS_CTRL_CH1_EN : 0); > > - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg); > + regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, reg); > } > > static void fsl_ldb_atomic_disable(struct drm_bridge *bridge, > @@ -231,12 +225,12 @@ static void fsl_ldb_atomic_disable(struct drm_bridge > *bridge, > /* Stop channel(s). */ > if (fsl_ldb->devdata->lvds_en_bit) > /* Set LVDS_CTRL_LVDS_EN bit to disable. */ > - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, > + regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, > LVDS_CTRL_LVDS_EN); > else > if (!fsl_ldb->devdata->single_ctrl_reg) > - regmap_write(fsl_ldb->regmap, > fsl_ldb->devdata->lvds_ctrl, 0); > - regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, 0); > + regmap_write(fsl_ldb->regmap, fsl_ldb->lvds_ctrl, 0); > + regmap_write(fsl_ldb->regmap, fsl_ldb->ldb_ctrl, 0); > > clk_disable_unprepare(fsl_ldb->clk); > } > @@ -296,7 +290,7 @@ static int fsl_ldb_probe(struct platform_device *pdev) > struct device_node *remote1, *remote2; > struct drm_panel *panel; > struct fsl_ldb *fsl_ldb; > - int dual_link; > + int dual_link, idx, ret; > > fsl_ldb = devm_drm_bridge_alloc(dev, struct fsl_ldb, bridge, &funcs); > if (IS_ERR(fsl_ldb)) > @@ -309,6 +303,22 @@ static int fsl_ldb_probe(struct platform_device *pdev) > fsl_ldb->dev = &pdev->dev; > fsl_ldb->bridge.of_node = dev->of_node; > > + idx = of_property_match_string(dev->of_node, "reg-names", "ldb"); > + if (idx < 0) > + return idx; > + > + ret = of_property_read_u32_index(dev->of_node, "reg", 2 * idx, > &fsl_ldb->ldb_ctrl); > + if (ret) > + return ret; > + > + idx = of_property_match_string(dev->of_node, "reg-names", "lvds"); > + if (idx < 0) > + return idx; Hey, i.MX6SX LDB's single_ctrl_reg is true. This would break i.MX6SX since this returns with error code? > + > + ret = of_property_read_u32_index(dev->of_node, "reg", 2 * idx, > &fsl_ldb->lvds_ctrl); > + if (ret) > + return ret; I'm not sure if these of_property_xxx function calls are correct or not, but they look pretty heavy. Can they be replaced with of_property_read_reg()? > + > fsl_ldb->clk = devm_clk_get(dev, "ldb"); > if (IS_ERR(fsl_ldb->clk)) > return PTR_ERR(fsl_ldb->clk); -- Regards, Liu Ying
