[PATCH v4 0/3] arm64: dts: rockchip: Fix vdec register blocks order on RK3576/RK3588

[PATCH v4 0/3] arm64: dts: rockchip: Fix vdec register blocks order on RK3576/RK3588

[Cristian Ciocaltea]

When building device trees for the RK3576 based boards, DTC shows the
following complaint:

  rk3576.dtsi:1282.30-1304.5: Warning (simple_bus_reg): /soc/video-codec@27b00000: simple-bus unit address format error, expected "27b00100"

The first patch updates 'reg-names' property in rockchip,vdec binding to
allow providing the register blocks following the address-based order
and, consequently, ensure the video decoder unit address points to the
first register range.

The next two patches reorder 'reg' and 'reg-names' for the impacted
RK3576 & RK3588 vdec nodes.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v4:
- Removed the unnecessary one-entry item from the reg-names oneOf clause
- Minor adjustment of the commit descriptions as 'function' naming was
  not used in the context of the older SoCs
- Link to v3: https://lore.kernel.org/r/20260225-vdec-reg-order-rk3576-v3-0-5a2ebe1b11a8@collabora.com

Changes in v3:
- Mark the current 'reg-names' listing in the binding as deprecated and
  introduce an alternative 'link,function,cache' one
- Drop the Fixes tags from all patches and updated commit descriptions
- Link to v2: https://lore.kernel.org/r/20260223-vdec-reg-order-rk3576-v2-0-daf4942dfc02@collabora.com

Changes in v2:
- Added patch for updating rockchip,vdec.yaml binding
- Added patch for updating RK3588 vdec nodes
- Link to v1: https://lore.kernel.org/r/20260223-vdec-reg-order-rk3576-v1-1-560976566bd3@collabora.com

---
Cristian Ciocaltea (3):
      media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}
      arm64: dts: rockchip: Fix vdec register blocks order on RK3576
      arm64: dts: rockchip: Update vdec register blocks order on RK3588

 .../devicetree/bindings/media/rockchip,vdec.yaml     | 20 ++++++++++++--------
 arch/arm64/boot/dts/rockchip/rk3576.dtsi             |  6 +++---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi        | 12 ++++++------
 3 files changed, 21 insertions(+), 17 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260223-vdec-reg-order-rk3576-cc2ec6e05e98

[PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Cristian Ciocaltea]

With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
register blocks have been provided for the video decoder unit.

However, the binding does not properly describe the new hardware layout,
as it breaks the convention expecting the unit address to indicate the
start of the first register range, i.e. 'function' block is listed
before 'link' instead of the opposite.

Since the binding changes have been already released and a fix would
bring up an ABI break, mark the current 'reg-names' ordering as
deprecated and introduce an alternative 'link,function,cache' listing
which follows the address-based ordering according to the TRM.

Additionally, drop the 'reg' description items as the order is not fixed
anymore, while the information they offer is not very relevant anyway.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 .../devicetree/bindings/media/rockchip,vdec.yaml     | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
index 809fda45b3bd..c513b68d2c72 100644
--- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
@@ -28,16 +28,20 @@ properties:
 
   reg:
     minItems: 1
-    items:
-      - description: The function configuration registers base
-      - description: The link table configuration registers base
-      - description: The cache configuration registers base
+    maxItems: 3
 
   reg-names:
-    items:
-      - const: function
-      - const: link
-      - const: cache
+    oneOf:
+      - items:
+          - const: link
+          - const: function
+          - const: cache
+      - items:
+          - const: function
+          - const: link
+          - const: cache
+        deprecated: true
+        description: Use link,function,cache block order instead.
 
   interrupts:
     maxItems: 1

-- 
2.52.0

[PATCH v4 2/3] arm64: dts: rockchip: Fix vdec register blocks order on RK3576

[Cristian Ciocaltea]

When building device trees for the RK3576 based boards, DTC shows the
following complaint:

  rk3576.dtsi:1282.30-1304.5: Warning (simple_bus_reg): /soc/video-codec@27b00000: simple-bus unit address format error, expected "27b00100"

