public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
@ 2026-03-29  7:51 Liu Ying
  2026-03-29 17:42 ` Marco Felsch
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Liu Ying @ 2026-03-29  7:51 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut
  Cc: Laurentiu Palcu, Marco Felsch, dri-devel, devicetree,
	linux-kernel, Liu Ying

LDB's parent device could be a syscon which doesn't allow a reg property
to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
without a reg property(LDB is also a child device of the Media blk-ctrl).
To make the LDB schema be able to describe LDBs without the reg property
like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.

Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 .../bindings/display/bridge/fsl,ldb.yaml           | 23 ++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
index 7f380879fffd..5f6dc2b11d7b 100644
--- a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
@@ -28,6 +28,7 @@ properties:
     const: ldb
 
   reg:
+    minItems: 1
     maxItems: 2
 
   reg-names:
@@ -68,7 +69,6 @@ required:
   - compatible
   - clocks
   - ports
-  - reg
 
 allOf:
   - if:
@@ -83,12 +83,23 @@ allOf:
         ports:
           properties:
             port@2: false
+
   - if:
-      not:
-        properties:
-          compatible:
-            contains:
-              const: fsl,imx6sx-ldb
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx6sx-ldb
+              - fsl,imx8mp-ldb
+    then:
+      required:
+        - reg
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,imx8mp-ldb
     then:
       required:
         - reg-names

---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260329-fsl_ldb_schema_fix-4fe01c42bff3

Best regards,
-- 
Liu Ying <victor.liu@nxp.com>


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-29  7:51 [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs Liu Ying
@ 2026-03-29 17:42 ` Marco Felsch
  2026-03-29 18:29   ` Marek Vasut
  2026-03-30  2:05   ` Liu Ying
  2026-03-31  7:49 ` Claude review: " Claude Code Review Bot
  2026-03-31  7:49 ` Claude Code Review Bot
  2 siblings, 2 replies; 10+ messages in thread
From: Marco Felsch @ 2026-03-29 17:42 UTC (permalink / raw)
  To: Liu Ying
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Laurentiu Palcu,
	dri-devel, devicetree, linux-kernel

Hi Liu,

sorry for not writting back earlier, the last weeks were quite busy.

On 26-03-29, Liu Ying wrote:
> LDB's parent device could be a syscon which doesn't allow a reg property
> to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
> has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
> without a reg property(LDB is also a child device of the Media blk-ctrl).
> To make the LDB schema be able to describe LDBs without the reg property
> like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.

NACK, we want to describe the HW and from HW PoV the LDB is and was
always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.

> Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")

Therefore I would just revert this patch completely.

Regards,
  Marco

> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>  .../bindings/display/bridge/fsl,ldb.yaml           | 23 ++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
> index 7f380879fffd..5f6dc2b11d7b 100644
> --- a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
> @@ -28,6 +28,7 @@ properties:
>      const: ldb
>  
>    reg:
> +    minItems: 1
>      maxItems: 2
>  
>    reg-names:
> @@ -68,7 +69,6 @@ required:
>    - compatible
>    - clocks
>    - ports
> -  - reg
>  
>  allOf:
>    - if:
> @@ -83,12 +83,23 @@ allOf:
>          ports:
>            properties:
>              port@2: false
> +
>    - if:
> -      not:
> -        properties:
> -          compatible:
> -            contains:
> -              const: fsl,imx6sx-ldb
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,imx6sx-ldb
> +              - fsl,imx8mp-ldb
> +    then:
> +      required:
> +        - reg
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx8mp-ldb
>      then:
>        required:
>          - reg-names
> 
> ---
> base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
> change-id: 20260329-fsl_ldb_schema_fix-4fe01c42bff3
> 
> Best regards,
> -- 
> Liu Ying <victor.liu@nxp.com>
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-29 17:42 ` Marco Felsch
@ 2026-03-29 18:29   ` Marek Vasut
  2026-03-30  6:29     ` Marco Felsch
  2026-03-30  2:05   ` Liu Ying
  1 sibling, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2026-03-29 18:29 UTC (permalink / raw)
  To: Marco Felsch, Liu Ying
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Laurentiu Palcu,
	dri-devel, devicetree, linux-kernel

