On 11/5/25 7:03 PM, Luca Ceresoli wrote:
Hello Luca,
sorry for the late reply.
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 <[email protected]>
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.
Actually, the LDB was always meant to parse the 'reg' offsets out of the
DT, it then went somewhat ... wrong ... and we ended up with hard-coded
reg<->compatible mapping. It was never intended to be that way. That is
all there is to it, there isn't any deeper reason behind it.
What would you add into the commit message ?
The above paragraph is a good draft of what I woudl add.
[0]
https://lore.kernel.org/dri-devel/[email protected]/
@@ -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.
I can drop the 'likely' part.
If you are sure it's not "likely" but "sure", then OK. However it all
depends on the bindings, which leads to the below question.
Fixed in V3
+ 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?
I think if you have two two reg values, you should have reg-names , so
the binding should be updated ?
If the bindings are unclear or ambiguous (or wrong) then they should be
fixed in the first place. With bad bindings we can either have a bad but
compliant implementation or a good but non-compliant implementation.
Binding update sent:
https://lore.kernel.org/dri-devel/[email protected]/