Provide the register blocks using the 'link,function,cache' listing,
which follows the address-based ordering and, implicitly, ensures the
unit address points to the first register range.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3576.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index 49ccdf12ef7e..45eb0d053a6f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
@@ -1281,10 +1281,10 @@ gpu: gpu@27800000 {
 
 		vdec: video-codec@27b00000 {
 			compatible = "rockchip,rk3576-vdec";
-			reg = <0x0 0x27b00100 0x0 0x500>,
-			      <0x0 0x27b00000 0x0 0x100>,
+			reg = <0x0 0x27b00000 0x0 0x100>,
+			      <0x0 0x27b00100 0x0 0x500>,
 			      <0x0 0x27b00600 0x0 0x100>;
-			reg-names = "function", "link", "cache";
+			reg-names = "link", "function", "cache";
 			interrupts = <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cru ACLK_RKVDEC_ROOT>, <&cru HCLK_RKVDEC>,
 				 <&cru ACLK_RKVDEC_ROOT_BAK>, <&cru CLK_RKVDEC_CORE>,

-- 
2.52.0

[PATCH v4 3/3] arm64: dts: rockchip: Update vdec register blocks order on RK3588

[Cristian Ciocaltea]

Provide the register blocks for RK3588 vdec0 & vdec1 nodes using the
'link,function,cache' listing, which follows the address-based ordering
and, implicitly, ensures the unit address points to the first register
range.

This aligns with a similar fix for RK3576 where DTC complained about the
bus address format.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 7fe9593d8c19..b95129f85aba 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1355,10 +1355,10 @@ vepu121_3_mmu: iommu@fdbac800 {
 
 	vdec0: video-codec@fdc38000 {
 		compatible = "rockchip,rk3588-vdec";
-		reg = <0x0 0xfdc38100 0x0 0x500>,
-		      <0x0 0xfdc38000 0x0 0x100>,
+		reg = <0x0 0xfdc38000 0x0 0x100>,
+		      <0x0 0xfdc38100 0x0 0x500>,
 		      <0x0 0xfdc38600 0x0 0x100>;
-		reg-names = "function", "link", "cache";
+		reg-names = "link", "function", "cache";
 		interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH 0>;
 		clocks = <&cru ACLK_RKVDEC0>, <&cru HCLK_RKVDEC0>, <&cru CLK_RKVDEC0_CA>,
 			 <&cru CLK_RKVDEC0_CORE>, <&cru CLK_RKVDEC0_HEVC_CA>;
@@ -1387,10 +1387,10 @@ vdec0_mmu: iommu@fdc38700 {
 
 	vdec1: video-codec@fdc40000 {
 		compatible = "rockchip,rk3588-vdec";
-		reg = <0x0 0xfdc40100 0x0 0x500>,
-		      <0x0 0xfdc40000 0x0 0x100>,
+		reg = <0x0 0xfdc40000 0x0 0x100>,
+		      <0x0 0xfdc40100 0x0 0x500>,
 		      <0x0 0xfdc40600 0x0 0x100>;
-		reg-names = "function", "link", "cache";
+		reg-names = "link", "function", "cache";
 		interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH 0>;
 		clocks = <&cru ACLK_RKVDEC1>, <&cru HCLK_RKVDEC1>, <&cru CLK_RKVDEC1_CA>,
 			 <&cru CLK_RKVDEC1_CORE>, <&cru CLK_RKVDEC1_HEVC_CA>;

-- 
2.52.0

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2915 bytes --]

On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
> register blocks have been provided for the video decoder unit.
> 
> However, the binding does not properly describe the new hardware layout,
> as it breaks the convention expecting the unit address to indicate the
> start of the first register range, i.e. 'function' block is listed
> before 'link' instead of the opposite.

I don't understand this commit message or rationale for an ABI break.
Changing the unit address seems like a "free" fix to your problem,
especially when reg-names is not a required property that you can rely
on. Actually, there may be a bug in the driver - it expects reg-names
for rk3576-vdec and rk3588-vdec but the binding doesn't mandate their
presence for those devices.

Deprecating the order also makes little sense to me, given that some of
these devices only have one reg entry, which as far as I can tell from
looking at the driver *is* the "function" region, so it can never be
entirely deprecated.

Confused,
Conor.

> 
> Since the binding changes have been already released and a fix would
> bring up an ABI break, mark the current 'reg-names' ordering as
> deprecated and introduce an alternative 'link,function,cache' listing
> which follows the address-based ordering according to the TRM.
> 
> Additionally, drop the 'reg' description items as the order is not fixed
> anymore, while the information they offer is not very relevant anyway.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  .../devicetree/bindings/media/rockchip,vdec.yaml     | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> index 809fda45b3bd..c513b68d2c72 100644
> --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> @@ -28,16 +28,20 @@ properties:
>  
>    reg:
>      minItems: 1
> -    items:
> -      - description: The function configuration registers base
> -      - description: The link table configuration registers base
> -      - description: The cache configuration registers base
> +    maxItems: 3
>  
>    reg-names:
> -    items:
> -      - const: function
> -      - const: link
> -      - const: cache
> +    oneOf:
> +      - items:
> +          - const: link
> +          - const: function
> +          - const: cache
> +      - items:
> +          - const: function
> +          - const: link
> +          - const: cache
> +        deprecated: true
> +        description: Use link,function,cache block order instead.
>  
>    interrupts:
>      maxItems: 1
> 
> -- 
> 2.52.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 4412 bytes --]

Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
> > With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
> > register blocks have been provided for the video decoder unit.
> > 
> > However, the binding does not properly describe the new hardware layout,
> > as it breaks the convention expecting the unit address to indicate the
> > start of the first register range, i.e. 'function' block is listed
> > before 'link' instead of the opposite.
> 
> I don't understand this commit message or rationale for an ABI break.
> Changing the unit address seems like a "free" fix to your problem,
> especially when reg-names is not a required property that you can rely
> on. Actually, there may be a bug in the driver - it expects reg-names
> for rk3576-vdec and rk3588-vdec but the binding doesn't mandate their
> presence for those devices.

If the bindings had been held instead of released early, the order would be what
is done in this patch for sure, and no one would have complained. These binding
have never been used by anyone so far, and what you are asking is to create a
DTS that deviate from the vendor provided documentation for the IP base address.
I know you have all technical fancy reasoning, but I like when things are
functional and effective to work with.

> 
> Deprecating the order also makes little sense to me, given that some of
> these devices only have one reg entry, which as far as I can tell from
> looking at the driver *is* the "function" region, so it can never be
> entirely deprecated.

What I'd like to see, is a binding expression that behave like a set, not a
list, and leave the ordering open. As people keep repeating, there is nothing in
a binding that assist to define the right ordering (its not address or base
addres aware). That basically means, we can't as reviewer see that ordering is
going to imposing using a base address in the unit name (which is a convenience,
not a rule I suppose) that differ from the vendor documented base address.

By explicitly removing the ordering in the binding, we create a strict rule that
driver should retrieve this by name, and never assume the ordering, which I
personally like.

thoughts ?

Nicolas

> 
> Confused,
> Conor.
> 
> > 
> > Since the binding changes have been already released and a fix would
> > bring up an ABI break, mark the current 'reg-names' ordering as
> > deprecated and introduce an alternative 'link,function,cache' listing
> > which follows the address-based ordering according to the TRM.
> > 
> > Additionally, drop the 'reg' description items as the order is not fixed
> > anymore, while the information they offer is not very relevant anyway.
> > 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > ---
> >  .../devicetree/bindings/media/rockchip,vdec.yaml     | 20 ++++++++++++-----
> > ---
> >  1 file changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > index 809fda45b3bd..c513b68d2c72 100644
> > --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > @@ -28,16 +28,20 @@ properties:
> >  
> >    reg:
> >      minItems: 1
> > -    items:
> > -      - description: The function configuration registers base
> > -      - description: The link table configuration registers base
> > -      - description: The cache configuration registers base
> > +    maxItems: 3
> >  
> >    reg-names:
> > -    items:
> > -      - const: function
> > -      - const: link
> > -      - const: cache
> > +    oneOf:
> > +      - items:
> > +          - const: link
> > +          - const: function
> > +          - const: cache
> > +      - items:
> > +          - const: function
> > +          - const: link
> > +          - const: cache
> > +        deprecated: true
> > +        description: Use link,function,cache block order instead.
> >  
> >    interrupts:
> >      maxItems: 1
> > 
> > -- 
> > 2.52.0
> > 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 6355 bytes --]

On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
> Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> > On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
> > > With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
> > > register blocks have been provided for the video decoder unit.
> > > 
> > > However, the binding does not properly describe the new hardware layout,
> > > as it breaks the convention expecting the unit address to indicate the
> > > start of the first register range, i.e. 'function' block is listed
> > > before 'link' instead of the opposite.
> > 
> > I don't understand this commit message or rationale for an ABI break.
> > Changing the unit address seems like a "free" fix to your problem,
> > especially when reg-names is not a required property that you can rely
> > on. Actually, there may be a bug in the driver - it expects reg-names
> > for rk3576-vdec and rk3588-vdec but the binding doesn't mandate their
> > presence for those devices.
> 
> If the bindings had been held instead of released early, the order would be what
> is done in this patch for sure, and no one would have complained. These binding
> have never been used by anyone so far, and what you are asking is to create a

20:35:32 conor /stuff/linux$ rg "rk3576-vdec"

arch/arm64/boot/dts/rockchip/rk3576.dtsi
1283:			compatible = "rockchip,rk3576-vdec";

20:35:34 conor /stuff/linux$ rg rk3588-vdec

arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
1357:		compatible = "rockchip,rk3588-vdec";
1389:		compatible = "rockchip,rk3588-vdec";

These look like users, at least at first look. Some elaboration on that
would be good - impact on users definitely should be covered in the
commit message of an ABI break.

> DTS that deviate from the vendor provided documentation for the IP base address.
> I know you have all technical fancy reasoning, but I like when things are
> functional and effective to work with.

FWIW, I don't think you were trying to be dismissive, just trying to
make a valid point about usability being important, but providing clear
justifications for why the order of properties should be changed (or
other similar ABI breaks) is really important. This is v4, and the
rationale you've given here about the documentation, which is the actual
motivation for changing things around, doesn't appear in the commit
message, even after a back and forth with Krzysztof in an earlier
version.

> > Deprecating the order also makes little sense to me, given that some of
> > these devices only have one reg entry, which as far as I can tell from
> > looking at the driver *is* the "function" region, so it can never be
> > entirely deprecated.
> 
> What I'd like to see, is a binding expression that behave like a set, not a
> list, and leave the ordering open. As people keep repeating, there is nothing in
> a binding that assist to define the right ordering (its not address or base
> addres aware). That basically means, we can't as reviewer see that ordering is
> going to imposing using a base address in the unit name (which is a convenience,
> not a rule I suppose) that differ from the vendor documented base address.
> 
> By explicitly removing the ordering in the binding, we create a strict rule that
> driver should retrieve this by name, and never assume the ordering, which I
> personally like.
> 
> thoughts ?

Yeah, you can do this, but to avoid potential breaks you have to do it
from the start, not after the fact. Probably there's bindings that get
acked every day that do do this. Even the retcon is okay to do when
reg-names is mandated by the binding and the users use reg-names in my
opinion.

In this case, the driver is currently buggy, because, as I mentioned, it
uses reg-names without reg-names being required on the platforms with
more than 1 reg property. Probably the binding should make reg-names
mandatory for these platforms even without this patch, but it *has* to
IMO for this proposed change to be applicable.

But anyway, the takeaway from my original mail should be "Conor is not
happy with the rationale the commit message provides for this change,
as it doesn't explain why you want to do it". I necessarily object to
the change itself.

Conor.

> > > Since the binding changes have been already released and a fix would
> > > bring up an ABI break, mark the current 'reg-names' ordering as
> > > deprecated and introduce an alternative 'link,function,cache' listing
> > > which follows the address-based ordering according to the TRM.
> > > 
> > > Additionally, drop the 'reg' description items as the order is not fixed
> > > anymore, while the information they offer is not very relevant anyway.
> > > 
> > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > > ---
> > >  .../devicetree/bindings/media/rockchip,vdec.yaml     | 20 ++++++++++++-----
> > > ---
> > >  1 file changed, 12 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > > b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > > index 809fda45b3bd..c513b68d2c72 100644
> > > --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > > +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > > @@ -28,16 +28,20 @@ properties:
> > >  
> > >    reg:
> > >      minItems: 1
> > > -    items:
> > > -      - description: The function configuration registers base
> > > -      - description: The link table configuration registers base
> > > -      - description: The cache configuration registers base
> > > +    maxItems: 3
> > >  
> > >    reg-names:
> > > -    items:
> > > -      - const: function
> > > -      - const: link
> > > -      - const: cache
> > > +    oneOf:
> > > +      - items:
> > > +          - const: link
> > > +          - const: function
> > > +          - const: cache
> > > +      - items:
> > > +          - const: function
> > > +          - const: link
> > > +          - const: cache
> > > +        deprecated: true
> > > +        description: Use link,function,cache block order instead.
> > >  
> > >    interrupts:
> > >      maxItems: 1
> > > 
> > > -- 
> > > 2.52.0
> > > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 9051 bytes --]

Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
> On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
> > Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> > > On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
> > > > With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
> > > > register blocks have been provided for the video decoder unit.
> > > > 
> > > > However, the binding does not properly describe the new hardware layout,
> > > > as it breaks the convention expecting the unit address to indicate the
> > > > start of the first register range, i.e. 'function' block is listed
> > > > before 'link' instead of the opposite.
> > > 
> > > I don't understand this commit message or rationale for an ABI break.
> > > Changing the unit address seems like a "free" fix to your problem,
> > > especially when reg-names is not a required property that you can rely
> > > on. Actually, there may be a bug in the driver - it expects reg-names
> > > for rk3576-vdec and rk3588-vdec but the binding doesn't mandate their
> > > presence for those devices.
> > 
> > If the bindings had been held instead of released early, the order would be what
> > is done in this patch for sure, and no one would have complained. These binding
> > have never been used by anyone so far, and what you are asking is to create a
> 
> 20:35:32 conor /stuff/linux$ rg "rk3576-vdec"
> 
> arch/arm64/boot/dts/rockchip/rk3576.dtsi
> 1283:			compatible = "rockchip,rk3576-vdec";
> 
> 20:35:34 conor /stuff/linux$ rg rk3588-vdec
> 
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> 1357:		compatible = "rockchip,rk3588-vdec";
> 1389:		compatible = "rockchip,rk3588-vdec";
> 
> These look like users, at least at first look. Some elaboration on that
> would be good - impact on users definitely should be covered in the
> commit message of an ABI break.

Right, let me add some context. rk3576.dtsi/rk3588-base.dtsi became a user of
the binding in 7.0-rc1, which is a pre-release. In fact, I initially thought the
bindings was merged at the same time, which Krzysztof highlighted as my mistake.
And the driver modification that utilize multiple reg ranges was also introduced
in 7.0-rc1. More clarification on that below.

> 
> > DTS that deviate from the vendor provided documentation for the IP base address.
> > I know you have all technical fancy reasoning, but I like when things are
> > functional and effective to work with.
> 
> FWIW, I don't think you were trying to be dismissive, just trying to
> make a valid point about usability being important, but providing clear
> justifications for why the order of properties should be changed (or
> other similar ABI breaks) is really important. This is v4, and the
> rationale you've given here about the documentation, which is the actual
> motivation for changing things around, doesn't appear in the commit
> message, even after a back and forth with Krzysztof in an earlier
> version.

Ack, I'm not the one doing the work (just a reviewer), but was involved in
picking the work, and trying to learn enough so I can perhaps catch this next
time.

> 
> > > Deprecating the order also makes little sense to me, given that some of
> > > these devices only have one reg entry, which as far as I can tell from
> > > looking at the driver *is* the "function" region, so it can never be
> > > entirely deprecated.
> > 
> > What I'd like to see, is a binding expression that behave like a set, not a
> > list, and leave the ordering open. As people keep repeating, there is nothing in
> > a binding that assist to define the right ordering (its not address or base
> > addres aware). That basically means, we can't as reviewer see that ordering is
> > going to imposing using a base address in the unit name (which is a convenience,
> > not a rule I suppose) that differ from the vendor documented base address.
> > 
> > By explicitly removing the ordering in the binding, we create a strict rule that
> > driver should retrieve this by name, and never assume the ordering, which I
> > personally like.
> > 
> > thoughts ?
> 
> Yeah, you can do this, but to avoid potential breaks you have to do it
> from the start, not after the fact. Probably there's bindings that get
> acked every day that do do this. Even the retcon is okay to do when
> reg-names is mandated by the binding and the users use reg-names in my
> opinion.

I think from the above analyses, since the usage only starts in rc1, we have
room for improving it knowing we aren't creating problem for anyone. Note that I
have no idea what the syntax is to "do this", and I doubt either Detlev or
Cristian have a clue.

> 
> In this case, the driver is currently buggy, because, as I mentioned, it
> uses reg-names without reg-names being required on the platforms with
> more than 1 reg property. Probably the binding should make reg-names
> mandatory for these platforms even without this patch, but it *has* to
> IMO for this proposed change to be applicable.

That forced me to check the driver. So for RK33xx and older, there is only one
range, and the driver will just pick the one entry expected:


	if (rkvdec->variant->has_single_reg_region) {
		rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
		if (IS_ERR(rkvdec->regs))
			return PTR_ERR(rkvdec->regs);
	} else {
		rkvdec->regs = devm_platform_ioremap_resource_byname(pdev, "function");
		if (IS_ERR(rkvdec->regs))
			return PTR_ERR(rkvdec->regs);

		rkvdec->link = devm_platform_ioremap_resource_byname(pdev, "link");
		if (IS_ERR(rkvdec->link))
			return PTR_ERR(rkvdec->link);
	}


Where for RK35xx variants, it only pick the resources by name. I don't see the
bug that you see, but I believe this was just a supposition, that you didn't
check the code.

So what happened with the binding, is that in preparation for the new SoC, they
added the named range, but probably without really realizing that there was an
ordering imposed, or realizing the order was far from ideal.

> 
> But anyway, the takeaway from my original mail should be "Conor is not
> happy with the rationale the commit message provides for this change,
> as it doesn't explain why you want to do it". I necessarily object to
> the change itself.

Understood, I think Cristian is a bit sandwich between my rational to make
things match the doc, and Krzysztof requesting to preserve the ABI. I think
that's fixable, but again, if there is a syntax to just remove the ordering from
there, knowing there is no user of that order in released kernel yet, that would
be my preference over deprecation syntax.

regards,
Nicolas

> 
> Conor.
> 
> > > > Since the binding changes have been already released and a fix would
> > > > bring up an ABI break, mark the current 'reg-names' ordering as
> > > > deprecated and introduce an alternative 'link,function,cache' listing
> > > > which follows the address-based ordering according to the TRM.
> > > > 
> > > > Additionally, drop the 'reg' description items as the order is not fixed
> > > > anymore, while the information they offer is not very relevant anyway.
> > > > 
> > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > > > ---
> > > >  .../devicetree/bindings/media/rockchip,vdec.yaml     | 20 ++++++++++++-----
> > > > ---
> > > >  1 file changed, 12 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > > > b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > > > index 809fda45b3bd..c513b68d2c72 100644
> > > > --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > > > @@ -28,16 +28,20 @@ properties:
> > > >  
> > > >    reg:
> > > >      minItems: 1
> > > > -    items:
> > > > -      - description: The function configuration registers base
> > > > -      - description: The link table configuration registers base
> > > > -      - description: The cache configuration registers base
> > > > +    maxItems: 3
> > > >  
> > > >    reg-names:
> > > > -    items:
> > > > -      - const: function
> > > > -      - const: link
> > > > -      - const: cache
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - const: link
> > > > +          - const: function
> > > > +          - const: cache
> > > > +      - items:
> > > > +          - const: function
> > > > +          - const: link
> > > > +          - const: cache
> > > > +        deprecated: true
> > > > +        description: Use link,function,cache block order instead.
> > > >  
> > > >    interrupts:
> > > >      maxItems: 1
> > > > 
> > > > -- 
> > > > 2.52.0
> > > > 
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1883 bytes --]

On Thu, Feb 26, 2026 at 04:56:30PM -0500, Nicolas Dufresne wrote:
> Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
> > On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
> > > Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> > In this case, the driver is currently buggy, because, as I mentioned, it
> > uses reg-names without reg-names being required on the platforms with
> > more than 1 reg property. Probably the binding should make reg-names
> > mandatory for these platforms even without this patch, but it *has* to
> > IMO for this proposed change to be applicable.
> 
> That forced me to check the driver. So for RK33xx and older, there is only one
> range, and the driver will just pick the one entry expected:
> 
> 
> 	if (rkvdec->variant->has_single_reg_region) {
> 		rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
> 		if (IS_ERR(rkvdec->regs))
> 			return PTR_ERR(rkvdec->regs);
> 	} else {
> 		rkvdec->regs = devm_platform_ioremap_resource_byname(pdev, "function");
> 		if (IS_ERR(rkvdec->regs))
> 			return PTR_ERR(rkvdec->regs);
> 
> 		rkvdec->link = devm_platform_ioremap_resource_byname(pdev, "link");
> 		if (IS_ERR(rkvdec->link))
> 			return PTR_ERR(rkvdec->link);
> 	}
> 
> 
> Where for RK35xx variants, it only pick the resources by name. I don't see the
> bug that you see, but I believe this was just a supposition, that you didn't
> check the code.

Busy reading path of exile patch notes, so sniping this comment only...

This is a bug, not a supposition, and it's that snippet from the
driver that prompted my comment.. That code requires that if
->has_single_reg_region is set that the dts provides reg-names, but the
binding does not mandate reg-names for rk3576-vdec and rk3588-vdec, so
the driver will fail to probe on a dts that the binding says is valid.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2621 bytes --]