On 3/29/26 7:42 PM, Marco Felsch wrote:

Hello Marco,

> sorry for not writting back earlier, the last weeks were quite busy.

Tell me about it ...

> On 26-03-29, Liu Ying wrote:
>> LDB's parent device could be a syscon which doesn't allow a reg property
>> to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
>> has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
>> without a reg property(LDB is also a child device of the Media blk-ctrl).
>> To make the LDB schema be able to describe LDBs without the reg property
>> like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.
> 
> NACK, we want to describe the HW and from HW PoV the LDB is and was
> always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.
> 
>> Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
> 
> Therefore I would just revert this patch completely.
Last time, I pointed out the hardware is part of syscon, but as a 
subnode and therefore with reg properties. What is the problem there ?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-29 17:42 ` Marco Felsch
  2026-03-29 18:29   ` Marek Vasut
@ 2026-03-30  2:05   ` Liu Ying
  2026-03-30 10:28     ` Marco Felsch
  1 sibling, 1 reply; 10+ messages in thread
From: Liu Ying @ 2026-03-30  2:05 UTC (permalink / raw)
  To: Marco Felsch
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Laurentiu Palcu,
	dri-devel, devicetree, linux-kernel

On Sun, Mar 29, 2026 at 07:42:23PM +0200, Marco Felsch wrote:
> Hi Liu,

Hi Marco,

> 
> sorry for not writting back earlier, the last weeks were quite busy.
> 
> On 26-03-29, Liu Ying wrote:
>> LDB's parent device could be a syscon which doesn't allow a reg property
>> to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
>> has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
>> without a reg property(LDB is also a child device of the Media blk-ctrl).
>> To make the LDB schema be able to describe LDBs without the reg property
>> like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.
> 
> NACK, we want to describe the HW and from HW PoV the LDB is and was
> always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.

The reality is that i.MX6SX and i.MX8MP LDB DT nodes are already in-tree.
People may take them as ABI(not only for Linux, but also for other
potential projects which use the LDB schema and/or the DT nodes).

> 
>> Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
> 
> Therefore I would just revert this patch completely.

IMHO, it doesn't make too much difference between my patch and reverting
this offending patch, because of the ABI, i.e., the reg properties in
i.MX6SX and i.MX8MP LDB DT nodes are supposed to be stable.

I feel that what you are asking for is even more than simply reverting
this offending patch, that is to say, completely disallowing the reg and
reg-names properties for LDBs across all SoCs.  But again, that would
break the ABI.

> 
> Regards,
>   Marco
> 
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>>  .../bindings/display/bridge/fsl,ldb.yaml           | 23 ++++++++++++++++------
>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>> index 7f380879fffd..5f6dc2b11d7b 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>> @@ -28,6 +28,7 @@ properties:
>>      const: ldb
>>  
>>    reg:
>> +    minItems: 1
>>      maxItems: 2
>>  
>>    reg-names:
>> @@ -68,7 +69,6 @@ required:
>>    - compatible
>>    - clocks
>>    - ports
>> -  - reg
>>  
>>  allOf:
>>    - if:
>> @@ -83,12 +83,23 @@ allOf:
>>          ports:
>>            properties:
>>              port@2: false
>> +
>>    - if:
>> -      not:
>> -        properties:
>> -          compatible:
>> -            contains:
>> -              const: fsl,imx6sx-ldb
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - fsl,imx6sx-ldb
>> +              - fsl,imx8mp-ldb
>> +    then:
>> +      required:
>> +        - reg
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: fsl,imx8mp-ldb
>>      then:
>>        required:
>>          - reg-names
>>
>> ---
>> base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
>> change-id: 20260329-fsl_ldb_schema_fix-4fe01c42bff3
>>
>> Best regards,
>> -- 
>> Liu Ying <victor.liu@nxp.com>
>>
>>
> 

