linux-kernel - Re: [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51b28810-9d17-0505-56e7-6a3e13749fd9@manjaro.org>
Date: Fri, 14 Nov 2025 15:30:18 +0100
From: "Dragan Simic" <dsimic@...jaro.org>
To: "Michael Opdenacker" <michael.opdenacker@...tcommit.com>
Cc: "Rob Herring" <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>, "Heiko Stuebner" <heiko@...ech.de>, devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree

Hello Michael,

On Friday, November 14, 2025 14:54 CET, Michael Opdenacker <michael.opdenacker@...tcommit.com> wrote:
> Thanks a lot for your review and feedback!
> 
> On 11/14/25 03:26, Dragan Simic wrote:
> > Thanks for this patch!  Please, see some comments below.
> >
> > On Tuesday, November 11, 2025 18:20 CET, michael.opdenacker@...tcommit.com wrote:
> >> From: Michael Opdenacker <michael.opdenacker@...tcommit.com>
> >>
> >> Add initial device tree support for Asus Tinkerboard 3 [1] and 3S [2],
> >> which are SBCs based on the Rockchip 3566 SoC.
> > For consistency and because it's a proper noun, this should be
> > s/Tinkerboard/Tinker Board/.
> 
> Fixed in all the patches.

Thanks.

> > The board .dts/.dtb files should include "-board", i.e. these should
> > be "rk3566-tinker-board-3.dtb" and "rk3566-tinker-board-3s.dtb"
> > instead, because there's no real need for shortening.  These boards
> > are simply named "Tinker Board", which should be preserved.
> 
> Done too. However, I used these names for consistency with what was used 
> on arm(32) for the original Tinker Board:
> 
> arch/arm/boot/dts/rockchip/rk3288-tinker.dts
> arch/arm/boot/dts/rockchip/rk3288-tinker-s.dts
> arch/arm/boot/dts/rockchip/rk3288-tinker.dtsi
> 
> I guess it's fine to ignore what arm did right? It won't live as long as 
> arm64 (I attend Arnd's talk about arm 32).

Thanks.  Yes, I saw that naming for the other Tinker Board, based
on RK3288, which I'd consider a mistake from the past we should heed
from, instead of repeating it. :)

[snip]

> >> +/ {
> >> +	model = "Rockchip RK3566 Asus Tinker Board 3";
> > For consistency and to avoid redundancy, the "Rockchip RK3566"
> > part should be removed.
> 
> Done.

Thanks.

> >> +	compatible = "asus,rk3566-tinker-3", "rockchip,rk3566";
> >
> > Actually, the compatible should be "asus,rk3566-tinker-board-3"
> > instead, because there's no real need for shortening it.
> 
> No problem to do it. However, here we have a slightly bigger problem: it 
> would be inconsistent with the bindings for the original Tinker Board in 
> the same rockchip.yaml file:
> 
>        - description: Asus Tinker board
>          items:
>            - const: asus,rk3288-tinker
>            - const: rockchip,rk3288
> 
>        - description: Asus Tinker board S
>          items:
>            - const: asus,rk3288-tinker-s
>            - const: rockchip,rk3288
> 
> What do you think? The discrepancy would be quite visiible.

I'd still consider that old naming a mistake from the past we should
heed from, instead of repeating it for the sake of consistency that's
already lacking left and right.

> >> +	compatible = "asus,rk3566-tinker-3s", "asus,rk3566-tinker-3", "rockchip,rk3566";
> > The compatibles should be as below instead, for the same reasons
> > as already explained above.
> >
> >    "asus,rk3566-tinker-board-3s", "asus,rk3566-tinker-board-3", "rockchip,rk3566"
> 
> Yes, whatever is decided for the compatible strings.
>
> > Though, perhaps it would be better to not include the "asus,rk3566-
> > tinker-board-3" part, because I think it's pretty much redundant.
> 
> My reasoning was that Tinker Board 3S is a superset of Tinker Board 3 
> (additional eMMC and headers).
> If someday some code is associated to the compatible string for Tinker 
> 3, than Tinker 3S should use it too, right? Unless we want to have the 
> possibility to ignore some Tinker3 code in Tinker 3S for some reason. 
> Then, it's better indeed that 3S doesn't use the Tinker 3 compatible 
> string. It seems we have more options with what you're suggesting.

You were right about having "asus,rk3566-tinker-board-3" there
as well originally, because that would allow us to possibly define
some specifics later that apply to both the 3 and 3S variants.

Just like "asus,rk3566-tinker-board-3" refines "rockchip,rk3566"
and makes it more specific, "asus,rk3566-tinker-board-3s" also does
the same to "asus,rk3566-tinker-board-3".