Re: [PATCH v2] drm/bridge: fsl-ldb: Parse register offsets from DT

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

Hello Marek,

+Laurentiu Palcu

On Sun Nov 2, 2025 at 6:02 PM CET, Marek Vasut wrote:
> The DT binding for this bridge describe register offsets for the LDB,
> parse the register offsets from DT instead of hard-coding them in the
> driver. No functional change.
>
> Signed-off-by: Marek Vasut <marek.vasut@mailbox.org>

I was initially a bit skeptical because normally register offsets are
derived from the compatible string, not from device tree. But then I
realized this is about the LDB which has its two registers in the
MEDIA_BLK. This means all in all this looks somewhat like an integration
aspect (the LDB component uses two resources of the MEDIA_CLK component)
and your patch mekse sense.

So my only remark is that the above may be in the commit message, to make
the "why" clear from the beginning. It took a bit of research for me to
find out.

Laurentiu's patch adding i.MX94 support will conflict with this
one. Whichever gets applied after the other will have to be adapted
accordingly.

[0] https://lore.kernel.org/dri-devel/20251103-dcif-upstreaming-v6-3-76fcecfda919@oss.nxp.com/

> @@ -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 */

Uh? If it is "likely" it means we are not sure this code is not introducing
regressions, and that would be bad.

> +	idx = of_property_match_string(dev->of_node, "reg-names", "ldb");
> +	if (idx < 0) {
> +		fsl_ldb->single_ctrl_reg = true;
> +		idx = 0;
> +	}

From the bindings I understand that having two 'reg' values and no
'reg-names' at all is legal. Your patch implies differently. Who's right
here?

> +	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;
> +	}
> +

Best regards,
Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com