-- 
Regards,
Liu Ying

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-29 18:29   ` Marek Vasut
@ 2026-03-30  6:29     ` Marco Felsch
  2026-03-30 23:22       ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Marco Felsch @ 2026-03-30  6:29 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Liu Ying, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Marek Vasut, Laurentiu Palcu, dri-devel, devicetree, linux-kernel

Hi Marek,

On 26-03-29, Marek Vasut wrote:
> On 3/29/26 7:42 PM, Marco Felsch wrote:
> 
> Hello Marco,
> 
> > sorry for not writting back earlier, the last weeks were quite busy.
> 
> Tell me about it ...

^^

> > On 26-03-29, Liu Ying wrote:
> > > LDB's parent device could be a syscon which doesn't allow a reg property
> > > to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
> > > has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
> > > without a reg property(LDB is also a child device of the Media blk-ctrl).
> > > To make the LDB schema be able to describe LDBs without the reg property
> > > like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.
> > 
> > NACK, we want to describe the HW and from HW PoV the LDB is and was
> > always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.
> > 
> > > Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
> > 
> > Therefore I would just revert this patch completely.
> Last time, I pointed out the hardware is part of syscon, but as a subnode
> and therefore with reg properties. What is the problem there ?

To quote the DT spec here:

"""
The reg property describes the address of the device’s resources within
the address space defined by its parent bus.
"""

The parent bus is not the parent iomuxc (i.MX6X) nor the blk-ctrl
(i.MX8MP/93) device. Therefore this is wrong IMHO and should be dropped.

There will be no regression from driver POV since your patchset which
would add the support to parse the reg is not merged yet.

