Received-SPF: pass (google.com: domain of linux-kernel+bounces-191957-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <82435177-1a4a-46c0-9a12-c056647d587f@kwiboo.se>
Date: Tue, 28 May 2024 10:43:43 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support
To: "=?UTF-8?Q?Heiko_St=C3=BCbner?=" <heiko@sntech.de>, 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
References: <20240515121634.23945-1-dmt.yashin@gmail.com>
 <81aa0e4e-a3c7-41d1-8cd2-4d060730b37a@gmail.com>
 <20240517085832.365ac878@booty> <4771649.rnE6jSC6OK@diego>
Content-Language: en-US
From: Jonas Karlman <jonas@kwiboo.se>
In-Reply-To: <4771649.rnE6jSC6OK@diego>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 2024-05-28 10:17, Heiko Stübner wrote:
> 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.

The GRF_CHIP_ID reg (0xFF000800) can be used to identify what model is
used at runtime:

RK3308: 0xCEA (errata: chip id value is 32'h0cea (32'd3306))
RK3308B: 0x3308
RK3308BS: 0x3308C

Vendor U-Boot make use of this reg to identify what model is running:
https://github.com/rockchip-linux/u-boot/blob/next-dev/arch/arm/include/asm/arch-rockchip/cpu.h#L68-L82

I can only validate on real hw that the reg value is 0x3308 for RK3308B.

Regards,
Jonas

> 
> Heiko
>