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: 4412 bytes --]

Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> 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 it breaks the convention expecting the unit address to indicate the
> > start of the first register range, i.e. 'function' block is listed
> > before 'link' instead of the opposite.
> 
> I don't understand this commit message or rationale for an ABI break.
> Changing the unit address seems like a "free" fix to your problem,
> especially when reg-names is not a required property that you can rely
> on. Actually, there may be a bug in the driver - it expects reg-names
> for rk3576-vdec and rk3588-vdec but the binding doesn't mandate their
> presence for those devices.

If the bindings had been held instead of released early, the order would be what
is done in this patch for sure, and no one would have complained. These binding
have never been used by anyone so far, and what you are asking is to create a
DTS that deviate from the vendor provided documentation for the IP base address.
I know you have all technical fancy reasoning, but I like when things are
functional and effective to work with.

> 
> Deprecating the order also makes little sense to me, given that some of
> these devices only have one reg entry, which as far as I can tell from
> looking at the driver *is* the "function" region, so it can never be
> entirely deprecated.

What I'd like to see, is a binding expression that behave like a set, not a
list, and leave the ordering open. As people keep repeating, there is nothing in
a binding that assist to define the right ordering (its not address or base
addres aware). That basically means, we can't as reviewer see that ordering is
going to imposing using a base address in the unit name (which is a convenience,
not a rule I suppose) that differ from the vendor documented base address.

By explicitly removing the ordering in the binding, we create a strict rule that
driver should retrieve this by name, and never assume the ordering, which I
personally like.

thoughts ?

Nicolas

> 
> Confused,
> Conor.
> 
> > 
> > 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.
> > 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > ---
> >  .../devicetree/bindings/media/rockchip,vdec.yaml     | 20 ++++++++++++-----
> > ---
> >  1 file changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > index 809fda45b3bd..c513b68d2c72 100644
> > --- a/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > +++ b/Documentation/devicetree/bindings/media/rockchip,vdec.yaml
> > @@ -28,16 +28,20 @@ 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:
> > -      - const: function
> > -      - const: link
> > -      - const: cache
> > +    oneOf:
> > +      - 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
> > 

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