Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support

From: "Heiko Stübner" <heiko@sntech.de>
To: Dmitry Yashin <dmt.yashin@gmail.com>,
	Luca Ceresoli <luca.ceresoli@bootlin.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Rob Herring <robh@kernel.org>,
	Jianqun Xu <jay.xu@rock-chips.com>,
	devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
Date: Tue, 28 May 2024 10:17:40 +0200	[thread overview]
Message-ID: <4771649.rnE6jSC6OK@diego> (raw)
In-Reply-To: <20240517085832.365ac878@booty>

Am Freitag, 17. Mai 2024, 08:58:32 CEST schrieb Luca Ceresoli:
> Hello Dmitry,
> 
> On Thu, 16 May 2024 17:06:46 +0500
> Dmitry Yashin <dmt.yashin@gmail.com> wrote:
> 
> > Hi Luca,
> > 
> > On 15.05.24 21:29, Luca Ceresoli wrote:
> > > I'm skeptical about this being bound to a new DT compatible. As far as I
> > > know the RK3308 and RK3308B are mostly equivalent, so it looks as the
> > > pinctrl implementation could be detected at runtime. This would let
> > > products to be built with either chip version and work on any without
> > > any DT change.  
> > 
> > 
> > Thanks for your feedback.
> > 
> > Indeed, these SoC's have a lot in common, but as I can see the rk3308b
> > has more blocks, like extra PWM's (rk3308 datasheet 1.5 [0] shows only
> > 1x PWM 4ch, when rk3308b and rk3308b-s have 3x PWM 4ch), 1-wire and
> > CAN controller (mentioned in the TRM, but dropped from rk3308b
> > datasheet for some reason).
> > 
> > So, in my view, it really makes sense to add rk3308b.dtsi, where extra
> > PWM's, pinctrl compatible and its pin functions can be moved. And if
> > its not worth it, then I will try to adapt the entire series to runtime
> > config based on cpuid like you suggested.
> 
> Having a rk3308b.dtsi would probably make sense, yes, as there are
> several differences as you described. However for the pinctrl it seems
> probably not necessary.
> 
> I've seen actual products being manufactured with two different RK3308
> variants in different lots of production, but with the same DT that has
> rockchip,rk3308-pinctrl in it. Those would need a _selective_ DT
> upgrade in order to benefit from your changes.
> 
> And even if a product had always used the B variant, it would need DT
> upgrade when upgrading to a kernel with your changes. Otherwise with
> patch 1/3 of this series the pictrl driver would lose many routes after
> upgrading the kernel (but not the DT): can this lead to
> previously-working devices to stop working? I think this is a
> fundamental question to reply.

If things can be runtime-detectable, they should be detected at runtime.
So yes, while we need to know that it is a rk3308-something before
via the dt, if we can distinguish between the rk3308 variants at runtime
we should definitly do so.

Heiko