Le jeudi 26 février 2026 à 22:15 +0000, Conor Dooley a écrit :
> On Thu, Feb 26, 2026 at 04:56:30PM -0500, Nicolas Dufresne wrote:
> > Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
> > > On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
> > > > Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> > > In this case, the driver is currently buggy, because, as I mentioned, it
> > > uses reg-names without reg-names being required on the platforms with
> > > more than 1 reg property. Probably the binding should make reg-names
> > > mandatory for these platforms even without this patch, but it *has* to
> > > IMO for this proposed change to be applicable.
> > 
> > That forced me to check the driver. So for RK33xx and older, there is only
> > one
> > range, and the driver will just pick the one entry expected:
> > 
> > 
> > 	if (rkvdec->variant->has_single_reg_region) {
> > 		rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
> > 		if (IS_ERR(rkvdec->regs))
> > 			return PTR_ERR(rkvdec->regs);
> > 	} else {
> > 		rkvdec->regs = devm_platform_ioremap_resource_byname(pdev,
> > "function");
> > 		if (IS_ERR(rkvdec->regs))
> > 			return PTR_ERR(rkvdec->regs);
> > 
> > 		rkvdec->link = devm_platform_ioremap_resource_byname(pdev,
> > "link");
> > 		if (IS_ERR(rkvdec->link))
> > 			return PTR_ERR(rkvdec->link);
> > 	}
> > 
> > 
> > Where for RK35xx variants, it only pick the resources by name. I don't see
> > the
> > bug that you see, but I believe this was just a supposition, that you didn't
> > check the code.
> 
> Busy reading path of exile patch notes, so sniping this comment only...
> 
> This is a bug, not a supposition, and it's that snippet from the
> driver that prompted my comment.. That code requires that if
> ->has_single_reg_region is set that the dts provides reg-names, but the
> binding does not mandate reg-names for rk3576-vdec and rk3588-vdec, so
> the driver will fail to probe on a dts that the binding says is valid.

Got it, to me nothing in the binding is intentional, just a big lack of
understanding what the syntax meant. So if we agree to make the binding enforce
having names for rk3576-vdec and rk3588-vdec (and keep RK33 and older the same),
It would make me more happy to maintain it.

It also better highlight why adding a second order simply creates an ambiguity.

Let me know, if we can go forward with that, I'll help documenting known users,
their introduction into 7.0-rc1 only and why this incident and fix should have
no impact on anyone.

Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Krzysztof Kozlowski]

On Thu, Feb 26, 2026 at 10:15:49PM +0000, Conor Dooley wrote:
> > 
> > 
> > 	if (rkvdec->variant->has_single_reg_region) {
> > 		rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
> > 		if (IS_ERR(rkvdec->regs))
> > 			return PTR_ERR(rkvdec->regs);
> > 	} else {
> > 		rkvdec->regs = devm_platform_ioremap_resource_byname(pdev, "function");
> > 		if (IS_ERR(rkvdec->regs))
> > 			return PTR_ERR(rkvdec->regs);
> > 
> > 		rkvdec->link = devm_platform_ioremap_resource_byname(pdev, "link");
> > 		if (IS_ERR(rkvdec->link))
> > 			return PTR_ERR(rkvdec->link);
> > 	}
> > 
> > 
> > Where for RK35xx variants, it only pick the resources by name. I don't see the
> > bug that you see, but I believe this was just a supposition, that you didn't
> > check the code.
> 
> Busy reading path of exile patch notes, so sniping this comment only...
> 
> This is a bug, not a supposition, and it's that snippet from the
> driver that prompted my comment.. That code requires that if
> ->has_single_reg_region is set that the dts provides reg-names, but the

No, the opposite. With has_single_reg_region you take first entry and
ignore names.

> binding does not mandate reg-names for rk3576-vdec and rk3588-vdec, so

Best regards,
Krzysztof

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Krzysztof Kozlowski]

On Thu, Feb 26, 2026 at 06:43:31PM +0000, Conor Dooley wrote:
> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
> > With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
> > register blocks have been provided for the video decoder unit.
> > 
> > However, the binding does not properly describe the new hardware layout,
> > as it breaks the convention expecting the unit address to indicate the
> > start of the first register range, i.e. 'function' block is listed
> > before 'link' instead of the opposite.
> 
> I don't understand this commit message or rationale for an ABI break.


The description is indeed wrong. The binding properly describes new
hardware layout. They just don't like it.

Hardware has three separate address spaces and EXACT three correct
separate address spaces are described the binding.