Regards,
  Marco

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-30  2:05   ` Liu Ying
@ 2026-03-30 10:28     ` Marco Felsch
  2026-03-31  2:01       ` Liu Ying
  0 siblings, 1 reply; 10+ messages in thread
From: Marco Felsch @ 2026-03-30 10:28 UTC (permalink / raw)
  To: Liu Ying
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Laurentiu Palcu,
	dri-devel, devicetree, linux-kernel

On 26-03-30, Liu Ying wrote:
> On Sun, Mar 29, 2026 at 07:42:23PM +0200, Marco Felsch wrote:
> > Hi Liu,
> 
> Hi Marco,
> 
> > 
> > sorry for not writting back earlier, the last weeks were quite busy.
> > 
> > On 26-03-29, Liu Ying wrote:
> >> LDB's parent device could be a syscon which doesn't allow a reg property
> >> to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
> >> has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
> >> without a reg property(LDB is also a child device of the Media blk-ctrl).
> >> To make the LDB schema be able to describe LDBs without the reg property
> >> like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.
> > 
> > NACK, we want to describe the HW and from HW PoV the LDB is and was
> > always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.
> 
> The reality is that i.MX6SX and i.MX8MP LDB DT nodes are already in-tree.

What do you mean by 'already in-tee'?

> People may take them as ABI(not only for Linux, but also for other
> potential projects which use the LDB schema and/or the DT nodes).

Not sure about this. The DT sould be backward compatible, meaning an old
DT shouldn't break new users e.g. old DT with 'reg' property present
shouldn't break new u-boot/barebox/linux/... drivers. But a new DT could
fix/change the behavior for new u-boot/barebox/linux/... drivers.

So no, I don't see a problem here.

> >> Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
> > 
> > Therefore I would just revert this patch completely.
> 
> IMHO, it doesn't make too much difference between my patch and reverting
> this offending patch, because of the ABI, i.e., the reg properties in
> i.MX6SX and i.MX8MP LDB DT nodes are supposed to be stable.

Please see above. If that would be the case, your DT must be rock-solid
bug-free from day one, which is highly unlikely.

> I feel that what you are asking for is even more than simply reverting
> this offending patch, that is to say, completely disallowing the reg and
> reg-names properties for LDBs across all SoCs.  But again, that would
> break the ABI.

Please see above. IMHO it's more confusing if the same "IP" requires the
'reg' for i.MX6SX/8MP but doesn't require it for the i.MX93. Therefore I
would like to keep it consistent.

Regards,
  Marco

> 
> > 
> > Regards,
> >   Marco
> > 
> >> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> >> ---
> >>  .../bindings/display/bridge/fsl,ldb.yaml           | 23 ++++++++++++++++------
> >>  1 file changed, 17 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
> >> index 7f380879fffd..5f6dc2b11d7b 100644
> >> --- a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
> >> +++ b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
> >> @@ -28,6 +28,7 @@ properties:
> >>      const: ldb
> >>  
> >>    reg:
> >> +    minItems: 1
> >>      maxItems: 2
> >>  
> >>    reg-names:
> >> @@ -68,7 +69,6 @@ required:
> >>    - compatible
> >>    - clocks
> >>    - ports
> >> -  - reg
> >>  
> >>  allOf:
> >>    - if:
> >> @@ -83,12 +83,23 @@ allOf:
> >>          ports:
> >>            properties:
> >>              port@2: false
> >> +
> >>    - if:
> >> -      not:
> >> -        properties:
> >> -          compatible:
> >> -            contains:
> >> -              const: fsl,imx6sx-ldb
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            enum:
> >> +              - fsl,imx6sx-ldb
> >> +              - fsl,imx8mp-ldb
> >> +    then:
> >> +      required:
> >> +        - reg
> >> +
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            const: fsl,imx8mp-ldb
> >>      then:
> >>        required:
> >>          - reg-names
> >>
> >> ---
> >> base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
> >> change-id: 20260329-fsl_ldb_schema_fix-4fe01c42bff3
> >>
> >> Best regards,
> >> -- 
> >> Liu Ying <victor.liu@nxp.com>
> >>
> >>
> > 
> 
> -- 
> Regards,
> Liu Ying
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-30  6:29     ` Marco Felsch
@ 2026-03-30 23:22       ` Marek Vasut
  0 siblings, 0 replies; 10+ messages in thread
From: Marek Vasut @ 2026-03-30 23:22 UTC (permalink / raw)
  To: Marco Felsch
  Cc: Liu Ying, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Marek Vasut, Laurentiu Palcu, dri-devel, devicetree, linux-kernel

On 3/30/26 8:29 AM, Marco Felsch wrote:

Hello Marco,

>>> On 26-03-29, Liu Ying wrote:
>>>> LDB's parent device could be a syscon which doesn't allow a reg property
>>>> to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
>>>> has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
>>>> without a reg property(LDB is also a child device of the Media blk-ctrl).
>>>> To make the LDB schema be able to describe LDBs without the reg property
>>>> like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.
>>>
>>> NACK, we want to describe the HW and from HW PoV the LDB is and was
>>> always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.
>>>
>>>> Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
>>>
>>> Therefore I would just revert this patch completely.
>> Last time, I pointed out the hardware is part of syscon, but as a subnode
>> and therefore with reg properties. What is the problem there ?
> 
> To quote the DT spec here:
> 
> """
> The reg property describes the address of the device’s resources within
> the address space defined by its parent bus.
> """

That parent bus would be the syscon, wouldn't it.

> The parent bus is not the parent iomuxc (i.MX6X) nor the blk-ctrl
> (i.MX8MP/93) device. Therefore this is wrong IMHO and should be dropped.

How so ? What is the parent bus ?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-30 10:28     ` Marco Felsch
@ 2026-03-31  2:01       ` Liu Ying
  0 siblings, 0 replies; 10+ messages in thread
From: Liu Ying @ 2026-03-31  2:01 UTC (permalink / raw)
  To: Marco Felsch
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Laurentiu Palcu,
	dri-devel, devicetree, linux-kernel

Hi Marco,

On Mon, Mar 30, 2026 at 12:28:00PM +0200, Marco Felsch wrote:
> On 26-03-30, Liu Ying wrote:
>> On Sun, Mar 29, 2026 at 07:42:23PM +0200, Marco Felsch wrote:
>>> Hi Liu,
>>
>> Hi Marco,
>>
>>>
>>> sorry for not writting back earlier, the last weeks were quite busy.
>>>
>>> On 26-03-29, Liu Ying wrote:
>>>> LDB's parent device could be a syscon which doesn't allow a reg property
>>>> to be present in it's child devices, e.g., NXP i.MX93 Media blk-ctrl
>>>> has a child device NXP i.MX93 Parallel Display Format Configuration(PDFC)
>>>> without a reg property(LDB is also a child device of the Media blk-ctrl).
>>>> To make the LDB schema be able to describe LDBs without the reg property
>>>> like i.MX93 LDB, require the reg property only for i.MX6SX/8MP LDBs.
>>>
>>> NACK, we want to describe the HW and from HW PoV the LDB is and was
>>> always part of a syscon. This is the case for all SoCs i.MX6SX/8MP/93.
>>
>> The reality is that i.MX6SX and i.MX8MP LDB DT nodes are already in-tree.
> 
> What do you mean by 'already in-tee'?

I mean the LDB DT nodes are already present in device trees.

You may find the LDB DT nodes with the below command line.
git grep -A5 lvds_bridge arch/ | grep -E '6sx|8mp'

> 
>> People may take them as ABI(not only for Linux, but also for other
>> potential projects which use the LDB schema and/or the DT nodes).
> 
> Not sure about this. The DT sould be backward compatible, meaning an old
> DT shouldn't break new users e.g. old DT with 'reg' property present
> shouldn't break new u-boot/barebox/linux/... drivers. But a new DT could
> fix/change the behavior for new u-boot/barebox/linux/... drivers.
> 
> So no, I don't see a problem here.

An OS could try to get the LDB register(s) via the reg and reg-names
property which are currently present in i.MX6SX and i.MX8MP LDB DT nodes.
If we remove the properties from the DT nodes, then that OS would be broken.
That's the ABI breakage problem I talked about.

> 
>>>> Fixes: 8aa2f0ac08d3 ("dt-bindings: display: bridge: ldb: Add check for reg and reg-names")
>>>
>>> Therefore I would just revert this patch completely.
>>
>> IMHO, it doesn't make too much difference between my patch and reverting
>> this offending patch, because of the ABI, i.e., the reg properties in
>> i.MX6SX and i.MX8MP LDB DT nodes are supposed to be stable.
> 
> Please see above. If that would be the case, your DT must be rock-solid
> bug-free from day one, which is highly unlikely.
> 
>> I feel that what you are asking for is even more than simply reverting
>> this offending patch, that is to say, completely disallowing the reg and
>> reg-names properties for LDBs across all SoCs.  But again, that would
>> break the ABI.
> 
> Please see above. IMHO it's more confusing if the same "IP" requires the
> 'reg' for i.MX6SX/8MP but doesn't require it for the i.MX93. Therefore I
> would like to keep it consistent.

I agree that it's ideal to keep it consistent, however, in order to avoid
the fore-mentioned ABI breakage problem, we have to keep the reg property
being present in i.MX6SX and i.MX8MP LDB DT nodes.

> 
> Regards,
>   Marco
> 
>>
>>>
>>> Regards,
>>>   Marco
>>>
>>>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>>>> ---
>>>>  .../bindings/display/bridge/fsl,ldb.yaml           | 23 ++++++++++++++++------
>>>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>>>> index 7f380879fffd..5f6dc2b11d7b 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
>>>> @@ -28,6 +28,7 @@ properties:
>>>>      const: ldb
>>>>  
>>>>    reg:
>>>> +    minItems: 1
>>>>      maxItems: 2
>>>>  
>>>>    reg-names:
>>>> @@ -68,7 +69,6 @@ required:
>>>>    - compatible
>>>>    - clocks
>>>>    - ports
>>>> -  - reg
>>>>  
>>>>  allOf:
>>>>    - if:
>>>> @@ -83,12 +83,23 @@ allOf:
>>>>          ports:
>>>>            properties:
>>>>              port@2: false
>>>> +
>>>>    - if:
>>>> -      not:
>>>> -        properties:
>>>> -          compatible:
>>>> -            contains:
>>>> -              const: fsl,imx6sx-ldb
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - fsl,imx6sx-ldb
>>>> +              - fsl,imx8mp-ldb
>>>> +    then:
>>>> +      required:
>>>> +        - reg
>>>> +
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: fsl,imx8mp-ldb
>>>>      then:
>>>>        required:
>>>>          - reg-names
>>>>
>>>> ---
>>>> base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
>>>> change-id: 20260329-fsl_ldb_schema_fix-4fe01c42bff3
>>>>
>>>> Best regards,
>>>> -- 
>>>> Liu Ying <victor.liu@nxp.com>
>>>>
>>>>
>>>
>>
>> -- 
>> Regards,
>> Liu Ying
>>
> 

-- 
Regards,
Liu Ying

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Claude review: dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-29  7:51 [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs Liu Ying
  2026-03-29 17:42 ` Marco Felsch
