* [PATCH v1 1/3] dt-bindings: display: rockchip: analogix-dp: Expose inherited properties
2026-05-14 7:01 [PATCH v1 0/3] Add eDP lane mapping support Damon Ding
@ 2026-05-14 7:01 ` Damon Ding
2026-05-14 18:16 ` Conor Dooley
2026-05-16 1:13 ` Claude review: " Claude Code Review Bot
2026-05-14 7:01 ` [PATCH v1 2/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Damon Ding @ 2026-05-14 7:01 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
Expose the inherited properties from the base analogix-dp schema
to satisfy unevaluatedProperties constraints.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../bindings/display/rockchip/rockchip,analogix-dp.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index bb75d898a5c5..896ded87880f 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -50,6 +50,13 @@ properties:
aux-bus:
$ref: /schemas/display/dp-aux-bus.yaml#
+ reg: true
+ interrupts: true
+ phys: true
+ phy-names: true
+ force-hpd: true
+ ports: true
+
required:
- compatible
- clocks
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v1 1/3] dt-bindings: display: rockchip: analogix-dp: Expose inherited properties
2026-05-14 7:01 ` [PATCH v1 1/3] dt-bindings: display: rockchip: analogix-dp: Expose inherited properties Damon Ding
@ 2026-05-14 18:16 ` Conor Dooley
2026-05-15 3:57 ` Damon Ding
2026-05-16 1:13 ` Claude review: " Claude Code Review Bot
1 sibling, 1 reply; 13+ messages in thread
From: Conor Dooley @ 2026-05-14 18:16 UTC (permalink / raw)
To: Damon Ding
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]
On Thu, May 14, 2026 at 03:01:31PM +0800, Damon Ding wrote:
> Expose the inherited properties from the base analogix-dp schema
> to satisfy unevaluatedProperties constraints.
>
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Given it's unevaluatedProperties, not addtionalProperties, this patch
shouldn't be needed?
> ---
> .../bindings/display/rockchip/rockchip,analogix-dp.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> index bb75d898a5c5..896ded87880f 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> @@ -50,6 +50,13 @@ properties:
> aux-bus:
> $ref: /schemas/display/dp-aux-bus.yaml#
>
> + reg: true
> + interrupts: true
> + phys: true
> + phy-names: true
> + force-hpd: true
> + ports: true
> +
> required:
> - compatible
> - clocks
> --
> 2.34.1
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/3] dt-bindings: display: rockchip: analogix-dp: Expose inherited properties
2026-05-14 18:16 ` Conor Dooley
@ 2026-05-15 3:57 ` Damon Ding
2026-05-15 9:04 ` Conor Dooley
0 siblings, 1 reply; 13+ messages in thread
From: Damon Ding @ 2026-05-15 3:57 UTC (permalink / raw)
To: Conor Dooley
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
Hi Conor,
On 5/15/2026 2:16 AM, Conor Dooley wrote:
> On Thu, May 14, 2026 at 03:01:31PM +0800, Damon Ding wrote:
>> Expose the inherited properties from the base analogix-dp schema
>> to satisfy unevaluatedProperties constraints.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>
> Given it's unevaluatedProperties, not addtionalProperties, this patch
> shouldn't be needed?
>
When I remove both the top-level data-lanes property and those explicit
"xxx: true" property entries and run the dtbs check with:
make CHECK_DTBS=y CROSS_COMPILE=aarch64-linux-gnu- LT0=none LLVM=1
LLVM_IAS=1 ARCH=arm64 rockchip/rk3588-evb1-v10.dtb
rockchip/rk3588s-evb1-v10.dtb rockchip/rk3399-sapphire-excavator.dtb
rockchip/rk3576-evb1-v10.dtb -j4
It results in validation errors like these:
/home/ding/drm-misc/arch/arm64/boot/dts/rockchip/rk3576-evb1-v10.dtb:
edp@27dc0000: ports:port@1:endpoint: Unevaluated properties are not
allowed ('data-lanes' was unexpected)
from schema $id:
http://devicetree.org/schemas/display/rockchip/rockchip,analogix-dp.yaml#
/home/ding/drm-misc/arch/arm64/boot/dts/rockchip/rk3576-evb1-v10.dtb:
edp@27dc0000: Unevaluated properties are not allowed ('force-hpd',
'interrupts', 'phy-names', 'phys', 'ports', 'reg' were unexpected)
from schema $id:
http://devicetree.org/schemas/display/rockchip/rockchip,analogix-dp.yaml#
I suspect that the properties defined in the child binding are
overriding/masking all the inherited properties from the parent Analogix
DP schema.
Is there a better way to fix this issue without explicitly listing all
inherited properties as true?
Best regards,
Damon
>> ---
>> .../bindings/display/rockchip/rockchip,analogix-dp.yaml | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> index bb75d898a5c5..896ded87880f 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> @@ -50,6 +50,13 @@ properties:
>> aux-bus:
>> $ref: /schemas/display/dp-aux-bus.yaml#
>>
>> + reg: true
>> + interrupts: true
>> + phys: true
>> + phy-names: true
>> + force-hpd: true
>> + ports: true
>> +
>> required:
>> - compatible
>> - clocks
>> --
>> 2.34.1
>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v1 1/3] dt-bindings: display: rockchip: analogix-dp: Expose inherited properties
2026-05-15 3:57 ` Damon Ding
@ 2026-05-15 9:04 ` Conor Dooley
0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-05-15 9:04 UTC (permalink / raw)
To: Damon Ding
Cc: Conor Dooley, hjc, heiko, andy.yan, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, robh, krzk+dt, conor+dt,
andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, jonas,
jernej.skrabec, nicolas.frattaroli, cristian.ciocaltea,
sebastian.reichel, dmitry.baryshkov, luca.ceresoli, dianders,
m.szyprowski, dri-devel, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3781 bytes --]
On Fri, May 15, 2026 at 11:57:58AM +0800, Damon Ding wrote:
> Hi Conor,
>
> On 5/15/2026 2:16 AM, Conor Dooley wrote:
> > On Thu, May 14, 2026 at 03:01:31PM +0800, Damon Ding wrote:
> > > Expose the inherited properties from the base analogix-dp schema
> > > to satisfy unevaluatedProperties constraints.
> > >
> > > Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> >
> > Given it's unevaluatedProperties, not addtionalProperties, this patch
> > shouldn't be needed?
> >
>
> When I remove both the top-level data-lanes property and those explicit
> "xxx: true" property entries and run the dtbs check with:
>
> make CHECK_DTBS=y CROSS_COMPILE=aarch64-linux-gnu- LT0=none LLVM=1
> LLVM_IAS=1 ARCH=arm64 rockchip/rk3588-evb1-v10.dtb
> rockchip/rk3588s-evb1-v10.dtb rockchip/rk3399-sapphire-excavator.dtb
> rockchip/rk3576-evb1-v10.dtb -j4
>
> It results in validation errors like these:
>
> /home/ding/drm-misc/arch/arm64/boot/dts/rockchip/rk3576-evb1-v10.dtb:
> edp@27dc0000: ports:port@1:endpoint: Unevaluated properties are not allowed
> ('data-lanes' was unexpected)
> from schema $id:
> http://devicetree.org/schemas/display/rockchip/rockchip,analogix-dp.yaml#
> /home/ding/drm-misc/arch/arm64/boot/dts/rockchip/rk3576-evb1-v10.dtb:
> edp@27dc0000: Unevaluated properties are not allowed ('force-hpd',
> 'interrupts', 'phy-names', 'phys', 'ports', 'reg' were unexpected)
> from schema $id:
> http://devicetree.org/schemas/display/rockchip/rockchip,analogix-dp.yaml#
>
> I suspect that the properties defined in the child binding are
> overriding/masking all the inherited properties from the parent Analogix DP
> schema.
>
> Is there a better way to fix this issue without explicitly listing all
> inherited properties as true?
The example in this file uses most of the properties that you mention
above:
dp@ff970000 {
compatible = "rockchip,rk3288-dp";
reg = <0xff970000 0x4000>;
interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_EDP>, <&cru PCLK_EDP_CTRL>;
clock-names = "dp", "pclk";
phys = <&dp_phy>;
phy-names = "dp";
resets = <&cru 111>;
reset-names = "dp";
rockchip,grf = <&grf>;
pinctrl-0 = <&edp_hpd>;
pinctrl-names = "default";
dt_binding_check reports no problems with this node, so I think the
problem might lie elsewhere?
There's no edp node in the dts you mention above, so this looks like an
interaction with something that's not yet upstream.
If this is required for the rk3576 edp, then you should include this
patch in the rk3576 edp support series rather than this one anyway where
it can actually be evaluated alongside the node it apparently causes
problems with.
pw-bot: changes-requested
Thanks,
Conor.
>
> Best regards,
> Damon
>
> > > ---
> > > .../bindings/display/rockchip/rockchip,analogix-dp.yaml | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> > > index bb75d898a5c5..896ded87880f 100644
> > > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> > > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> > > @@ -50,6 +50,13 @@ properties:
> > > aux-bus:
> > > $ref: /schemas/display/dp-aux-bus.yaml#
> > > + reg: true
> > > + interrupts: true
> > > + phys: true
> > > + phy-names: true
> > > + force-hpd: true
> > > + ports: true
> > > +
> > > required:
> > > - compatible
> > > - clocks
> > > --
> > > 2.34.1
> > >
> > >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Claude review: dt-bindings: display: rockchip: analogix-dp: Expose inherited properties
2026-05-14 7:01 ` [PATCH v1 1/3] dt-bindings: display: rockchip: analogix-dp: Expose inherited properties Damon Ding
2026-05-14 18:16 ` Conor Dooley
@ 2026-05-16 1:13 ` Claude Code Review Bot
1 sibling, 0 replies; 13+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 1:13 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Looks correct.**
This patch adds `true` declarations for properties inherited from the base `analogix,dp.yaml` schema to satisfy `unevaluatedProperties: false` in the Rockchip binding file. The properties exposed (`reg`, `interrupts`, `phys`, `phy-names`, `force-hpd`, `ports`) all exist in the parent schema and are already used in the example DT node at the bottom of the file (e.g., `reg`, `interrupts`, `phys`, `phy-names`).
```yaml
+ reg: true
+ interrupts: true
+ phys: true
+ phy-names: true
+ force-hpd: true
+ ports: true
```
This is a straightforward schema fix. It's technically a prerequisite for the series (since patch 2 modifies the parent schema), but it's also a valid standalone fix — these properties should have been listed already given the `unevaluatedProperties: false` constraint.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 2/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
2026-05-14 7:01 [PATCH v1 0/3] Add eDP lane mapping support Damon Ding
2026-05-14 7:01 ` [PATCH v1 1/3] dt-bindings: display: rockchip: analogix-dp: Expose inherited properties Damon Ding
@ 2026-05-14 7:01 ` Damon Ding
2026-05-14 18:19 ` Conor Dooley
2026-05-16 1:13 ` Claude review: " Claude Code Review Bot
2026-05-14 7:01 ` [PATCH v1 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping Damon Ding
2026-05-16 1:13 ` Claude review: Add eDP lane mapping support Claude Code Review Bot
3 siblings, 2 replies; 13+ messages in thread
From: Damon Ding @ 2026-05-14 7:01 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
Add data-lanes property support to the port@1 endpoint for physical
lane mapping configuration.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../bindings/display/bridge/analogix,dp.yaml | 24 +++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
index 62f0521b0924..a82f9b7776c0 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
@@ -36,19 +36,35 @@ properties:
Hotplug detect GPIO.
Indicates which GPIO should be used for hotplug detection
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ deprecated: true
+ minItems: 1
+ maxItems: 4
+ items:
+ maximum: 3
+
ports:
$ref: /schemas/graph.yaml#/properties/ports
properties:
port@0:
$ref: /schemas/graph.yaml#/properties/port
- description:
- Input node to receive pixel data.
+ description: Input node to receive pixel data.
port@1:
$ref: /schemas/graph.yaml#/properties/port
- description:
- Port node with one endpoint connected to a dp-connector node.
+ description: Port node with one endpoint connected to sink device node.
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+ items:
+ enum: [ 0, 1, 2, 3 ]
required:
- port@0
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v1 2/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
2026-05-14 7:01 ` [PATCH v1 2/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
@ 2026-05-14 18:19 ` Conor Dooley
2026-05-15 2:53 ` Damon Ding
2026-05-16 1:13 ` Claude review: " Claude Code Review Bot
1 sibling, 1 reply; 13+ messages in thread
From: Conor Dooley @ 2026-05-14 18:19 UTC (permalink / raw)
To: Damon Ding
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]
On Thu, May 14, 2026 at 03:01:32PM +0800, Damon Ding wrote:
> Add data-lanes property support to the port@1 endpoint for physical
> lane mapping configuration.
>
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> ---
> .../bindings/display/bridge/analogix,dp.yaml | 24 +++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
> index 62f0521b0924..a82f9b7776c0 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
> @@ -36,19 +36,35 @@ properties:
> Hotplug detect GPIO.
> Indicates which GPIO should be used for hotplug detection
>
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + deprecated: true
Why are you adding a new property as deprecated? Why does this duplicate
what you're adding to the port node? At the very least, your commit is
lacking an explanation.
pw-bot: changes-requested
Cheers,
Conor.
> + minItems: 1
> + maxItems: 4
> + items:
> + maximum: 3
> +
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
>
> properties:
> port@0:
> $ref: /schemas/graph.yaml#/properties/port
> - description:
> - Input node to receive pixel data.
> + description: Input node to receive pixel data.
>
> port@1:
> $ref: /schemas/graph.yaml#/properties/port
> - description:
> - Port node with one endpoint connected to a dp-connector node.
> + description: Port node with one endpoint connected to sink device node.
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> + items:
> + enum: [ 0, 1, 2, 3 ]
>
> required:
> - port@0
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
2026-05-14 18:19 ` Conor Dooley
@ 2026-05-15 2:53 ` Damon Ding
0 siblings, 0 replies; 13+ messages in thread
From: Damon Ding @ 2026-05-15 2:53 UTC (permalink / raw)
To: Conor Dooley
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
Hi Conor,
On 5/15/2026 2:19 AM, Conor Dooley wrote:
> On Thu, May 14, 2026 at 03:01:32PM +0800, Damon Ding wrote:
>> Add data-lanes property support to the port@1 endpoint for physical
>> lane mapping configuration.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>> ---
>> .../bindings/display/bridge/analogix,dp.yaml | 24 +++++++++++++++----
>> 1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
>> index 62f0521b0924..a82f9b7776c0 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
>> @@ -36,19 +36,35 @@ properties:
>> Hotplug detect GPIO.
>> Indicates which GPIO should be used for hotplug detection
>>
>> + data-lanes:
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + deprecated: true
>
> Why are you adding a new property as deprecated? Why does this duplicate
> what you're adding to the port node? At the very least, your commit is
> lacking an explanation.
> pw-bot: changes-requested
>
Thank you for the review.
I referenced the implementation in
Documentation/devicetree/bindings/display/msm/dp-controller.yaml. After
evaluating QCOM DP driver and other drivers, I agree that implementing
'data-lanes' in the endpoint node is the preferred and common approach
nowadays.
The top-level `data-lanes` property together with the `deprecated` mark
was leftover content from my early reference and arrangement, which I
neglected to delete before submitting. It is redundant, unreasonable and
should not have been added at all.
I will fix it in the next version.
Best regards,
Damon
>
>> + minItems: 1
>> + maxItems: 4
>> + items:
>> + maximum: 3
>> +
>> ports:
>> $ref: /schemas/graph.yaml#/properties/ports
>>
>> properties:
>> port@0:
>> $ref: /schemas/graph.yaml#/properties/port
>> - description:
>> - Input node to receive pixel data.
>> + description: Input node to receive pixel data.
>>
>> port@1:
>> $ref: /schemas/graph.yaml#/properties/port
>> - description:
>> - Port node with one endpoint connected to a dp-connector node.
>> + description: Port node with one endpoint connected to sink device node.
>> + properties:
>> + endpoint:
>> + $ref: /schemas/media/video-interfaces.yaml#
>> + unevaluatedProperties: false
>> + properties:
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>> + items:
>> + enum: [ 0, 1, 2, 3 ]
>>
>> required:
>> - port@0
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Claude review: dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
2026-05-14 7:01 ` [PATCH v1 2/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
2026-05-14 18:19 ` Conor Dooley
@ 2026-05-16 1:13 ` Claude Code Review Bot
1 sibling, 0 replies; 13+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 1:13 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Minor questions.**
The patch adds `data-lanes` at two levels:
1. **Top-level `data-lanes` (deprecated)**:
```yaml
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ deprecated: true
+ minItems: 1
+ maxItems: 4
+ items:
+ maximum: 3
```
2. **Endpoint-level `data-lanes` in `port@1`**:
```yaml
+ port@1:
+ ...
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+ items:
+ enum: [ 0, 1, 2, 3 ]
```
**Questions/Issues:**
- **Top-level deprecated `data-lanes`**: The cover letter doesn't mention backward compatibility with any existing DTS files that use a top-level `data-lanes`. Is there existing hardware using a non-standard top-level `data-lanes`? If not, adding a deprecated property that was never used seems unnecessary. If there are existing users, the driver patch (3/3) doesn't parse it from the top level at all, only from the endpoint. This should be clarified in the commit message.
- **`enum` vs `maximum`**: The top-level uses `maximum: 3` while the endpoint uses `enum: [0, 1, 2, 3]`. These are functionally equivalent for non-negative integers but the inconsistency is slightly odd. The endpoint style (`enum`) is more explicit and preferred.
- The description change from "Port node with one endpoint connected to a dp-connector node" to "...sink device node" is fine — it's more general and accurate for eDP panels that aren't connector nodes.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping
2026-05-14 7:01 [PATCH v1 0/3] Add eDP lane mapping support Damon Ding
2026-05-14 7:01 ` [PATCH v1 1/3] dt-bindings: display: rockchip: analogix-dp: Expose inherited properties Damon Ding
2026-05-14 7:01 ` [PATCH v1 2/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
@ 2026-05-14 7:01 ` Damon Ding
2026-05-16 1:13 ` Claude review: " Claude Code Review Bot
2026-05-16 1:13 ` Claude review: Add eDP lane mapping support Claude Code Review Bot
3 siblings, 1 reply; 13+ messages in thread
From: Damon Ding @ 2026-05-14 7:01 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
Parse the optional 'data-lanes' device tree property to support
custom physical lane mapping configuration.
If no valid configuration is found, fall back to the default
lane map (0, 1, 2, 3) automatically and keep the driver running.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../drm/bridge/analogix/analogix_dp_core.c | 56 +++++++++++++++++++
.../drm/bridge/analogix/analogix_dp_core.h | 4 +-
.../gpu/drm/bridge/analogix/analogix_dp_reg.c | 15 +++--
.../gpu/drm/bridge/analogix/analogix_dp_reg.h | 4 ++
4 files changed, 70 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index c8eb3511f92a..a7d19c3be7e0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1234,6 +1234,59 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
.detect = analogix_dp_bridge_detect,
};
+static int analogix_dp_dt_parse_lanes_map(struct analogix_dp_device *dp)
+{
+ struct video_info *video_info = &dp->video_info;
+ struct device_node *endpoint;
+ u32 tmp[LANE_COUNT4];
+ u32 map[LANE_COUNT4] = {0, 1, 2, 3};
+ bool used[LANE_COUNT4] = {false};
+ int num_lanes;
+ int ret, i;
+
+ memcpy(video_info->lane_map, map, sizeof(map));
+
+ num_lanes = drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, 1,
+ video_info->max_lane_count);
+ if (num_lanes < 0)
+ return -EINVAL;
+
+ endpoint = of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1);
+ if (!endpoint)
+ return -EINVAL;
+
+ ret = of_property_read_u32_array(endpoint, "data-lanes", tmp, num_lanes);
+ of_node_put(endpoint);
+ if (ret)
+ return -EINVAL;
+
+ for (i = 0; i < num_lanes; i++) {
+ if (tmp[i] >= LANE_COUNT4) {
+ dev_dbg(dp->dev, "data-lanes[%d] = %u is out of range\n", i, tmp[i]);
+ return -EINVAL;
+ }
+
+ if (used[tmp[i]]) {
+ dev_dbg(dp->dev, "data-lanes[%d] = %u is duplicate\n", i, tmp[i]);
+ return -EINVAL;
+ }
+
+ used[tmp[i]] = true;
+ map[i] = tmp[i];
+ }
+
+ for (i = 0; i < LANE_COUNT4 && num_lanes < LANE_COUNT4; i++) {
+ if (!used[i])
+ map[num_lanes++] = i;
+ }
+
+ dev_dbg(dp->dev, "Using parsed lane map: <%u %u %u %u>\n", map[0], map[1], map[2], map[3]);
+
+ memcpy(video_info->lane_map, map, sizeof(map));
+
+ return 0;
+}
+
static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
{
struct device_node *dp_node = dp->dev->of_node;
@@ -1266,6 +1319,9 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
break;
}
+ if (analogix_dp_dt_parse_lanes_map(dp))
+ dev_dbg(dp->dev, "No valid data-lanes found, using default lane map\n");
+
return 0;
}
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 17347448c6b0..634fad241e69 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -137,6 +137,8 @@ struct video_info {
int max_link_rate;
enum link_lane_count_type max_lane_count;
+
+ u32 lane_map[LANE_COUNT4];
};
struct link_train {
@@ -175,7 +177,7 @@ struct analogix_dp_device {
/* analogix_dp_reg.c */
void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable);
void analogix_dp_stop_video(struct analogix_dp_device *dp);
-void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable);
+void analogix_dp_lane_mapping(struct analogix_dp_device *dp);
void analogix_dp_init_analog_param(struct analogix_dp_device *dp);
void analogix_dp_init_interrupt(struct analogix_dp_device *dp);
void analogix_dp_reset(struct analogix_dp_device *dp);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 6207ded7ffd5..f5f55bd25062 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -48,16 +48,15 @@ void analogix_dp_stop_video(struct analogix_dp_device *dp)
writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
}
-void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable)
+void analogix_dp_lane_mapping(struct analogix_dp_device *dp)
{
+ u32 *lane_map = dp->video_info.lane_map;
u32 reg;
- if (enable)
- reg = LANE3_MAP_LOGIC_LANE_0 | LANE2_MAP_LOGIC_LANE_1 |
- LANE1_MAP_LOGIC_LANE_2 | LANE0_MAP_LOGIC_LANE_3;
- else
- reg = LANE3_MAP_LOGIC_LANE_3 | LANE2_MAP_LOGIC_LANE_2 |
- LANE1_MAP_LOGIC_LANE_1 | LANE0_MAP_LOGIC_LANE_0;
+ reg = lane_map[0] << LANE0_MAP_SHIFT;
+ reg |= lane_map[1] << LANE1_MAP_SHIFT;
+ reg |= lane_map[2] << LANE2_MAP_SHIFT;
+ reg |= lane_map[3] << LANE3_MAP_SHIFT;
writel(reg, dp->reg_base + ANALOGIX_DP_LANE_MAP);
}
@@ -140,7 +139,7 @@ void analogix_dp_reset(struct analogix_dp_device *dp)
usleep_range(20, 30);
- analogix_dp_lane_swap(dp, 0);
+ analogix_dp_lane_mapping(dp);
writel(0x0, dp->reg_base + ANALOGIX_DP_SYS_CTL_1);
writel(0x40, dp->reg_base + ANALOGIX_DP_SYS_CTL_2);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
index 12735139046c..ac914e37089b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
@@ -209,6 +209,10 @@
#define LANE0_MAP_LOGIC_LANE_1 (0x1 << 0)
#define LANE0_MAP_LOGIC_LANE_2 (0x2 << 0)
#define LANE0_MAP_LOGIC_LANE_3 (0x3 << 0)
+#define LANE3_MAP_SHIFT (6)
+#define LANE2_MAP_SHIFT (4)
+#define LANE1_MAP_SHIFT (2)
+#define LANE0_MAP_SHIFT (0)
/* ANALOGIX_DP_ANALOG_CTL_1 */
#define TX_TERMINAL_CTRL_50_OHM (0x1 << 4)
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Claude review: drm/bridge: analogix_dp: Add support for optional data-lanes mapping
2026-05-14 7:01 ` [PATCH v1 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping Damon Ding
@ 2026-05-16 1:13 ` Claude Code Review Bot
0 siblings, 0 replies; 13+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 1:13 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Has issues to address.**
**1. Error handling in `analogix_dp_dt_parse_lanes_map` returns `-EINVAL` for non-error conditions:**
```c
+ num_lanes = drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, 1,
+ video_info->max_lane_count);
+ if (num_lanes < 0)
+ return -EINVAL;
```
`drm_of_get_data_lanes_count_ep` returns `-ENODEV` when the `data-lanes` property is simply absent, which is the normal case for most boards. Returning `-EINVAL` here loses the distinction between "property not present" (normal) and "property present but invalid" (an error). The caller:
```c
+ if (analogix_dp_dt_parse_lanes_map(dp))
+ dev_dbg(dp->dev, "No valid data-lanes found, using default lane map\n");
```
...treats all failures the same — silently falling back. This means a genuinely malformed `data-lanes` property (e.g., `data-lanes = <0 0 1 1>;` with duplicates, or `data-lanes = <5>;` with out-of-range values) is silently ignored and falls back to the default. This is a **data integrity concern**: a board designer who configures lane mapping but makes a typo will get no indication that their configuration was ignored.
**Suggestion**: Return the actual error. In the caller, only fall back silently on `-ENODEV`; propagate or at minimum `dev_warn()` on other errors so that DT mistakes are visible.
**2. Redundant `memcpy` — the default map is written twice on the success path:**
```c
+ u32 map[LANE_COUNT4] = {0, 1, 2, 3};
+ ...
+ memcpy(video_info->lane_map, map, sizeof(map)); // <-- sets defaults
+ ...
+ // parse and modify map[]
+ ...
+ memcpy(video_info->lane_map, map, sizeof(map)); // <-- sets final
```
The first `memcpy` at the start is only needed for the early-return error paths. Consider moving the initial default write to the caller, or just removing the first `memcpy` and only doing it in the error path. Minor style nit.
**3. `LANE_COUNT4` is an enum value of `4`, not a `#define`:**
```c
+ u32 tmp[LANE_COUNT4];
+ u32 map[LANE_COUNT4] = {0, 1, 2, 3};
+ bool used[LANE_COUNT4] = {false};
```
Using an enum value as an array size is technically fine in C (it's a compile-time constant), but `LANE_COUNT4 = 4` is semantically about link lane count negotiation, not "the fixed number of physical lanes on the chip." A `#define ANALOGIX_DP_MAX_LANES 4` would be clearer in intent, but this is a minor style preference. The existing codebase already uses `LANE_COUNT4` in some places as a max, so this is consistent.
**4. The fill-remaining-lanes logic:**
```c
+ for (i = 0; i < LANE_COUNT4 && num_lanes < LANE_COUNT4; i++) {
+ if (!used[i])
+ map[num_lanes++] = i;
+ }
```
This fills in unmapped physical lanes beyond what `data-lanes` specifies. For example, if `data-lanes = <2 3>`, the final map becomes `{2, 3, 0, 1}`. This behavior seems reasonable for 2-lane configurations where lanes 2 and 3 are unused but the register needs all 4 entries. However, it's not documented anywhere — consider adding a brief comment or noting this in the commit message.
**5. Register shift defines — minor style:**
```c
+#define LANE3_MAP_SHIFT (6)
+#define LANE2_MAP_SHIFT (4)
+#define LANE1_MAP_SHIFT (2)
+#define LANE0_MAP_SHIFT (0)
```
Parentheses around single integer constants are unnecessary per kernel coding style (though this matches the existing defines above like `(0x1 << 6)`). Very minor.
**6. The refactoring from `analogix_dp_lane_swap(dp, bool enable)` to `analogix_dp_lane_mapping(dp)` is clean:**
The old function only supported two states (identity or full reversal). The new function reads from `video_info->lane_map` which supports arbitrary permutations. The only caller was `analogix_dp_reset()` which always passed `enable=0`, so the "swap" functionality was never actually used. The refactoring is safe.
**Summary of patch 3 action items:**
- Differentiate between missing `data-lanes` (normal, silent) and malformed `data-lanes` (should warn)
- Consider removing the redundant first `memcpy`
- Document or mention the fill-remaining-lanes behavior
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 13+ messages in thread
* Claude review: Add eDP lane mapping support
2026-05-14 7:01 [PATCH v1 0/3] Add eDP lane mapping support Damon Ding
` (2 preceding siblings ...)
2026-05-14 7:01 ` [PATCH v1 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping Damon Ding
@ 2026-05-16 1:13 ` Claude Code Review Bot
3 siblings, 0 replies; 13+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 1:13 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: Add eDP lane mapping support
Author: Damon Ding <damon.ding@rock-chips.com>
Patches: 9
Reviewed: 2026-05-16T11:13:13.157156
---
This is a clean, well-structured 3-patch series that adds configurable eDP/DP physical lane mapping via the `data-lanes` device tree property to the Analogix DP bridge driver. The use case is legitimate — correcting PCB lane swaps and adapting mismatched lane pin definitions between SoC and panel without hardware changes.
The series is logically ordered: DT binding fixups first, then binding additions, then the driver implementation. The driver patch is conservative, falling back to the default identity mapping when no `data-lanes` is specified.
I have a few concerns, mostly around the parsing logic in patch 3 that silently absorbs errors that should probably be propagated, and a question about the deprecated top-level `data-lanes` property added in patch 2.
**Overall: Looks reasonable with minor issues to address.**
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 13+ messages in thread