> Changing the unit address seems like a "free" fix to your problem,
> especially when reg-names is not a required property that you can rely
> on. Actually, there may be a bug in the driver - it expects reg-names
> for rk3576-vdec and rk3588-vdec but the binding doesn't mandate their
> presence for those devices.
> 
> Deprecating the order also makes little sense to me, given that some of
> these devices only have one reg entry, which as far as I can tell from
> looking at the driver *is* the "function" region, so it can never be
> entirely deprecated.

There is "if" at the bottom of the binding, so the one entry does not
use reg-names.

Best regards,
Krzysztof

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Krzysztof Kozlowski]

On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
> register blocks have been provided for the video decoder unit.
> 
> However, the binding does not properly describe the new hardware layout,

As you shown me last time with excerpt of address spaces from
datasheet/manual, the binding correctly describes the hardware and above
sentence is not true.

> as it breaks the convention expecting the unit address to indicate the
> start of the first register range, i.e. 'function' block is listed

Imprecise wording. "start of the main or primary register range"

(if you have 0x1000 with one reg and 0x20000000 with everything, the
unit address will be 0x20000000).

> before 'link' instead of the opposite.
> 
> Since the binding changes have been already released and a fix would
> bring up an ABI break, mark the current 'reg-names' ordering as
> deprecated and introduce an alternative 'link,function,cache' listing
> which follows the address-based ordering according to the TRM.
> 
> Additionally, drop the 'reg' description items as the order is not fixed
> anymore, while the information they offer is not very relevant anyway.

This is fine for me.

Best regards,
Krzysztof

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]

On Fri, Feb 27, 2026 at 08:38:13AM +0100, Krzysztof Kozlowski wrote:
> On Thu, Feb 26, 2026 at 10:15:49PM +0000, Conor Dooley wrote:
> > > 
> > > 
> > > 	if (rkvdec->variant->has_single_reg_region) {
> > > 		rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
> > > 		if (IS_ERR(rkvdec->regs))
> > > 			return PTR_ERR(rkvdec->regs);
> > > 	} else {
> > > 		rkvdec->regs = devm_platform_ioremap_resource_byname(pdev, "function");
> > > 		if (IS_ERR(rkvdec->regs))
> > > 			return PTR_ERR(rkvdec->regs);
> > > 
> > > 		rkvdec->link = devm_platform_ioremap_resource_byname(pdev, "link");
> > > 		if (IS_ERR(rkvdec->link))
> > > 			return PTR_ERR(rkvdec->link);
> > > 	}
> > > 
> > > 
> > > Where for RK35xx variants, it only pick the resources by name. I don't see the
> > > bug that you see, but I believe this was just a supposition, that you didn't
> > > check the code.
> > 
> > Busy reading path of exile patch notes, so sniping this comment only...
> > 
> > This is a bug, not a supposition, and it's that snippet from the
> > driver that prompted my comment.. That code requires that if
> > ->has_single_reg_region is set that the dts provides reg-names, but the
> 
> No, the opposite. With has_single_reg_region you take first entry and
> ignore names.

Right, that's of course what I meant, I just a word ;)

> > binding does not mandate reg-names for rk3576-vdec and rk3588-vdec, so
> 
> Best regards,
> Krzysztof
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Cristian Ciocaltea]

Hi Krzysztof, Conor,

On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>> register blocks have been provided for the video decoder unit.
>>
>> However, the binding does not properly describe the new hardware layout,
> 
> As you shown me last time with excerpt of address spaces from
> datasheet/manual, the binding correctly describes the hardware and above
> sentence is not true.
> 
>> as it breaks the convention expecting the unit address to indicate the
>> start of the first register range, i.e. 'function' block is listed
> 
> Imprecise wording. "start of the main or primary register range"
> 
> (if you have 0x1000 with one reg and 0x20000000 with everything, the
> unit address will be 0x20000000).
> 
>> before 'link' instead of the opposite.
>>
>> Since the binding changes have been already released and a fix would
>> bring up an ABI break, mark the current 'reg-names' ordering as
>> deprecated and introduce an alternative 'link,function,cache' listing
>> which follows the address-based ordering according to the TRM.
>>
>> Additionally, drop the 'reg' description items as the order is not fixed
>> anymore, while the information they offer is not very relevant anyway.
> 
> This is fine for me.

Thanks for the additional feedback!

If I'm not mistaken (please correct me), the only remaining (hard)
blocker for the series would be to improve this commit message.

How about the following:

    With the introduction of the RK3588 SoC, and RK3576 afterwards, three
    register blocks have been provided for the video decoder unit instead of
    just one, which are further referenced in the datasheet by 'link table',
    'function' and 'cache'.  The former is present at the top of the
    listing, starting at video decoder unit base address.

    However, while documenting RK3588, the binding broke the convention
    expecting the unit address to indicate the start of the primary register
    range, i.e. the 'function' block got listed before the 'link' one.

    Since the binding changes have been already released and a fix would
    bring up an ABI break, mark the current 'reg-names' ordering as
    deprecated and introduce an alternative 'link,function,cache' listing
    which follows the address-based ordering according to the TRM.

    Additionally, drop the 'reg' description items as the order is not fixed
    anymore, while the information they offer is not very relevant anyway.

Regards,
Cristian

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Krzysztof Kozlowski]

On 27/02/2026 12:37, Cristian Ciocaltea wrote:
> Hi Krzysztof, Conor,
> 
> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
>> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>>> register blocks have been provided for the video decoder unit.
>>>
>>> However, the binding does not properly describe the new hardware layout,
>>
>> As you shown me last time with excerpt of address spaces from
>> datasheet/manual, the binding correctly describes the hardware and above
>> sentence is not true.
>>
>>> as it breaks the convention expecting the unit address to indicate the
>>> start of the first register range, i.e. 'function' block is listed
>>
>> Imprecise wording. "start of the main or primary register range"
>>
>> (if you have 0x1000 with one reg and 0x20000000 with everything, the
>> unit address will be 0x20000000).
>>
>>> before 'link' instead of the opposite.
>>>
>>> Since the binding changes have been already released and a fix would
>>> bring up an ABI break, mark the current 'reg-names' ordering as
>>> deprecated and introduce an alternative 'link,function,cache' listing
>>> which follows the address-based ordering according to the TRM.
>>>
>>> Additionally, drop the 'reg' description items as the order is not fixed
>>> anymore, while the information they offer is not very relevant anyway.
>>
>> This is fine for me.
> 
> Thanks for the additional feedback!
> 
> If I'm not mistaken (please correct me), the only remaining (hard)
> blocker for the series would be to improve this commit message.
> 
> How about the following:
> 
>     With the introduction of the RK3588 SoC, and RK3576 afterwards, three
>     register blocks have been provided for the video decoder unit instead of
>     just one, which are further referenced in the datasheet by 'link table',
>     'function' and 'cache'.  The former is present at the top of the
>     listing, starting at video decoder unit base address.
> 
>     However, while documenting RK3588, the binding broke the convention
>     expecting the unit address to indicate the start of the primary register
>     range, i.e. the 'function' block got listed before the 'link' one.
> 
>     Since the binding changes have been already released and a fix would
>     bring up an ABI break, mark the current 'reg-names' ordering as
>     deprecated and introduce an alternative 'link,function,cache' listing
>     which follows the address-based ordering according to the TRM.
> 
>     Additionally, drop the 'reg' description items as the order is not fixed
>     anymore, while the information they offer is not very relevant anyway.

Yes, it's fine. My comments were actually not blocking, I just wanted to
wait if discussion with Conor resolves somehow.

But for me anyway:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>


Best regards,
Krzysztof

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3094 bytes --]

On Fri, Feb 27, 2026 at 01:37:17PM +0200, Cristian Ciocaltea wrote:
> Hi Krzysztof, Conor,
> 
> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
> > On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
> >> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
> >> register blocks have been provided for the video decoder unit.
> >>
> >> However, the binding does not properly describe the new hardware layout,
> > 
> > As you shown me last time with excerpt of address spaces from
> > datasheet/manual, the binding correctly describes the hardware and above
> > sentence is not true.
> > 
> >> as it breaks the convention expecting the unit address to indicate the
> >> start of the first register range, i.e. 'function' block is listed
> > 
> > Imprecise wording. "start of the main or primary register range"
> > 
> > (if you have 0x1000 with one reg and 0x20000000 with everything, the
> > unit address will be 0x20000000).
> > 
> >> before 'link' instead of the opposite.
> >>
> >> Since the binding changes have been already released and a fix would
> >> bring up an ABI break, mark the current 'reg-names' ordering as
> >> deprecated and introduce an alternative 'link,function,cache' listing
> >> which follows the address-based ordering according to the TRM.
> >>
> >> Additionally, drop the 'reg' description items as the order is not fixed
> >> anymore, while the information they offer is not very relevant anyway.
> > 
> > This is fine for me.
> 
> Thanks for the additional feedback!
> 
> If I'm not mistaken (please correct me), the only remaining (hard)
> blocker for the series would be to improve this commit message.

No, you also need to fix the problem I pointed out about reg-names being
optional on the devices you're relying on reg-names for. The new commit
message I am happy with, provided you also add the information Nicolas
provided about the impact on users.

> 
> How about the following:
> 
>     With the introduction of the RK3588 SoC, and RK3576 afterwards, three
>     register blocks have been provided for the video decoder unit instead of
>     just one, which are further referenced in the datasheet by 'link table',
>     'function' and 'cache'.  The former is present at the top of the
>     listing, starting at video decoder unit base address.
> 
>     However, while documenting RK3588, the binding broke the convention
>     expecting the unit address to indicate the start of the primary register
>     range, i.e. the 'function' block got listed before the 'link' one.
> 
>     Since the binding changes have been already released and a fix would
>     bring up an ABI break, mark the current 'reg-names' ordering as
>     deprecated and introduce an alternative 'link,function,cache' listing
>     which follows the address-based ordering according to the TRM.
> 
>     Additionally, drop the 'reg' description items as the order is not fixed
>     anymore, while the information they offer is not very relevant anyway.
> 
> Regards,
> Cristian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2294 bytes --]

On Thu, Feb 26, 2026 at 04:56:30PM -0500, Nicolas Dufresne wrote:
> Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
> > On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
> > > Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :

> > > > Deprecating the order also makes little sense to me, given that some of
> > > > these devices only have one reg entry, which as far as I can tell from
> > > > looking at the driver *is* the "function" region, so it can never be
> > > > entirely deprecated.
> > > 
> > > What I'd like to see, is a binding expression that behave like a set, not a
> > > list, and leave the ordering open. As people keep repeating, there is nothing in
> > > a binding that assist to define the right ordering (its not address or base
> > > addres aware). That basically means, we can't as reviewer see that ordering is
> > > going to imposing using a base address in the unit name (which is a convenience,
> > > not a rule I suppose) that differ from the vendor documented base address.
> > > 
> > > By explicitly removing the ordering in the binding, we create a strict rule that
> > > driver should retrieve this by name, and never assume the ordering, which I
> > > personally like.
> > > 
> > > thoughts ?
> > 
> > Yeah, you can do this, but to avoid potential breaks you have to do it
> > from the start, not after the fact. Probably there's bindings that get
> > acked every day that do do this. Even the retcon is okay to do when
> > reg-names is mandated by the binding and the users use reg-names in my
> > opinion.
> 
> I think from the above analyses, since the usage only starts in rc1, we have
> room for improving it knowing we aren't creating problem for anyone. Note that I
> have no idea what the syntax is to "do this", and I doubt either Detlev or
> Cristian have a clue.

I think this is the only bit that really still needs a reply, this can
be solved by adding reg-names as "required" to the existing conditional
portion of the binding. There's probably hundreds of examples if one
does a search for "then:\n.*required:" to use a basis for the change
here. Probably should be an independent change, since it is needed even
without the re-order given the bug I brought up.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Cristian Ciocaltea]

On 2/27/26 7:13 PM, Conor Dooley wrote:
> On Fri, Feb 27, 2026 at 01:37:17PM +0200, Cristian Ciocaltea wrote:
>> Hi Krzysztof, Conor,
>>
>> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
>>> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>>>> register blocks have been provided for the video decoder unit.
>>>>
>>>> However, the binding does not properly describe the new hardware layout,
>>>
>>> As you shown me last time with excerpt of address spaces from
>>> datasheet/manual, the binding correctly describes the hardware and above
>>> sentence is not true.
>>>
>>>> as it breaks the convention expecting the unit address to indicate the
>>>> start of the first register range, i.e. 'function' block is listed
>>>
>>> Imprecise wording. "start of the main or primary register range"
>>>
>>> (if you have 0x1000 with one reg and 0x20000000 with everything, the
>>> unit address will be 0x20000000).
>>>
>>>> before 'link' instead of the opposite.
>>>>
>>>> Since the binding changes have been already released and a fix would
>>>> bring up an ABI break, mark the current 'reg-names' ordering as
>>>> deprecated and introduce an alternative 'link,function,cache' listing
>>>> which follows the address-based ordering according to the TRM.
>>>>
>>>> Additionally, drop the 'reg' description items as the order is not fixed
>>>> anymore, while the information they offer is not very relevant anyway.
>>>
>>> This is fine for me.
>>
>> Thanks for the additional feedback!
>>
>> If I'm not mistaken (please correct me), the only remaining (hard)
>> blocker for the series would be to improve this commit message.
> 
> No, you also need to fix the problem I pointed out about reg-names being
> optional on the devices you're relying on reg-names for. 

My only concern is that by marking reg-names as required we would break the ABI,
since the RK3588 related changes in the binding (not the DTS ones) got already
released (i.e. since v6.17). That's also the reason we went with this deprecated
order approach.

> The new commit
> message I am happy with, provided you also add the information Nicolas
> provided about the impact on users.

Nicolas, can you please provide here the statement so that we can agree on the
wording?

Thanks,
Cristian

> 
>>
>> How about the following:
>>
>>     With the introduction of the RK3588 SoC, and RK3576 afterwards, three
>>     register blocks have been provided for the video decoder unit instead of
>>     just one, which are further referenced in the datasheet by 'link table',
>>     'function' and 'cache'.  The former is present at the top of the
>>     listing, starting at video decoder unit base address.
>>
>>     However, while documenting RK3588, the binding broke the convention
>>     expecting the unit address to indicate the start of the primary register
>>     range, i.e. the 'function' block got listed before the 'link' one.
>>
>>     Since the binding changes have been already released and a fix would
>>     bring up an ABI break, mark the current 'reg-names' ordering as
>>     deprecated and introduce an alternative 'link,function,cache' listing
>>     which follows the address-based ordering according to the TRM.
>>
>>     Additionally, drop the 'reg' description items as the order is not fixed
>>     anymore, while the information they offer is not very relevant anyway.
>>
>> Regards,
>> Cristian

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Cristian Ciocaltea]

On 2/27/26 7:18 PM, Conor Dooley wrote:
> On Thu, Feb 26, 2026 at 04:56:30PM -0500, Nicolas Dufresne wrote:
>> Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
>>> On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
>>>> Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> 
>>>>> Deprecating the order also makes little sense to me, given that some of
>>>>> these devices only have one reg entry, which as far as I can tell from
>>>>> looking at the driver *is* the "function" region, so it can never be
>>>>> entirely deprecated.
>>>>
>>>> What I'd like to see, is a binding expression that behave like a set, not a
>>>> list, and leave the ordering open. As people keep repeating, there is nothing in
>>>> a binding that assist to define the right ordering (its not address or base
>>>> addres aware). That basically means, we can't as reviewer see that ordering is
>>>> going to imposing using a base address in the unit name (which is a convenience,
>>>> not a rule I suppose) that differ from the vendor documented base address.
>>>>
>>>> By explicitly removing the ordering in the binding, we create a strict rule that
>>>> driver should retrieve this by name, and never assume the ordering, which I
>>>> personally like.
>>>>
>>>> thoughts ?
>>>
>>> Yeah, you can do this, but to avoid potential breaks you have to do it
>>> from the start, not after the fact. Probably there's bindings that get
>>> acked every day that do do this. Even the retcon is okay to do when
>>> reg-names is mandated by the binding and the users use reg-names in my
>>> opinion.
>>
>> I think from the above analyses, since the usage only starts in rc1, we have
>> room for improving it knowing we aren't creating problem for anyone. Note that I
>> have no idea what the syntax is to "do this", and I doubt either Detlev or
>> Cristian have a clue.
> 
> I think this is the only bit that really still needs a reply, this can
> be solved by adding reg-names as "required" to the existing conditional
> portion of the binding. There's probably hundreds of examples if one
> does a search for "then:\n.*required:" to use a basis for the change
> here. Probably should be an independent change, since it is needed even
> without the re-order given the bug I brought up.

As mentioned in my previous reply, the actual problem is that the binding has
been already released, and I'm not sure we can change this without breaking the
ABI.

Regards,
Cristian

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 8168 bytes --]

On Fri, Feb 27, 2026 at 07:49:33PM +0200, Cristian Ciocaltea wrote:
> On 2/27/26 7:18 PM, Conor Dooley wrote:
> > On Thu, Feb 26, 2026 at 04:56:30PM -0500, Nicolas Dufresne wrote:
> >> Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
> >>> On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
> >>>> Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> > 
> >>>>> Deprecating the order also makes little sense to me, given that some of
> >>>>> these devices only have one reg entry, which as far as I can tell from
> >>>>> looking at the driver *is* the "function" region, so it can never be
> >>>>> entirely deprecated.
> >>>>
> >>>> What I'd like to see, is a binding expression that behave like a set, not a
> >>>> list, and leave the ordering open. As people keep repeating, there is nothing in
> >>>> a binding that assist to define the right ordering (its not address or base
> >>>> addres aware). That basically means, we can't as reviewer see that ordering is
> >>>> going to imposing using a base address in the unit name (which is a convenience,
> >>>> not a rule I suppose) that differ from the vendor documented base address.
> >>>>
> >>>> By explicitly removing the ordering in the binding, we create a strict rule that
> >>>> driver should retrieve this by name, and never assume the ordering, which I
> >>>> personally like.
> >>>>
> >>>> thoughts ?
> >>>
> >>> Yeah, you can do this, but to avoid potential breaks you have to do it
> >>> from the start, not after the fact. Probably there's bindings that get
> >>> acked every day that do do this. Even the retcon is okay to do when
> >>> reg-names is mandated by the binding and the users use reg-names in my
> >>> opinion.
> >>
> >> I think from the above analyses, since the usage only starts in rc1, we have
> >> room for improving it knowing we aren't creating problem for anyone. Note that I
> >> have no idea what the syntax is to "do this", and I doubt either Detlev or
> >> Cristian have a clue.
> > 
> > I think this is the only bit that really still needs a reply, this can
> > be solved by adding reg-names as "required" to the existing conditional
> > portion of the binding. There's probably hundreds of examples if one
> > does a search for "then:\n.*required:" to use a basis for the change
> > here. Probably should be an independent change, since it is needed even
> > without the re-order given the bug I brought up.
> 
> As mentioned in my previous reply, the actual problem is that the binding has
> been already released, and I'm not sure we can change this without breaking the
> ABI.

I feel like I am losing my mind here lol. Forget about this patch for a
moment and consider the driver right now. The code currently looks like
this:
	if (rkvdec->variant->has_single_reg_region) {
		rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
		if (IS_ERR(rkvdec->regs))
			return PTR_ERR(rkvdec->regs);
	} else {
		rkvdec->regs = devm_platform_ioremap_resource_byname(pdev, "function");
		if (IS_ERR(rkvdec->regs))
			return PTR_ERR(rkvdec->regs);

		rkvdec->link = devm_platform_ioremap_resource_byname(pdev, "link");
		if (IS_ERR(rkvdec->link))
			return PTR_ERR(rkvdec->link);
	}
This means you will fail to probe on any platform that does not have
has_single_reg_region set if the reg-names property is not present.
rk3588-vdec uses:
	static const struct rkvdec_variant vdpu381_variant = {
		.coded_fmts = vdpu381_coded_fmts,
		.num_coded_fmts = ARRAY_SIZE(vdpu381_coded_fmts),
		.rcb_sizes = vdpu381_rcb_sizes,
		.num_rcb_sizes = ARRAY_SIZE(vdpu381_rcb_sizes),
		.ops = &vdpu381_variant_ops,
	};
The binding does not currently require rk3588-vdec use reg-names. This
means that the driver will fail to probe if someone provides what the
binding considers to be a valid devicetree.

There are two ways to resolve this. The first is to do the following:
| diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
| index 967c452ab61f7..f508fc4746a87 100644
| --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
| +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
| @@ -1800,11 +1800,11 @@ static int rkvdec_probe(struct platform_device *pdev)
|  		if (IS_ERR(rkvdec->regs))
|  			return PTR_ERR(rkvdec->regs);
|  	} else {
| -		rkvdec->regs = devm_platform_ioremap_resource_byname(pdev, "function");
| +		rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
|  		if (IS_ERR(rkvdec->regs))
|  			return PTR_ERR(rkvdec->regs);
|  
| -		rkvdec->link = devm_platform_ioremap_resource_byname(pdev, "link");
| +		rkvdec->link = devm_platform_ioremap_resource(pdev, 1);
|  		if (IS_ERR(rkvdec->link))
|  			return PTR_ERR(rkvdec->link);
|  	}

The second is to make the reg-names property mandatory.

The first way does not constitute an ABI break. The second way is an ABI
break, as you rightly point out.

Now thinking about your patch here. Without it, the following is a valid
vdec node on rk3588.
	video-codec@fdc38000 {
		compatible = "rockchip,rk3588-vdec";
		reg = <0x0 0xfdc38100 0x0 0x500>,
		      <0x0 0xfdc38000 0x0 0x100>,
		      <0x0 0xfdc38600 0x0 0x100>;
		interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH 0>;
		clocks = <&cru ACLK_RKVDEC0>, <&cru HCLK_RKVDEC0>, <&cru CLK_RKVDEC0_CA>,
			 <&cru CLK_RKVDEC0_CORE>, <&cru CLK_RKVDEC0_HEVC_CA>;
		clock-names = "axi", "ahb", "cabac", "core", "hevc_cabac";
		assigned-clocks = <&cru ACLK_RKVDEC0>, <&cru CLK_RKVDEC0_CORE>,
				  <&cru CLK_RKVDEC0_CA>, <&cru CLK_RKVDEC0_HEVC_CA>;
		assigned-clock-rates = <800000000>, <600000000>,
				       <600000000>, <1000000000>;
		iommus = <&vdec0_mmu>;
		power-domains = <&power RK3588_PD_RKVDEC0>;
		resets = <&cru SRST_A_RKVDEC0>, <&cru SRST_H_RKVDEC0>, <&cru SRST_RKVDEC0_CA>,
			 <&cru SRST_RKVDEC0_CORE>, <&cru SRST_RKVDEC0_HEVC_CA>;
		reset-names = "axi", "ahb", "cabac", "core", "hevc_cabac";
		sram = <&vdec0_sram>;
	};
Ignore the fact that this will not currently probe for a moment, assuming
we applied my diff above to the driver, and look at what will be a valid
node after your patch:
	video-codec@fdc38000 {
		compatible = "rockchip,rk3588-vdec";
		reg = <0x0 0xfdc38000 0x0 0x100>,
		      <0x0 0xfdc38100 0x0 0x500>,
		      <0x0 0xfdc38600 0x0 0x100>;
		interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH 0>;
		clocks = <&cru ACLK_RKVDEC0>, <&cru HCLK_RKVDEC0>, <&cru CLK_RKVDEC0_CA>,
			 <&cru CLK_RKVDEC0_CORE>, <&cru CLK_RKVDEC0_HEVC_CA>;
		clock-names = "axi", "ahb", "cabac", "core", "hevc_cabac";
		assigned-clocks = <&cru ACLK_RKVDEC0>, <&cru CLK_RKVDEC0_CORE>,
				  <&cru CLK_RKVDEC0_CA>, <&cru CLK_RKVDEC0_HEVC_CA>;
		assigned-clock-rates = <800000000>, <600000000>,
				       <600000000>, <1000000000>;
		iommus = <&vdec0_mmu>;
		power-domains = <&power RK3588_PD_RKVDEC0>;
		resets = <&cru SRST_A_RKVDEC0>, <&cru SRST_H_RKVDEC0>, <&cru SRST_RKVDEC0_CA>,
			 <&cru SRST_RKVDEC0_CORE>, <&cru SRST_RKVDEC0_HEVC_CA>;
		reset-names = "axi", "ahb", "cabac", "core", "hevc_cabac";
		sram = <&vdec0_sram>;
	};
Driver is going to break here, cos it will pick up the link region and
set rkvdec->regs to it!

So, the only way to accommodate the deprecated scheme and the new scheme
is to use the reg-names property (as the driver currently does).

So yes, while what I propose is an ABI break, the driver currently
expects reg-names to be mandatory for the rk3588-vdec. Additionally, new
required properties are only really a meaningful ABI break if the driver
is changed to required them, since that would render old devicetrees
non-functional. The driver in question already requires them, so that's
pretty moot! Were it not for your patch here, I would say that my diff
should be applied to the driver instead of making the property required.

reg-names is a dependency for your ABI-annihilation, as my example above
demonstrates, so I find it really bemusing that you're worried about its
impact!

Hopefully I've made my point about reg-names being mandatory this time
around?

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Cristian Ciocaltea]

On 2/27/26 8:10 PM, Conor Dooley wrote:
> On Fri, Feb 27, 2026 at 07:49:33PM +0200, Cristian Ciocaltea wrote:
>> On 2/27/26 7:18 PM, Conor Dooley wrote:
>>> On Thu, Feb 26, 2026 at 04:56:30PM -0500, Nicolas Dufresne wrote:
>>>> Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
>>>>> On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
>>>>>> Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
>>>
>>>>>>> Deprecating the order also makes little sense to me, given that some of
>>>>>>> these devices only have one reg entry, which as far as I can tell from
>>>>>>> looking at the driver *is* the "function" region, so it can never be
>>>>>>> entirely deprecated.
>>>>>>
>>>>>> What I'd like to see, is a binding expression that behave like a set, not a
>>>>>> list, and leave the ordering open. As people keep repeating, there is nothing in
>>>>>> a binding that assist to define the right ordering (its not address or base
>>>>>> addres aware). That basically means, we can't as reviewer see that ordering is
>>>>>> going to imposing using a base address in the unit name (which is a convenience,
>>>>>> not a rule I suppose) that differ from the vendor documented base address.
>>>>>>
>>>>>> By explicitly removing the ordering in the binding, we create a strict rule that
>>>>>> driver should retrieve this by name, and never assume the ordering, which I
>>>>>> personally like.
>>>>>>
>>>>>> thoughts ?
>>>>>
>>>>> Yeah, you can do this, but to avoid potential breaks you have to do it
>>>>> from the start, not after the fact. Probably there's bindings that get
>>>>> acked every day that do do this. Even the retcon is okay to do when
>>>>> reg-names is mandated by the binding and the users use reg-names in my
>>>>> opinion.
>>>>
>>>> I think from the above analyses, since the usage only starts in rc1, we have
>>>> room for improving it knowing we aren't creating problem for anyone. Note that I
>>>> have no idea what the syntax is to "do this", and I doubt either Detlev or
>>>> Cristian have a clue.
>>>
>>> I think this is the only bit that really still needs a reply, this can
>>> be solved by adding reg-names as "required" to the existing conditional
>>> portion of the binding. There's probably hundreds of examples if one
>>> does a search for "then:\n.*required:" to use a basis for the change
>>> here. Probably should be an independent change, since it is needed even
>>> without the re-order given the bug I brought up.
>>
>> As mentioned in my previous reply, the actual problem is that the binding has
>> been already released, and I'm not sure we can change this without breaking the
>> ABI.
 
[...]

> So yes, while what I propose is an ABI break, the driver currently
> expects reg-names to be mandatory for the rk3588-vdec. Additionally, new
> required properties are only really a meaningful ABI break if the driver
> is changed to required them, since that would render old devicetrees
> non-functional. The driver in question already requires them, so that's
> pretty moot! 

I think that's precisely the information I was looking for, i.e. breaking ABI
in this case is fully justified since we cannot really fix the driver.  I mean 
we would have time to do this, since the rk3588 related changes landed in
v7.0-rc1, but it's not feasible to accommodate to the current state of the
binding, as you clearly pointed out.

> Hopefully I've made my point about reg-names being mandatory this time
> around?

Totally, thanks for your time and sorry for the confusion around the topic!

My only question now is how should we proceed with this particular change - I'd
handle it in a dedicated patch, preceding this one.

Regards,
Cristian

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4156 bytes --]

On Fri, Feb 27, 2026 at 09:35:01PM +0200, Cristian Ciocaltea wrote:
> On 2/27/26 8:10 PM, Conor Dooley wrote:
> > On Fri, Feb 27, 2026 at 07:49:33PM +0200, Cristian Ciocaltea wrote:
> >> On 2/27/26 7:18 PM, Conor Dooley wrote:
> >>> On Thu, Feb 26, 2026 at 04:56:30PM -0500, Nicolas Dufresne wrote:
> >>>> Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
> >>>>> On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
> >>>>>> Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> >>>
> >>>>>>> Deprecating the order also makes little sense to me, given that some of
> >>>>>>> these devices only have one reg entry, which as far as I can tell from
> >>>>>>> looking at the driver *is* the "function" region, so it can never be
> >>>>>>> entirely deprecated.
> >>>>>>
> >>>>>> What I'd like to see, is a binding expression that behave like a set, not a
> >>>>>> list, and leave the ordering open. As people keep repeating, there is nothing in
> >>>>>> a binding that assist to define the right ordering (its not address or base
> >>>>>> addres aware). That basically means, we can't as reviewer see that ordering is
> >>>>>> going to imposing using a base address in the unit name (which is a convenience,
> >>>>>> not a rule I suppose) that differ from the vendor documented base address.
> >>>>>>
> >>>>>> By explicitly removing the ordering in the binding, we create a strict rule that
> >>>>>> driver should retrieve this by name, and never assume the ordering, which I
> >>>>>> personally like.
> >>>>>>
> >>>>>> thoughts ?
> >>>>>
> >>>>> Yeah, you can do this, but to avoid potential breaks you have to do it
> >>>>> from the start, not after the fact. Probably there's bindings that get
> >>>>> acked every day that do do this. Even the retcon is okay to do when
> >>>>> reg-names is mandated by the binding and the users use reg-names in my
> >>>>> opinion.
> >>>>
> >>>> I think from the above analyses, since the usage only starts in rc1, we have
> >>>> room for improving it knowing we aren't creating problem for anyone. Note that I
> >>>> have no idea what the syntax is to "do this", and I doubt either Detlev or
> >>>> Cristian have a clue.
> >>>
> >>> I think this is the only bit that really still needs a reply, this can
> >>> be solved by adding reg-names as "required" to the existing conditional
> >>> portion of the binding. There's probably hundreds of examples if one
> >>> does a search for "then:\n.*required:" to use a basis for the change
> >>> here. Probably should be an independent change, since it is needed even
> >>> without the re-order given the bug I brought up.
> >>
> >> As mentioned in my previous reply, the actual problem is that the binding has
> >> been already released, and I'm not sure we can change this without breaking the
> >> ABI.
>  
> [...]
> 
> > So yes, while what I propose is an ABI break, the driver currently
> > expects reg-names to be mandatory for the rk3588-vdec. Additionally, new
> > required properties are only really a meaningful ABI break if the driver
> > is changed to required them, since that would render old devicetrees
> > non-functional. The driver in question already requires them, so that's
> > pretty moot! 
> 
> I think that's precisely the information I was looking for, i.e. breaking ABI
> in this case is fully justified since we cannot really fix the driver.  I mean 
> we would have time to do this, since the rk3588 related changes landed in
> v7.0-rc1, but it's not feasible to accommodate to the current state of the
> binding, as you clearly pointed out.
> 
> > Hopefully I've made my point about reg-names being mandatory this time
> > around?
> 
> Totally, thanks for your time and sorry for the confusion around the topic!

No worries. I as kinda wondering if I had someone gone nuts and
misunderstaood what the driver was doing!

> My only question now is how should we proceed with this particular change - I'd
> handle it in a dedicated patch, preceding this one.

I would do it in a dedicated patch yeah, since it is independently
justifiable.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 3811 bytes --]

Le vendredi 27 février 2026 à 14:03 +0100, Krzysztof Kozlowski a écrit :
> On 27/02/2026 12:37, Cristian Ciocaltea wrote:
> > Hi Krzysztof, Conor,
> > 
> > On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
> > > On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
> > > > With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
> > > > register blocks have been provided for the video decoder unit.
> > > > 
> > > > However, the binding does not properly describe the new hardware layout,
> > > 
> > > As you shown me last time with excerpt of address spaces from
> > > datasheet/manual, the binding correctly describes the hardware and above
> > > sentence is not true.
> > > 
> > > > as it breaks the convention expecting the unit address to indicate the
> > > > start of the first register range, i.e. 'function' block is listed
> > > 
> > > Imprecise wording. "start of the main or primary register range"
> > > 
> > > (if you have 0x1000 with one reg and 0x20000000 with everything, the
> > > unit address will be 0x20000000).
> > > 
> > > > before 'link' instead of the opposite.
> > > > 
> > > > Since the binding changes have been already released and a fix would
> > > > bring up an ABI break, mark the current 'reg-names' ordering as
> > > > deprecated and introduce an alternative 'link,function,cache' listing
> > > > which follows the address-based ordering according to the TRM.
> > > > 
> > > > Additionally, drop the 'reg' description items as the order is not fixed
> > > > anymore, while the information they offer is not very relevant anyway.
> > > 
> > > This is fine for me.
> > 
> > Thanks for the additional feedback!
> > 
> > If I'm not mistaken (please correct me), the only remaining (hard)
> > blocker for the series would be to improve this commit message.
> > 
> > How about the following:
> > 
> >     With the introduction of the RK3588 SoC, and RK3576 afterwards, three
> >     register blocks have been provided for the video decoder unit instead of
> >     just one, which are further referenced in the datasheet by 'link table',
> >     'function' and 'cache'.  The former is present at the top of the
> >     listing, starting at video decoder unit base address.
> > 
> >     However, while documenting RK3588, the binding broke the convention
> >     expecting the unit address to indicate the start of the primary register
> >     range, i.e. the 'function' block got listed before the 'link' one.
> > 
> >     Since the binding changes have been already released and a fix would
> >     bring up an ABI break, mark the current 'reg-names' ordering as
> >     deprecated and introduce an alternative 'link,function,cache' listing

I would suggest something around:

		   and introduce the actual hardware order "link, function,   
 
                   cache" ...

The rationale is that there is no alternative, if you use the deprecated order,
a hypothetical driver maybe interpret the offsets wrong. There is no possible
backward compatibility with a new order, and you can't have two possible order
in this syntax.

Though, the rational to break ABI and backward compatibility is something we all
agreed on.

> >     which follows the address-based ordering according to the TRM.
> > 
> >     Additionally, drop the 'reg' description items as the order is not fixed
> >     anymore, while the information they offer is not very relevant anyway.
> 
> Yes, it's fine. My comments were actually not blocking, I just wanted to
> wait if discussion with Conor resolves somehow.
> 
> But for me anyway:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> 
> 
> Best regards,
> Krzysztof

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Krzysztof Kozlowski]

On 27/02/2026 18:42, Cristian Ciocaltea wrote:
> On 2/27/26 7:13 PM, Conor Dooley wrote:
>> On Fri, Feb 27, 2026 at 01:37:17PM +0200, Cristian Ciocaltea wrote:
>>> Hi Krzysztof, Conor,
>>>
>>> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
>>>> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>>>>> register blocks have been provided for the video decoder unit.
>>>>>
>>>>> However, the binding does not properly describe the new hardware layout,
>>>>
>>>> As you shown me last time with excerpt of address spaces from
>>>> datasheet/manual, the binding correctly describes the hardware and above
>>>> sentence is not true.
>>>>
>>>>> as it breaks the convention expecting the unit address to indicate the
>>>>> start of the first register range, i.e. 'function' block is listed
>>>>
>>>> Imprecise wording. "start of the main or primary register range"
>>>>
>>>> (if you have 0x1000 with one reg and 0x20000000 with everything, the
>>>> unit address will be 0x20000000).
>>>>
>>>>> before 'link' instead of the opposite.
>>>>>
>>>>> Since the binding changes have been already released and a fix would
>>>>> bring up an ABI break, mark the current 'reg-names' ordering as
>>>>> deprecated and introduce an alternative 'link,function,cache' listing
>>>>> which follows the address-based ordering according to the TRM.
>>>>>
>>>>> Additionally, drop the 'reg' description items as the order is not fixed
>>>>> anymore, while the information they offer is not very relevant anyway.
>>>>
>>>> This is fine for me.
>>>
>>> Thanks for the additional feedback!
>>>
>>> If I'm not mistaken (please correct me), the only remaining (hard)
>>> blocker for the series would be to improve this commit message.
>>
>> No, you also need to fix the problem I pointed out about reg-names being
>> optional on the devices you're relying on reg-names for. 
> 
> My only concern is that by marking reg-names as required we would break the ABI,

You are ALREADY BREAKING the ABI. Really, for absolutely non-important
cosmetic change in unit address, where I asked you repeatedly to fix the
unit address, you change the ABI affecting kernel and DTS users.

This is barely acceptable, but I am just annoyed already explain it to
you multiple times.

But now you claim, you can break ABI for cosmetic unimportant change,
but actually doing something meaningful is a no-go?

At least use correct arguments if you want to discuss.

> since the RK3588 related changes in the binding (not the DTS ones) got already
> released (i.e. since v6.17). That's also the reason we went with this deprecated
> order approach.
> 
>> The new commit
>> message I am happy with, provided you also add the information Nicolas
>> provided about the impact on users.
> 
> Nicolas, can you please provide here the statement so that we can agree on the
> wording?
> 
> Thanks,
> Cristian
> 
>>
>>>
>>> How about the following:
>>>
>>>     With the introduction of the RK3588 SoC, and RK3576 afterwards, three
>>>     register blocks have been provided for the video decoder unit instead of
>>>     just one, which are further referenced in the datasheet by 'link table',
>>>     'function' and 'cache'.  The former is present at the top of the
>>>     listing, starting at video decoder unit base address.
>>>
>>>     However, while documenting RK3588, the binding broke the convention
>>>     expecting the unit address to indicate the start of the primary register
>>>     range, i.e. the 'function' block got listed before the 'link' one.
>>>
>>>     Since the binding changes have been already released and a fix would
>>>     bring up an ABI break, mark the current 'reg-names' ordering as
>>>     deprecated and introduce an alternative 'link,function,cache' listing
>>>     which follows the address-based ordering according to the TRM.
>>>
>>>     Additionally, drop the 'reg' description items as the order is not fixed
>>>     anymore, while the information they offer is not very relevant anyway.
>>>
>>> Regards,
>>> Cristian
> 


Best regards,
Krzysztof

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Krzysztof Kozlowski]

On 28/02/2026 10:54, Krzysztof Kozlowski wrote:
> On 27/02/2026 18:42, Cristian Ciocaltea wrote:
>> On 2/27/26 7:13 PM, Conor Dooley wrote:
>>> On Fri, Feb 27, 2026 at 01:37:17PM +0200, Cristian Ciocaltea wrote:
>>>> Hi Krzysztof, Conor,
>>>>
>>>> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
>>>>> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>>>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>>>>>> register blocks have been provided for the video decoder unit.
>>>>>>
>>>>>> However, the binding does not properly describe the new hardware layout,
>>>>>
>>>>> As you shown me last time with excerpt of address spaces from
>>>>> datasheet/manual, the binding correctly describes the hardware and above
>>>>> sentence is not true.
>>>>>
>>>>>> as it breaks the convention expecting the unit address to indicate the
>>>>>> start of the first register range, i.e. 'function' block is listed
>>>>>
>>>>> Imprecise wording. "start of the main or primary register range"
>>>>>
>>>>> (if you have 0x1000 with one reg and 0x20000000 with everything, the
>>>>> unit address will be 0x20000000).
>>>>>
>>>>>> before 'link' instead of the opposite.
>>>>>>
>>>>>> Since the binding changes have been already released and a fix would
>>>>>> bring up an ABI break, mark the current 'reg-names' ordering as
>>>>>> deprecated and introduce an alternative 'link,function,cache' listing
>>>>>> which follows the address-based ordering according to the TRM.
>>>>>>
>>>>>> Additionally, drop the 'reg' description items as the order is not fixed
>>>>>> anymore, while the information they offer is not very relevant anyway.
>>>>>
>>>>> This is fine for me.
>>>>
>>>> Thanks for the additional feedback!
>>>>
>>>> If I'm not mistaken (please correct me), the only remaining (hard)
>>>> blocker for the series would be to improve this commit message.
>>>
>>> No, you also need to fix the problem I pointed out about reg-names being
>>> optional on the devices you're relying on reg-names for. 
>>
>> My only concern is that by marking reg-names as required we would break the ABI,
> 
> You are ALREADY BREAKING the ABI. Really, for absolutely non-important
> cosmetic change in unit address, where I asked you repeatedly to fix the
> unit address, you change the ABI affecting kernel and DTS users.
> 
> This is barely acceptable, but I am just annoyed already explain it to
> you multiple times.
> 
> But now you claim, you can break ABI for cosmetic unimportant change,
> but actually doing something meaningful is a no-go?
> 
> At least use correct arguments if you want to discuss.

