[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 at 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