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

[Krzysztof Kozlowski <krzk@kernel.org>]

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