On 11/3/25 01:02, Marek Vasut wrote: > The DT binding for this bridge describe register offsets for the LDB,
I'm repeating my comment on v1: 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] > --- > V2: - Switch to of_property_read_reg() > - Parse single-register LDB variants from DT too > --- > drivers/gpu/drm/bridge/fsl-ldb.c | 58 ++++++++++++++++++++------------ > 1 file changed, 36 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c > b/drivers/gpu/drm/bridge/fsl-ldb.c > index 5c3cf37200bce..2357cb2fbbe39 100644 > --- a/drivers/gpu/drm/bridge/fsl-ldb.c > +++ b/drivers/gpu/drm/bridge/fsl-ldb.c > @@ -8,6 +8,7 @@ > #include <linux/mfd/syscon.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_address.h> > #include <linux/of_graph.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > @@ -61,24 +62,13 @@ enum fsl_ldb_devtype { > }; > > struct fsl_ldb_devdata { > - u32 ldb_ctrl; > - u32 lvds_ctrl; > bool lvds_en_bit; > - bool single_ctrl_reg; > }; > > static const struct fsl_ldb_devdata fsl_ldb_devdata[] = { As I pointed out in v1 comment, this patch should remove struct fsl_ldb_devdata. > - [IMX6SX_LDB] = { > - .ldb_ctrl = 0x18, > - .single_ctrl_reg = true, > - }, > - [IMX8MP_LDB] = { > - .ldb_ctrl = 0x5c, > - .lvds_ctrl = 0x128, > - }, > + [IMX6SX_LDB] = { }, > + [IMX8MP_LDB] = { }, > [IMX93_LDB] = { > - .ldb_ctrl = 0x20, > - .lvds_ctrl = 0x24, > .lvds_en_bit = true, > }, > }; > @@ -90,8 +80,11 @@ struct fsl_ldb { > struct clk *clk; > struct regmap *regmap; > const struct fsl_ldb_devdata *devdata; > + u64 ldb_ctrl; > + u64 lvds_ctrl; > bool ch0_enabled; > bool ch1_enabled; > + bool single_ctrl_reg; > }; > > static bool fsl_ldb_is_dual(const struct fsl_ldb *fsl_ldb) > @@ -204,15 +197,15 @@ 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) > + if (fsl_ldb->single_ctrl_reg) > return; > > /* 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 +213,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 +224,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); > + if (!fsl_ldb->single_ctrl_reg) > + 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 +289,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 +302,27 @@ static int fsl_ldb_probe(struct platform_device *pdev) > fsl_ldb->dev = &pdev->dev; > fsl_ldb->bridge.of_node = dev->of_node; > > + /* No "reg-names" property likely means single-register LDB */ > + idx = of_property_match_string(dev->of_node, "reg-names", "ldb"); You don't need to match reg-names. Instead, just call of_property_read_reg() twice to get the first reg and the second reg by passing indexes 0 and 1 to it. If the second reg is not found, then set fsl_ldb->single_ctrl_reg to true. It would be good to take this chance to clean up reg and reg-names properties in fsl,ldb.yaml. See this complaint: DTC [C] arch/arm/boot/dts/nxp/imx/imx6sx-sdb.dtb arch/arm/boot/dts/nxp/imx/imx6sx-sdb.dtb: bridge@18 (fsl,imx6sx-ldb): reg: [[24, 4]] is too short from schema $id: http://devicetree.org/schemas/display/bridge/fsl,ldb.yaml# > + if (idx < 0) { > + fsl_ldb->single_ctrl_reg = true; > + idx = 0; > + } > + > + ret = of_property_read_reg(dev->of_node, idx, &fsl_ldb->ldb_ctrl, NULL); > + if (ret) > + return ret; > + > + if (!fsl_ldb->single_ctrl_reg) { > + idx = of_property_match_string(dev->of_node, "reg-names", > "lvds"); > + if (idx < 0) > + return idx; > + > + ret = of_property_read_reg(dev->of_node, idx, > &fsl_ldb->lvds_ctrl, NULL); > + if (ret) > + return ret; > + } > + > fsl_ldb->clk = devm_clk_get(dev, "ldb"); > if (IS_ERR(fsl_ldb->clk)) > return PTR_ERR(fsl_ldb->clk); -- Regards, Liu Ying
