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

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

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