@ 2026-03-31  7:49 ` Claude Code Review Bot
  2026-03-31  7:49 ` Claude Code Review Bot
  2 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-03-31  7:49 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
Author: Liu Ying <victor.liu@nxp.com>
Patches: 8
Reviewed: 2026-03-31T17:49:51.247576

---

This is a single-patch fix to the `fsl,ldb.yaml` dt-bindings schema. The problem is real: the i.MX93 LDB is a child of a syscon (Media blk-ctrl) that doesn't allow `reg` properties in its children, but the schema currently requires `reg` globally for all LDB variants. The fix correctly makes `reg` (and `reg-names`) conditional on the specific compatible strings that need them.

The patch is logically sound and the approach of using per-compatible `if/then` blocks is the standard dt-bindings pattern for this kind of differentiation. **No significant issues found.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Claude review: dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
  2026-03-29  7:51 [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs Liu Ying
  2026-03-29 17:42 ` Marco Felsch
  2026-03-31  7:49 ` Claude review: " Claude Code Review Bot
@ 2026-03-31  7:49 ` Claude Code Review Bot
  2 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-03-31  7:49 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Changes summary:**
1. Adds `minItems: 1` to the `reg` property definition
2. Removes `reg` from the global `required` list
3. Adds a new `if/then` block requiring `reg` only for `fsl,imx6sx-ldb` and `fsl,imx8mp-ldb`
4. Restructures the `reg-names` conditional from a negated check ("not imx6sx") to a positive match ("only imx8mp")

**Analysis:**

The logic changes are correct:

- **Before:** `reg` required for all three variants (imx6sx, imx8mp, imx93); `reg-names` required for imx8mp and imx93.
- **After:** `reg` required only for imx6sx and imx8mp; `reg-names` required only for imx8mp.

This is consistent — imx93 LDB needs neither `reg` nor `reg-names` since it lives under a syscon parent.

**Minor nits:**

1. `minItems: 1` on `reg` is technically redundant (1 is the implicit minimum for array properties in dt-schema), but it's harmless and clarifies intent alongside `maxItems: 2`. Not worth blocking over.

2. The commit message says "it's" (line 153 of the mbox) where it should be "its":
```
LDB's parent device could be a syscon which doesn't allow a reg property
to be present in it's child devices
```
Should be `its child devices`.

3. The restructuring of the `reg-names` conditional is a nice cleanup. The old form:
```yaml
  - if:
      not:
        properties:
          compatible:
            contains:
              const: fsl,imx6sx-ldb
    then:
      required:
        - reg-names
```
used a negation that implicitly required `reg-names` for any future compatible added to the schema. The new positive match:
```yaml
  - if:
      properties:
        compatible:
          contains:
            const: fsl,imx8mp-ldb
    then:
      required:
        - reg-names
```
is more explicit and safer for future compatible additions, which is the right direction.

**Verdict:** Looks good. The Fixes tag is appropriate. Only the "it's → its" typo in the commit message is worth mentioning to the author.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-03-31  7:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-29  7:51 [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs Liu Ying
2026-03-29 17:42 ` Marco Felsch
2026-03-29 18:29   ` Marek Vasut
2026-03-30  6:29     ` Marco Felsch
2026-03-30 23:22       ` Marek Vasut
2026-03-30  2:05   ` Liu Ying
2026-03-30 10:28     ` Marco Felsch
2026-03-31  2:01       ` Liu Ying
2026-03-31  7:49 ` Claude review: " Claude Code Review Bot
2026-03-31  7:49 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox