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

[PATCH v3 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 unit address points to the first register
range.

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

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@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      | 19 ++++++++++++-------
 arch/arm64/boot/dts/rockchip/rk3576.dtsi              |  6 +++---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi         | 12 ++++++------
 3 files changed, 21 insertions(+), 16 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260223-vdec-reg-order-rk3576-cc2ec6e05e98

[PATCH v3 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, the
'link' and 'cache' register blocks have been provided for the video
decoder unit in addition to the existing 'function' one, which now shows
up in between them (from address-based ordering point of view).

However, the binding does not properly describe this hardware layout, as
the new blocks are listed after the old one.  Therefore it breaks the
convention expecting the unit address to indicate the first register
range.

Since the binding changes have been already released and a fix would
bring up an ABI break, mark the current 'reg-names' listing as
deprecated and introduce an alternative 'link,function,cache' one.

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      | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
index 809fda45b3bd..3f6072e8baa5 100644
--- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
@@ -28,16 +28,21 @@ 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:
+    oneOf:
       - const: function
-      - const: link
-      - const: cache
+      - 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 v3 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 v3 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 v3 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}

[Krzysztof Kozlowski]

On 25/02/2026 13:19, Cristian Ciocaltea wrote:
> With the introduction of the RK3588 SoC, and RK3576 afterwards, the
> 'link' and 'cache' register blocks have been provided for the video
> decoder unit in addition to the existing 'function' one, which now shows
> up in between them (from address-based ordering point of view).
> 
> However, the binding does not properly describe this hardware layout, as
> the new blocks are listed after the old one.  Therefore it breaks the
> convention expecting the unit address to indicate the first register
> range.
> 
> Since the binding changes have been already released and a fix would
> bring up an ABI break, mark the current 'reg-names' listing as
> deprecated and introduce an alternative 'link,function,cache' one.
> 
> 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      | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> index 809fda45b3bd..3f6072e8baa5 100644
> --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> @@ -28,16 +28,21 @@ 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:
> +    oneOf:
>        - const: function

This is confusing, I think I missed that in previous patch because it
did not leave that part or I misread the diff hunk - why do you allow
one entry?

If the first entry is function, then all others MUST built on top, thus
this:

> +          - const: link
> +          - const: function
> +          - const: cache

is not correct.

No, you don't change the orders. So again, if you have such binding,
then you just fix the unit address leaving the binding as is.


> -      - const: link
> -      - const: cache
> +      - items:
> +          - const: link
> +          - const: function
> +          - const: cache
> +      - items:
> +          - const: function
> +          - const: link
> +          - const: cache
> +        deprecated: true
> +        description: Use link,function,cache block order instead.
>  
Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 25/02/2026 13:26, Krzysztof Kozlowski wrote:
> On 25/02/2026 13:19, Cristian Ciocaltea wrote:
>> With the introduction of the RK3588 SoC, and RK3576 afterwards, the
>> 'link' and 'cache' register blocks have been provided for the video
>> decoder unit in addition to the existing 'function' one, which now shows
>> up in between them (from address-based ordering point of view).
>>
>> However, the binding does not properly describe this hardware layout, as
>> the new blocks are listed after the old one.  Therefore it breaks the
>> convention expecting the unit address to indicate the first register
>> range.
>>
>> Since the binding changes have been already released and a fix would
>> bring up an ABI break, mark the current 'reg-names' listing as
>> deprecated and introduce an alternative 'link,function,cache' one.
>>
>> 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      | 19 ++++++++++++-------
>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>> index 809fda45b3bd..3f6072e8baa5 100644
>> --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>> +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>> @@ -28,16 +28,21 @@ 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:
>> +    oneOf:
>>        - const: function
> 
> This is confusing, I think I missed that in previous patch because it
> did not leave that part or I misread the diff hunk - why do you allow
> one entry?
> 
> If the first entry is function, then all others MUST built on top, thus
> this:
> 
>> +          - const: link
>> +          - const: function
>> +          - const: cache
> 
> is not correct.
> 
> No, you don't change the orders. So again, if you have such binding,
> then you just fix the unit address leaving the binding as is.
> 

I just now checked v2 - it had this first "function" left, so I missed
that part. I would give different review at v2, so you would not waste
time. Apologies for that.

Best regards,
Krzysztof

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

[Cristian Ciocaltea]

On 2/25/26 2:26 PM, Krzysztof Kozlowski wrote:
> On 25/02/2026 13:19, Cristian Ciocaltea wrote:
>> With the introduction of the RK3588 SoC, and RK3576 afterwards, the
>> 'link' and 'cache' register blocks have been provided for the video
>> decoder unit in addition to the existing 'function' one, which now shows
>> up in between them (from address-based ordering point of view).
>>
>> However, the binding does not properly describe this hardware layout, as
>> the new blocks are listed after the old one.  Therefore it breaks the
>> convention expecting the unit address to indicate the first register
>> range.
>>
>> Since the binding changes have been already released and a fix would
>> bring up an ABI break, mark the current 'reg-names' listing as
>> deprecated and introduce an alternative 'link,function,cache' one.
>>
>> 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      | 19 ++++++++++++-------
>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>> index 809fda45b3bd..3f6072e8baa5 100644
>> --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>> +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>> @@ -28,16 +28,21 @@ 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:
>> +    oneOf:
>>        - const: function
> 
> This is confusing, I think I missed that in previous patch because it
> did not leave that part or I misread the diff hunk - why do you allow
> one entry?

That's for the older SoCs, e.g. RK3288, RK3399, as the 'link' and 'cache' blocks
are only available for RK3576 & RK3588.

> 
> If the first entry is function, then all others MUST built on top, thus
> this:
> 
>> +          - const: link
>> +          - const: function
>> +          - const: cache
> 
> is not correct.
> 
> No, you don't change the orders. So again, if you have such binding,
> then you just fix the unit address leaving the binding as is.

Changing the unit address would mean it will point inside the register range,
rather than at the beginning of it.

Sorry, but I don't quite get why would this be a better approach than just
properly list the items according to the HW layout, i.e. following the
address-based ordering?

Thanks,
Cristian

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

[Krzysztof Kozlowski]

On 25/02/2026 14:36, Cristian Ciocaltea wrote:
> On 2/25/26 2:26 PM, Krzysztof Kozlowski wrote:
>> On 25/02/2026 13:19, Cristian Ciocaltea wrote:
>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, the
>>> 'link' and 'cache' register blocks have been provided for the video
>>> decoder unit in addition to the existing 'function' one, which now shows
>>> up in between them (from address-based ordering point of view).
>>>
>>> However, the binding does not properly describe this hardware layout, as
>>> the new blocks are listed after the old one.  Therefore it breaks the
>>> convention expecting the unit address to indicate the first register
>>> range.
>>>
>>> Since the binding changes have been already released and a fix would
>>> bring up an ABI break, mark the current 'reg-names' listing as
>>> deprecated and introduce an alternative 'link,function,cache' one.
>>>
>>> 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      | 19 ++++++++++++-------
>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>> index 809fda45b3bd..3f6072e8baa5 100644
>>> --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>> +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>> @@ -28,16 +28,21 @@ 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:
>>> +    oneOf:
>>>        - const: function
>>
>> This is confusing, I think I missed that in previous patch because it
>> did not leave that part or I misread the diff hunk - why do you allow
>> one entry?
> 
> That's for the older SoCs, e.g. RK3288, RK3399, as the 'link' and 'cache' blocks
> are only available for RK3576 & RK3588.

Yeah, I see in the bottom of the binding

> 
>>
>> If the first entry is function, then all others MUST built on top, thus
>> this:
>>
>>> +          - const: link
>>> +          - const: function
>>> +          - const: cache
>>
>> is not correct.
>>
>> No, you don't change the orders. So again, if you have such binding,
>> then you just fix the unit address leaving the binding as is.
> 
> Changing the unit address would mean it will point inside the register range,
> rather than at the beginning of it.

First, not true. If this was one register range, you would have one
entry. You cannot split entries. Split entries means you have two
SEPARATE register ranges.
Second, it does not matter because main rule stays - the unit address
describes the main address space. The main address space in other device
was called "function", so I assume the main address space is also here
the "function". Which makes sense, because link feels secondary to
functioning of the device.

> 
> Sorry, but I don't quite get why would this be a better approach than just
> properly list the items according to the HW layout, i.e. following the
> address-based ordering?

We always expect the list to grow, to have common set. That's rule given
during reviews multiple times. For multiple reasons, also explained
(consistency, maintenance and actually proper description of hardware
like the main reg address space).

Probably this was also given to that binding during discussions when it
was upstream, so your change reverts previous discussion and to that I
do not agree.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 25/02/2026 15:11, Krzysztof Kozlowski wrote:
> On 25/02/2026 14:36, Cristian Ciocaltea wrote:
>> On 2/25/26 2:26 PM, Krzysztof Kozlowski wrote:
>>> On 25/02/2026 13:19, Cristian Ciocaltea wrote:
>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, the
>>>> 'link' and 'cache' register blocks have been provided for the video
>>>> decoder unit in addition to the existing 'function' one, which now shows
>>>> up in between them (from address-based ordering point of view).
>>>>
>>>> However, the binding does not properly describe this hardware layout, as
>>>> the new blocks are listed after the old one.  Therefore it breaks the
>>>> convention expecting the unit address to indicate the first register
>>>> range.
>>>>
>>>> Since the binding changes have been already released and a fix would
>>>> bring up an ABI break, mark the current 'reg-names' listing as
>>>> deprecated and introduce an alternative 'link,function,cache' one.
>>>>
>>>> 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      | 19 ++++++++++++-------
>>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>>> index 809fda45b3bd..3f6072e8baa5 100644
>>>> --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>>> @@ -28,16 +28,21 @@ 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:
>>>> +    oneOf:
>>>>        - const: function
>>>
>>> This is confusing, I think I missed that in previous patch because it
>>> did not leave that part or I misread the diff hunk - why do you allow
>>> one entry?
>>
>> That's for the older SoCs, e.g. RK3288, RK3399, as the 'link' and 'cache' blocks
>> are only available for RK3576 & RK3588.
> 
> Yeah, I see in the bottom of the binding
> 
>>
>>>
>>> If the first entry is function, then all others MUST built on top, thus
>>> this:
>>>
>>>> +          - const: link
>>>> +          - const: function
>>>> +          - const: cache
>>>
>>> is not correct.
>>>
>>> No, you don't change the orders. So again, if you have such binding,
>>> then you just fix the unit address leaving the binding as is.
>>
>> Changing the unit address would mean it will point inside the register range,
>> rather than at the beginning of it.
> 
> First, not true. If this was one register range, you would have one
> entry. You cannot split entries. Split entries means you have two
> SEPARATE register ranges.
> Second, it does not matter because main rule stays - the unit address
> describes the main address space. The main address space in other device
> was called "function", so I assume the main address space is also here
> the "function". Which makes sense, because link feels secondary to
> functioning of the device.
> 
>>
>> Sorry, but I don't quite get why would this be a better approach than just
>> properly list the items according to the HW layout, i.e. following the
>> address-based ordering?

One more comment here, actually repeating Rob, because this awfully
reminds me discussions with Linaro folks around media/iris/camss where
they came with terrible idea of sorting everything by value
(reg+clocks+interrupts) - because binding does not know anything about
addresses, thus you cannot sort/order this list in the binding by
address. There is simple no "key" to sort with.

Best regards,
Krzysztof

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

[Cristian Ciocaltea]

On 2/25/26 4:11 PM, Krzysztof Kozlowski wrote:
> On 25/02/2026 14:36, Cristian Ciocaltea wrote:
>> On 2/25/26 2:26 PM, Krzysztof Kozlowski wrote:
>>> On 25/02/2026 13:19, Cristian Ciocaltea wrote:
>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, the
>>>> 'link' and 'cache' register blocks have been provided for the video
>>>> decoder unit in addition to the existing 'function' one, which now shows
>>>> up in between them (from address-based ordering point of view).
>>>>
>>>> However, the binding does not properly describe this hardware layout, as
>>>> the new blocks are listed after the old one.  Therefore it breaks the
>>>> convention expecting the unit address to indicate the first register
>>>> range.
>>>>
>>>> Since the binding changes have been already released and a fix would
>>>> bring up an ABI break, mark the current 'reg-names' listing as
>>>> deprecated and introduce an alternative 'link,function,cache' one.
>>>>
>>>> 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      | 19 ++++++++++++-------
>>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>>> index 809fda45b3bd..3f6072e8baa5 100644
>>>> --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
>>>> @@ -28,16 +28,21 @@ 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:
>>>> +    oneOf:
>>>>        - const: function
>>>
>>> This is confusing, I think I missed that in previous patch because it
>>> did not leave that part or I misread the diff hunk - why do you allow
>>> one entry?
>>
>> That's for the older SoCs, e.g. RK3288, RK3399, as the 'link' and 'cache' blocks
>> are only available for RK3576 & RK3588.
> 
> Yeah, I see in the bottom of the binding
> 
>>
>>>
>>> If the first entry is function, then all others MUST built on top, thus
>>> this:
>>>
>>>> +          - const: link
>>>> +          - const: function
>>>> +          - const: cache
>>>
>>> is not correct.
>>>
>>> No, you don't change the orders. So again, if you have such binding,
>>> then you just fix the unit address leaving the binding as is.
>>
>> Changing the unit address would mean it will point inside the register range,
>> rather than at the beginning of it.
> 
> First, not true. If this was one register range, you would have one
> entry. You cannot split entries. Split entries means you have two
> SEPARATE register ranges.
> Second, it does not matter because main rule stays - the unit address
> describes the main address space. The main address space in other device
> was called "function", so I assume the main address space is also here
> the "function". Which makes sense, because link feels secondary to
> functioning of the device.
> 
>>
>> Sorry, but I don't quite get why would this be a better approach than just
>> properly list the items according to the HW layout, i.e. following the
>> address-based ordering?
> 
> We always expect the list to grow, to have common set. That's rule given
> during reviews multiple times. For multiple reasons, also explained
> (consistency, maintenance and actually proper description of hardware
> like the main reg address space).
> 
> Probably this was also given to that binding during discussions when it
> was upstream, so your change reverts previous discussion and to that I
> do not agree.

Thank you for detailing this, I get your point now.

After digging a bit further, it looks like the "function" naming has been
introduced as part of the RK3588 support via commit c6ffb7e1fb90 ("media:
dt-bindings: rockchip: Document RK3588 Video Decoder bindings").

Morever, it also sets `reg-names: false` for all the SoCs other than RK3588 -
sorry for missing this initially.

Hence "function" wasn't used at all in the context of the older SoCs, while on
RK3588 & RK3576 there is no indication that "function" should be treated as the
main address space or anything like that.  E.g. RK3588 TRM clearly shows the
"link" range at the top of the listing, starting at video decoder unit base
address:

--------------------------------------------------------------------------------
Config Register                         |   Base addr
--------------------------------------------------------------------------------
VDPU381 core0/1 link table config base  |   VDPU381_core0/1_base+0x000
VDPU381 core0/1 function config base    |   VDPU381_core0/1_base+0x100
--------------------------------------------------------------------------------
                                        |   VDPU381_core0/1_base+0x600 for Y channel
VDPU381 core0/1 cache config base       |   VDPU381_core0/1_base+0x640 for C channel
                                        |   VDPU381_core0/1_base+0x680 for head channel
--------------------------------------------------------------------------------

Assuming the reasoning above is now good enough to move further with the
proposed approach, I can prepare a new revision dropping the unnecessary
one-entry item from the reg-names, while keeping all the rest in the series as
is.

Regards,
Cristian

Re: [PATCH v3 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:26:37AM +0200, Cristian Ciocaltea wrote:
> >> Sorry, but I don't quite get why would this be a better approach than just
> >> properly list the items according to the HW layout, i.e. following the
> >> address-based ordering?
> > 
> > We always expect the list to grow, to have common set. That's rule given
> > during reviews multiple times. For multiple reasons, also explained
> > (consistency, maintenance and actually proper description of hardware
> > like the main reg address space).
> > 
> > Probably this was also given to that binding during discussions when it
> > was upstream, so your change reverts previous discussion and to that I
> > do not agree.
> 
> Thank you for detailing this, I get your point now.
> 
> After digging a bit further, it looks like the "function" naming has been
> introduced as part of the RK3588 support via commit c6ffb7e1fb90 ("media:
> dt-bindings: rockchip: Document RK3588 Video Decoder bindings").
> 
> Morever, it also sets `reg-names: false` for all the SoCs other than RK3588 -
> sorry for missing this initially.
> 
> Hence "function" wasn't used at all in the context of the older SoCs, while on
> RK3588 & RK3576 there is no indication that "function" should be treated as the
> main address space or anything like that.  E.g. RK3588 TRM clearly shows the
> "link" range at the top of the listing, starting at video decoder unit base
> address:
> 
> --------------------------------------------------------------------------------
> Config Register                         |   Base addr
> --------------------------------------------------------------------------------
> VDPU381 core0/1 link table config base  |   VDPU381_core0/1_base+0x000
> VDPU381 core0/1 function config base    |   VDPU381_core0/1_base+0x100
> --------------------------------------------------------------------------------
>                                         |   VDPU381_core0/1_base+0x600 for Y channel
> VDPU381 core0/1 cache config base       |   VDPU381_core0/1_base+0x640 for C channel
>                                         |   VDPU381_core0/1_base+0x680 for head channel
> --------------------------------------------------------------------------------
> 
> Assuming the reasoning above is now good enough to move further with the
> proposed approach, I can prepare a new revision dropping the unnecessary
> one-entry item from the reg-names, while keeping all the rest in the series as
> is.

Yes, with drop of the oneOf this would be fine.

> 
> Regards,
> Cristian
> 

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

[Krzysztof Kozlowski]

On 26/02/2026 08:25, Krzysztof Kozlowski wrote:
> On Thu, Feb 26, 2026 at 12:26:37AM +0200, Cristian Ciocaltea wrote:
>>>> Sorry, but I don't quite get why would this be a better approach than just
>>>> properly list the items according to the HW layout, i.e. following the
>>>> address-based ordering?
>>>
>>> We always expect the list to grow, to have common set. That's rule given
>>> during reviews multiple times. For multiple reasons, also explained
>>> (consistency, maintenance and actually proper description of hardware
>>> like the main reg address space).
>>>
>>> Probably this was also given to that binding during discussions when it
>>> was upstream, so your change reverts previous discussion and to that I
>>> do not agree.
>>
>> Thank you for detailing this, I get your point now.
>>
>> After digging a bit further, it looks like the "function" naming has been
>> introduced as part of the RK3588 support via commit c6ffb7e1fb90 ("media:
>> dt-bindings: rockchip: Document RK3588 Video Decoder bindings").
>>
>> Morever, it also sets `reg-names: false` for all the SoCs other than RK3588 -
>> sorry for missing this initially.
>>
>> Hence "function" wasn't used at all in the context of the older SoCs, while on
>> RK3588 & RK3576 there is no indication that "function" should be treated as the
>> main address space or anything like that.  E.g. RK3588 TRM clearly shows the
>> "link" range at the top of the listing, starting at video decoder unit base
>> address:
>>
>> --------------------------------------------------------------------------------
>> Config Register                         |   Base addr
>> --------------------------------------------------------------------------------
>> VDPU381 core0/1 link table config base  |   VDPU381_core0/1_base+0x000
>> VDPU381 core0/1 function config base    |   VDPU381_core0/1_base+0x100
>> --------------------------------------------------------------------------------
>>                                         |   VDPU381_core0/1_base+0x600 for Y channel
>> VDPU381 core0/1 cache config base       |   VDPU381_core0/1_base+0x640 for C channel
>>                                         |   VDPU381_core0/1_base+0x680 for head channel
>> --------------------------------------------------------------------------------
>>
>> Assuming the reasoning above is now good enough to move further with the
>> proposed approach, I can prepare a new revision dropping the unnecessary
>> one-entry item from the reg-names, while keeping all the rest in the series as
>> is.
> 
> Yes, with drop of the oneOf this would be fine.

I meant, the "one item option" in oneOf.

Best regards,
Krzysztof

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

[Cristian Ciocaltea]

On 2/26/26 9:26 AM, Krzysztof Kozlowski wrote:
> On 26/02/2026 08:25, Krzysztof Kozlowski wrote:
>> On Thu, Feb 26, 2026 at 12:26:37AM +0200, Cristian Ciocaltea wrote:
>>>>> Sorry, but I don't quite get why would this be a better approach than just
>>>>> properly list the items according to the HW layout, i.e. following the
>>>>> address-based ordering?
>>>>
>>>> We always expect the list to grow, to have common set. That's rule given
>>>> during reviews multiple times. For multiple reasons, also explained
>>>> (consistency, maintenance and actually proper description of hardware
>>>> like the main reg address space).
>>>>
>>>> Probably this was also given to that binding during discussions when it
>>>> was upstream, so your change reverts previous discussion and to that I
>>>> do not agree.
>>>
>>> Thank you for detailing this, I get your point now.
>>>
>>> After digging a bit further, it looks like the "function" naming has been
>>> introduced as part of the RK3588 support via commit c6ffb7e1fb90 ("media:
>>> dt-bindings: rockchip: Document RK3588 Video Decoder bindings").
>>>
>>> Morever, it also sets `reg-names: false` for all the SoCs other than RK3588 -
>>> sorry for missing this initially.
>>>
>>> Hence "function" wasn't used at all in the context of the older SoCs, while on
>>> RK3588 & RK3576 there is no indication that "function" should be treated as the
>>> main address space or anything like that.  E.g. RK3588 TRM clearly shows the
>>> "link" range at the top of the listing, starting at video decoder unit base
>>> address:
>>>
>>> --------------------------------------------------------------------------------
>>> Config Register                         |   Base addr
>>> --------------------------------------------------------------------------------
>>> VDPU381 core0/1 link table config base  |   VDPU381_core0/1_base+0x000
>>> VDPU381 core0/1 function config base    |   VDPU381_core0/1_base+0x100
>>> --------------------------------------------------------------------------------
>>>                                         |   VDPU381_core0/1_base+0x600 for Y channel
>>> VDPU381 core0/1 cache config base       |   VDPU381_core0/1_base+0x640 for C channel
>>>                                         |   VDPU381_core0/1_base+0x680 for head channel
>>> --------------------------------------------------------------------------------
>>>
>>> Assuming the reasoning above is now good enough to move further with the
>>> proposed approach, I can prepare a new revision dropping the unnecessary
>>> one-entry item from the reg-names, while keeping all the rest in the series as
>>> is.
>>
>> Yes, with drop of the oneOf this would be fine.
> 
> I meant, the "one item option" in oneOf.

Yes, handled in v4:

https://lore.kernel.org/all/20260226-vdec-reg-order-rk3576-v4-0-b8d72dc75250@collabora.com/

Thanks,
Cristian

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

[Nicolas Dufresne]

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

Le mercredi 25 février 2026 à 14:19 +0200, Cristian Ciocaltea a écrit :
> With the introduction of the RK3588 SoC, and RK3576 afterwards, the
> 'link' and 'cache' register blocks have been provided for the video
> decoder unit in addition to the existing 'function' one, which now shows
> up in between them (from address-based ordering point of view).
> 
> However, the binding does not properly describe this hardware layout, as
> the new blocks are listed after the old one.  Therefore it breaks the
> convention expecting the unit address to indicate the first register
> range.
> 
> Since the binding changes have been already released and a fix would
> bring up an ABI break, mark the current 'reg-names' listing as
> deprecated and introduce an alternative 'link,function,cache' one.
> 
> 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>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  .../devicetree/bindings/media/rockchip,vdec.yaml      | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> index 809fda45b3bd..3f6072e8baa5 100644
> --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> @@ -28,16 +28,21 @@ 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:
> +    oneOf:
>        - const: function
> -      - const: link
> -      - const: cache
> +      - 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

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

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

[Nicolas Dufresne]

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

Le mercredi 25 février 2026 à 13:26 +0100, Krzysztof Kozlowski a écrit :
> On 25/02/2026 13:19, Cristian Ciocaltea wrote:
> > With the introduction of the RK3588 SoC, and RK3576 afterwards, the
> > 'link' and 'cache' register blocks have been provided for the video
> > decoder unit in addition to the existing 'function' one, which now shows
> > up in between them (from address-based ordering point of view).
> > 
> > However, the binding does not properly describe this hardware layout, as
> > the new blocks are listed after the old one.  Therefore it breaks the
> > convention expecting the unit address to indicate the first register
> > range.
> > 
> > Since the binding changes have been already released and a fix would
> > bring up an ABI break, mark the current 'reg-names' listing as
> > deprecated and introduce an alternative 'link,function,cache' one.
> > 
> > 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      | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > index 809fda45b3bd..3f6072e8baa5 100644
> > --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > @@ -28,16 +28,21 @@ 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:
> > +    oneOf:
> >        - const: function
> 
> This is confusing, I think I missed that in previous patch because it
> did not leave that part or I misread the diff hunk - why do you allow
> one entry?
> 
> If the first entry is function, then all others MUST built on top, thus
> this:
> 
> > +          - const: link
> > +          - const: function
> > +          - const: cache
> 
> is not correct.
> 
> No, you don't change the orders. So again, if you have such binding,
> then you just fix the unit address leaving the binding as is.

We need a better solution, its really helpful for the base address in these unit
name matches the documentation. And specially with this version of the hardware,
the link register is where you program the IP, the function registers are filled
by the IP when the previous job completes (but can be access when there is no
running jobs for various reason too, hence exposing it).

Nicolas

p.s. I didn't read this message first, don't read by Rb as offending, I'll hold
as long as needed.
> 
> 
> > -      - const: link
> > -      - const: cache
> > +      - items:
> > +          - const: link
> > +          - const: function
> > +          - const: cache
> > +      - items:
> > +          - const: function
> > +          - const: link
> > +          - const: cache
> > +        deprecated: true
> > +        description: Use link,function,cache block order instead.
> >  
> Best regards,
> Krzysztof

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