And I double checked now with Conor - your binding and drivers are
broken here and THIS you must fix. Not the unit address you are so
focused about.

Best regards,
Krzysztof

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Cristian Ciocaltea]

Hi Krzysztof,

On 2/28/26 11:58 AM, Krzysztof Kozlowski wrote:
> On 28/02/2026 10:54, Krzysztof Kozlowski wrote:
>> On 27/02/2026 18:42, Cristian Ciocaltea wrote:
>>> On 2/27/26 7:13 PM, Conor Dooley wrote:
>>>> On Fri, Feb 27, 2026 at 01:37:17PM +0200, Cristian Ciocaltea wrote:
>>>>> Hi Krzysztof, Conor,
>>>>>
>>>>> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
>>>>>> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>>>>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>>>>>>> register blocks have been provided for the video decoder unit.
>>>>>>>
>>>>>>> However, the binding does not properly describe the new hardware layout,
>>>>>>
>>>>>> As you shown me last time with excerpt of address spaces from
>>>>>> datasheet/manual, the binding correctly describes the hardware and above
>>>>>> sentence is not true.
>>>>>>
>>>>>>> as it breaks the convention expecting the unit address to indicate the
>>>>>>> start of the first register range, i.e. 'function' block is listed
>>>>>>
>>>>>> Imprecise wording. "start of the main or primary register range"
>>>>>>
>>>>>> (if you have 0x1000 with one reg and 0x20000000 with everything, the
>>>>>> unit address will be 0x20000000).
>>>>>>
>>>>>>> before 'link' instead of the opposite.
>>>>>>>
>>>>>>> Since the binding changes have been already released and a fix would
>>>>>>> bring up an ABI break, mark the current 'reg-names' ordering as
>>>>>>> deprecated and introduce an alternative 'link,function,cache' listing
>>>>>>> which follows the address-based ordering according to the TRM.
>>>>>>>
>>>>>>> Additionally, drop the 'reg' description items as the order is not fixed
>>>>>>> anymore, while the information they offer is not very relevant anyway.
>>>>>>
>>>>>> This is fine for me.
>>>>>
>>>>> Thanks for the additional feedback!
>>>>>
>>>>> If I'm not mistaken (please correct me), the only remaining (hard)
>>>>> blocker for the series would be to improve this commit message.
>>>>
>>>> No, you also need to fix the problem I pointed out about reg-names being
>>>> optional on the devices you're relying on reg-names for. 
>>>
>>> My only concern is that by marking reg-names as required we would break the ABI,
>>
>> You are ALREADY BREAKING the ABI. Really, for absolutely non-important
>> cosmetic change in unit address, where I asked you repeatedly to fix the
>> unit address, you change the ABI affecting kernel and DTS users.

I thought we've already reached consensus to allow extending the binding and
keep both lists, precisely to avoid breaking the ABI.  At least this was my
understanding according to your reply [1]:

  You can have also oneOf with older list "deprecated: true", if want to
  keep any users unaffected.

And this patch was meant to do exactly that.  Did I miss something?

>> This is barely acceptable, but I am just annoyed already explain it to
>> you multiple times.

There is no need to explain it again, we've got your point.  We've also brought our
arguments and I had the impression that we eventually agreed to keep the unit
address unchanged, based on your comments [2]:

  Yes, with drop of the oneOf this would be fine.
  I meant, the "one item option" in oneOf.

Is this not applicable anymore?

>> But now you claim, you can break ABI for cosmetic unimportant change,

No, breaking ABI wasn't our intention here.  If we put the issue with reg-names
being optional aside for a moment (as that one will be handled separately), is
there still a problem with the current revision?

>> but actually doing something meaningful is a no-go?

Making reg-names mandatory has been already clarified with Conor and agreed [3]
to be handled in a dedicated patch.  And that one will indeed break the ABI, but
it's unavoidable, unfortunately. 

>> At least use correct arguments if you want to discuss.

Sorry, I'm not sure what do you mean.  I really believed that we managed to
address all the open topics by now.

Thanks,
Cristian

[1] https://lore.kernel.org/all/1cdc36f2-6e51-492a-9063-7d0a784f5118@kernel.org/
[2] https://lore.kernel.org/all/12b30229-1c55-429d-8a3c-0d831c4d33ab@kernel.org/
[3] https://lore.kernel.org/all/20260227-urologist-gratitude-7984733f2d41@spud/

Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Cristian Ciocaltea]

On 3/3/26 2:26 AM, Cristian Ciocaltea wrote:
> Hi Krzysztof,
> 
> On 2/28/26 11:58 AM, Krzysztof Kozlowski wrote:
>> On 28/02/2026 10:54, Krzysztof Kozlowski wrote:
>>> On 27/02/2026 18:42, Cristian Ciocaltea wrote:
>>>> On 2/27/26 7:13 PM, Conor Dooley wrote:
>>>>> On Fri, Feb 27, 2026 at 01:37:17PM +0200, Cristian Ciocaltea wrote:
>>>>>> Hi Krzysztof, Conor,
>>>>>>
>>>>>> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
>>>>>>> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>>>>>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>>>>>>>> register blocks have been provided for the video decoder unit.
>>>>>>>>
>>>>>>>> However, the binding does not properly describe the new hardware layout,
>>>>>>>
>>>>>>> As you shown me last time with excerpt of address spaces from
>>>>>>> datasheet/manual, the binding correctly describes the hardware and above
>>>>>>> sentence is not true.
>>>>>>>
>>>>>>>> as it breaks the convention expecting the unit address to indicate the
>>>>>>>> start of the first register range, i.e. 'function' block is listed
>>>>>>>
>>>>>>> Imprecise wording. "start of the main or primary register range"
>>>>>>>
>>>>>>> (if you have 0x1000 with one reg and 0x20000000 with everything, the
>>>>>>> unit address will be 0x20000000).
>>>>>>>
>>>>>>>> before 'link' instead of the opposite.
>>>>>>>>
>>>>>>>> Since the binding changes have been already released and a fix would
>>>>>>>> bring up an ABI break, mark the current 'reg-names' ordering as
>>>>>>>> deprecated and introduce an alternative 'link,function,cache' listing
>>>>>>>> which follows the address-based ordering according to the TRM.
>>>>>>>>
>>>>>>>> Additionally, drop the 'reg' description items as the order is not fixed
>>>>>>>> anymore, while the information they offer is not very relevant anyway.
>>>>>>>
>>>>>>> This is fine for me.
>>>>>>
>>>>>> Thanks for the additional feedback!
>>>>>>
>>>>>> If I'm not mistaken (please correct me), the only remaining (hard)
>>>>>> blocker for the series would be to improve this commit message.
>>>>>
>>>>> No, you also need to fix the problem I pointed out about reg-names being
>>>>> optional on the devices you're relying on reg-names for. 
>>>>
>>>> My only concern is that by marking reg-names as required we would break the ABI,
>>>
>>> You are ALREADY BREAKING the ABI. Really, for absolutely non-important
>>> cosmetic change in unit address, where I asked you repeatedly to fix the
>>> unit address, you change the ABI affecting kernel and DTS users.
> 
> I thought we've already reached consensus to allow extending the binding and
> keep both lists, precisely to avoid breaking the ABI.  At least this was my
> understanding according to your reply [1]:
> 
>   You can have also oneOf with older list "deprecated: true", if want to
>   keep any users unaffected.
> 
> And this patch was meant to do exactly that.  Did I miss something?
> 
>>> This is barely acceptable, but I am just annoyed already explain it to
>>> you multiple times.
> 
> There is no need to explain it again, we've got your point.  We've also brought our
> arguments and I had the impression that we eventually agreed to keep the unit
> address unchanged, based on your comments [2]:
> 
>   Yes, with drop of the oneOf this would be fine.
>   I meant, the "one item option" in oneOf.
> 
> Is this not applicable anymore?
> 
>>> But now you claim, you can break ABI for cosmetic unimportant change,
> 
> No, breaking ABI wasn't our intention here.  If we put the issue with reg-names
> being optional aside for a moment (as that one will be handled separately), is
> there still a problem with the current revision?
> 
>>> but actually doing something meaningful is a no-go?
> 
> Making reg-names mandatory has been already clarified with Conor and agreed [3]
> to be handled in a dedicated patch.  And that one will indeed break the ABI, but
> it's unavoidable, unfortunately. 
> 
>>> At least use correct arguments if you want to discuss.
> 
> Sorry, I'm not sure what do you mean.  I really believed that we managed to
> address all the open topics by now.

I've just submitted v5.  For some reason the link to the cover letter [1]
doesn't seem to work, I'm getting:

  Message-ID <20260304-vdec-reg-order-rk3576-v5-0-7006fad42c3a@collabora.com>
  not found

But all the others are just fine, e.g. [2] is the for the 1st patch.  I've never
encountered something similar before.

Regards,
Cristian

[1] https://lore.kernel.org/all/20260304-vdec-reg-order-rk3576-v5-0-7006fad42c3a@collabora.com/
[2] https://lore.kernel.org/all/20260304-vdec-reg-order-rk3576-v5-1-7006fad42c3a@collabora.com/