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

From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
To: Conor Dooley <conor@kernel.org>
Cc: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>,
	Rob Herring	 <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley	 <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Detlev Casanova	 <detlev.casanova@collabora.com>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	 Mauro Carvalho Chehab	 <mchehab@kernel.org>,
	Hans Verkuil <hverkuil@kernel.org>,
	kernel@collabora.com, 	devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
		linux-rockchip@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Conor Dooley	 <conor.dooley@microchip.com>,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}
Date: Thu, 26 Feb 2026 17:41:20 -0500	[thread overview]
Message-ID: <328f330b4316f60241eb8a9935a46cc148fb4ae8.camel@collabora.com> (raw)
In-Reply-To: <20260226-ferocious-saturday-0e1f9bb28ec5@spud>

[-- Attachment #1: Type: text/plain, Size: 2621 bytes --]

Le jeudi 26 février 2026 à 22:15 +0000, Conor Dooley a écrit :
> On Thu, Feb 26, 2026 at 04:56:30PM -0500, Nicolas Dufresne wrote:
> > Le jeudi 26 février 2026 à 20:59 +0000, Conor Dooley a écrit :
> > > On Thu, Feb 26, 2026 at 02:45:11PM -0500, Nicolas Dufresne wrote:
> > > > Le jeudi 26 février 2026 à 18:43 +0000, Conor Dooley a écrit :
> > > In this case, the driver is currently buggy, because, as I mentioned, it
> > > uses reg-names without reg-names being required on the platforms with
> > > more than 1 reg property. Probably the binding should make reg-names
> > > mandatory for these platforms even without this patch, but it *has* to
> > > IMO for this proposed change to be applicable.
> > 
> > That forced me to check the driver. So for RK33xx and older, there is only
> > one
> > range, and the driver will just pick the one entry expected:
> > 
> > 
> > 	if (rkvdec->variant->has_single_reg_region) {
> > 		rkvdec->regs = devm_platform_ioremap_resource(pdev, 0);
> > 		if (IS_ERR(rkvdec->regs))
> > 			return PTR_ERR(rkvdec->regs);
> > 	} else {
> > 		rkvdec->regs = devm_platform_ioremap_resource_byname(pdev,
> > "function");
> > 		if (IS_ERR(rkvdec->regs))
> > 			return PTR_ERR(rkvdec->regs);
> > 
> > 		rkvdec->link = devm_platform_ioremap_resource_byname(pdev,
> > "link");
> > 		if (IS_ERR(rkvdec->link))
> > 			return PTR_ERR(rkvdec->link);
> > 	}
> > 
> > 
> > Where for RK35xx variants, it only pick the resources by name. I don't see
> > the
> > bug that you see, but I believe this was just a supposition, that you didn't
> > check the code.
> 
> Busy reading path of exile patch notes, so sniping this comment only...
> 
> This is a bug, not a supposition, and it's that snippet from the
> driver that prompted my comment.. That code requires that if
> ->has_single_reg_region is set that the dts provides reg-names, but the
> binding does not mandate reg-names for rk3576-vdec and rk3588-vdec, so
> the driver will fail to probe on a dts that the binding says is valid.

Got it, to me nothing in the binding is intentional, just a big lack of
understanding what the syntax meant. So if we agree to make the binding enforce
having names for rk3576-vdec and rk3588-vdec (and keep RK33 and older the same),
It would make me more happy to maintain it.

It also better highlight why adding a second order simply creates an ambiguity.

Let me know, if we can go forward with that, I'll help documenting known users,
their introduction into 7.0-rc1 only and why this incident and fix should have
no impact on anyone.

Nicolas

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