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

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

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