* [PATCH 1/2] dt-bindings: arm: rockchip: Asus Tinkerboard 3 and 3S [not found] <20251111172003.2324525-1-michael.opdenacker@rootcommit.com> @ 2025-11-11 17:20 ` michael.opdenacker 2025-11-13 8:35 ` Krzysztof Kozlowski 2025-11-14 2:00 ` Dragan Simic 2025-11-11 17:20 ` [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree michael.opdenacker 1 sibling, 2 replies; 9+ messages in thread From: michael.opdenacker @ 2025-11-11 17:20 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner Cc: Michael Opdenacker, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel From: Michael Opdenacker <michael.opdenacker@rootcommit.com> Document the compatible strings for Asus Tinkerboard 3 [1] and 3S [2], which are SBCs based on the Rockchip 3566 SoC. The "3S" version ("S" for "storage") just adds a 16 GB eMMC and a "mask ROM" DIP switch to the "3" version. Link: https://tinker-board.asus.com/series/tinker-board-3.html [1] Link: https://tinker-board.asus.com/series/tinker-board-3s.html [2] Signed-off-by: Michael Opdenacker <michael.opdenacker@rootcommit.com> --- Documentation/devicetree/bindings/arm/rockchip.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml index 6aceaa8acbb2..451597a6cffb 100644 --- a/Documentation/devicetree/bindings/arm/rockchip.yaml +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml @@ -86,6 +86,17 @@ properties: - const: asus,rk3288-tinker-s - const: rockchip,rk3288 + - description: Asus Tinker board 3 + items: + - const: asus,rk3566-tinker-3 + - const: rockchip,rk3566 + + - description: Asus Tinker board 3S + items: + - const: asus,rk3566-tinker-3s + - const: asus,rk3566-tinker-3 + - const: rockchip,rk3566 + - description: Beelink A1 items: - const: azw,beelink-a1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: rockchip: Asus Tinkerboard 3 and 3S 2025-11-11 17:20 ` [PATCH 1/2] dt-bindings: arm: rockchip: Asus Tinkerboard 3 and 3S michael.opdenacker @ 2025-11-13 8:35 ` Krzysztof Kozlowski 2025-11-14 2:00 ` Dragan Simic 1 sibling, 0 replies; 9+ messages in thread From: Krzysztof Kozlowski @ 2025-11-13 8:35 UTC (permalink / raw) To: michael.opdenacker Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel On Tue, Nov 11, 2025 at 05:20:22PM +0000, michael.opdenacker@rootcommit.com wrote: > From: Michael Opdenacker <michael.opdenacker@rootcommit.com> > > Document the compatible strings for Asus Tinkerboard 3 [1] and 3S [2], > which are SBCs based on the Rockchip 3566 SoC. > > The "3S" version ("S" for "storage") just adds a 16 GB eMMC > and a "mask ROM" DIP switch to the "3" version. > > Link: https://tinker-board.asus.com/series/tinker-board-3.html [1] > Link: https://tinker-board.asus.com/series/tinker-board-3s.html [2] > Signed-off-by: Michael Opdenacker <michael.opdenacker@rootcommit.com> > --- > Documentation/devicetree/bindings/arm/rockchip.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: rockchip: Asus Tinkerboard 3 and 3S 2025-11-11 17:20 ` [PATCH 1/2] dt-bindings: arm: rockchip: Asus Tinkerboard 3 and 3S michael.opdenacker 2025-11-13 8:35 ` Krzysztof Kozlowski @ 2025-11-14 2:00 ` Dragan Simic 1 sibling, 0 replies; 9+ messages in thread From: Dragan Simic @ 2025-11-14 2:00 UTC (permalink / raw) To: michael.opdenacker Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel Hello Michael, Thanks for this patch! Please, see some comments below. On Tuesday, November 11, 2025 18:20 CET, michael.opdenacker@rootcommit.com wrote: > From: Michael Opdenacker <michael.opdenacker@rootcommit.com> > > Document the compatible strings for Asus Tinkerboard 3 [1] and 3S [2], > which are SBCs based on the Rockchip 3566 SoC. For consistency, this should be s/Tinkerboard/Tinker Board/. > The "3S" version ("S" for "storage") just adds a 16 GB eMMC > and a "mask ROM" DIP switch to the "3" version. > > Link: https://tinker-board.asus.com/series/tinker-board-3.html [1] > Link: https://tinker-board.asus.com/series/tinker-board-3s.html [2] These two lines should read like this, to serve as references, with an empty line afterwards, of course: [1] https://tinker-board.asus.com/series/tinker-board-3.html [2] https://tinker-board.asus.com/series/tinker-board-3s.html > Signed-off-by: Michael Opdenacker <michael.opdenacker@rootcommit.com> > --- > Documentation/devicetree/bindings/arm/rockchip.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml > index 6aceaa8acbb2..451597a6cffb 100644 > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml > @@ -86,6 +86,17 @@ properties: > - const: asus,rk3288-tinker-s > - const: rockchip,rk3288 > > + - description: Asus Tinker board 3 This should be s/Tinker board/Tinker Board/, because it's actually a proper noun/name. > + items: > + - const: asus,rk3566-tinker-3 > + - const: rockchip,rk3566 > + > + - description: Asus Tinker board 3S The same as above. > + items: > + - const: asus,rk3566-tinker-3s > + - const: asus,rk3566-tinker-3 > + - const: rockchip,rk3566 > + > - description: Beelink A1 > items: > - const: azw,beelink-a1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree [not found] <20251111172003.2324525-1-michael.opdenacker@rootcommit.com> 2025-11-11 17:20 ` [PATCH 1/2] dt-bindings: arm: rockchip: Asus Tinkerboard 3 and 3S michael.opdenacker @ 2025-11-11 17:20 ` michael.opdenacker 2025-11-13 22:35 ` Heiko Stübner 2025-11-14 2:26 ` Dragan Simic 1 sibling, 2 replies; 9+ messages in thread From: michael.opdenacker @ 2025-11-11 17:20 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner Cc: Michael Opdenacker, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel From: Michael Opdenacker <michael.opdenacker@rootcommit.com> Add initial device tree support for Asus Tinkerboard 3 [1] and 3S [2], which are SBCs based on the Rockchip 3566 SoC. The "3S" version ("S" for "storage") just adds a 16 GB eMMC and a "mask ROM" DIP switch (to mask the eMMC and enter "Mask ROM" mode for recovery) to the "3" version. This adds support for: - Debug UART (/dev/ttyS2) - SD card (/dev/mmcblk1) - eMMC (/dev/mmcblk0, only on Tinkerboard 3S) - I2C: - i2c0 (internal bus with a PMIC and regulators) - i2c2 (internal bus with an at24 eeprom and an RTC device) - USB 2.0 ports - 2 GPIO LEDS Link: https://tinker-board.asus.com/series/tinker-board-3.html [1] Link: https://tinker-board.asus.com/series/tinker-board-3s.html [2] Signed-off-by: Michael Opdenacker <michael.opdenacker@rootcommit.com> --- arch/arm64/boot/dts/rockchip/Makefile | 2 + .../boot/dts/rockchip/rk3566-tinker-3.dts | 14 + .../boot/dts/rockchip/rk3566-tinker-3.dtsi | 264 ++++++++++++++++++ .../boot/dts/rockchip/rk3566-tinker-3s.dts | 30 ++ 4 files changed, 310 insertions(+) create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dtsi create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-tinker-3s.dts diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile index ad684e3831bc..381831cab20c 100644 --- a/arch/arm64/boot/dts/rockchip/Makefile +++ b/arch/arm64/boot/dts/rockchip/Makefile @@ -130,6 +130,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-lubancat-1.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-nanopi-r3s.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-bigtreetech-cb2-manta.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-bigtreetech-pi2.dtb +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-tinker-3.dtb +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-tinker-3s.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-fastrhino-r66s.dtb diff --git a/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts new file mode 100644 index 000000000000..938af35b9004 --- /dev/null +++ b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +/* + * Copyright (C) 2025 Michael Opdenacker <michael.opdenacker@rootcommit.com> + */ + +/dts-v1/; + +#include "rk3566-tinker-3.dtsi" + +/ { + model = "Rockchip RK3566 Asus Tinker Board 3"; + compatible = "asus,rk3566-tinker-3", "rockchip,rk3566"; +}; + diff --git a/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dtsi new file mode 100644 index 000000000000..45269d33b0cb --- /dev/null +++ b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dtsi @@ -0,0 +1,264 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +/* + * Copyright (C) 2025 Michael Opdenacker <michael.opdenacker@rootcommit.com> + */ + +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/leds/common.h> +#include <dt-bindings/pinctrl/rockchip.h> +#include <dt-bindings/soc/rockchip,vop2.h> +#include "rk3566.dtsi" + +/ { + aliases { + serial2 = &uart2; + mmc1 = &sdmmc0; + i2c0 = &i2c0; + i2c2 = &i2c2; + }; + + chosen { + stdout-path = "serial2:1500000n8"; + }; + + vcc3v3_sys: regulator-3v3-vcc-sys { + compatible = "regulator-fixed"; + regulator-name = "vcc3v3_sys"; + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + vin-supply = <&vcc5v0_sys>; + }; + + vcc5v0_sys: regulator-5v0-vcc-sys { + compatible = "regulator-fixed"; + regulator-name = "vcc5v0_sys"; + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; + + vcc5v0_usb_host: regulator-5v0-vcc-usb-host { + compatible = "regulator-fixed"; + enable-active-high; + gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&usb_host_pwren_h>; + regulator-name = "vcc5v0_usb_host"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + vin-supply = <&vcc5v0_sys>; + }; + + gpio_leds: gpio-leds { + compatible = "gpio-leds"; + + act-led { + gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>; + linux,default-trigger="mmc1"; +}; + + rsv-led { + gpios = <&gpio0 RK_PD6 GPIO_ACTIVE_HIGH>; + linux,default-trigger="none"; + }; + }; +}; + +&uart2 { + status = "okay"; +}; + +&i2c0 { + status = "okay"; + + rk809: pmic@20 { + compatible = "rockchip,rk809"; + reg = <0x20>; + assigned-clocks = <&cru I2S1_MCLKOUT_TX>; + assigned-clock-parents = <&cru CLK_I2S1_8CH_TX>; + #clock-cells = <1>; + clocks = <&cru I2S1_MCLKOUT_TX>; + clock-names = "mclk"; + clock-output-names = "rk809-clkout1", "rk809-clkout2"; + interrupt-parent = <&gpio0>; + interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>; + pinctrl-names = "default"; + pinctrl-0 = <&pmic_int_l>, <&i2s1m0_mclk>; + #sound-dai-cells = <0>; + system-power-controller; + wakeup-source; + + vcc1-supply = <&vcc3v3_sys>; + vcc2-supply = <&vcc3v3_sys>; + vcc3-supply = <&vcc3v3_sys>; + vcc4-supply = <&vcc3v3_sys>; + vcc5-supply = <&vcc3v3_sys>; + vcc6-supply = <&vcc3v3_sys>; + vcc7-supply = <&vcc3v3_sys>; + vcc8-supply = <&vcc3v3_sys>; + vcc9-supply = <&vcc3v3_sys>; + + regulators { + vcc_1v8: DCDC_REG5 { + regulator-name = "vcc_1v8"; + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + + vcc3v3_sd: SWITCH_REG2 { + regulator-name = "vcc3v3_sd"; + regulator-always-on; + regulator-boot-on; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + + vccio_sd: LDO_REG5 { + regulator-name = "vccio_sd"; + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + + vcc_3v3: SWITCH_REG1 { + regulator-name = "vcc_3v3"; + regulator-always-on; + regulator-boot-on; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + }; + }; + + vdd_cpu: regulator@40 { + compatible = "silergy,syr827"; + reg = <0x40>; + fcs,suspend-voltage-selector = <1>; + regulator-name = "vdd_cpu"; + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <830000>; + regulator-max-microvolt = <1200000>; + regulator-ramp-delay = <2300>; + vin-supply = <&vcc3v3_sys>; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; +}; + +&i2c2 { + status = "okay"; + + m24c08@50 { + compatible = "atmel,24c08"; + reg = <0x50>; + pinctrl-names = "default"; + pinctrl-0 = <&tb3_eeprom>; + status = "okay"; + }; + + rtc_isl1208: rtc@6f { + compatible = "isil,isl1208"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_externalrtc_reg>; + interrupt-names = "irq"; + interrupts-extended = <&gpio0 RK_PD3 IRQ_TYPE_EDGE_FALLING>; + reg = <0x6f>; + status = "okay"; + }; +}; + +&sdmmc0 { + bus-width = <4>; + cap-sd-highspeed; + disable-wp; + pinctrl-names = "default"; + pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>; + vmmc-supply = <&vcc3v3_sd>; + vqmmc-supply = <&vccio_sd>; + status = "okay"; +}; + +&usb_host0_ehci { + status = "okay"; +}; + +&usb_host0_ohci { + status = "okay"; +}; + +&usb_host1_ehci { + status = "okay"; +}; + +&usb_host1_ohci { + status = "okay"; +}; + +&usb2phy0 { + status = "okay"; +}; + +&usb2phy0_host { + phy-supply = <&vcc5v0_usb_host>; + status = "okay"; +}; + +&usb2phy1 { + status = "okay"; +}; + +&usb2phy1_host { + phy-supply = <&vcc5v0_usb_host>; + status = "okay"; +}; + +&pinctrl { + pmic { + pmic_int_l: pmic-int-l { + rockchip,pins = <0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>; + }; + }; + + usb { + usb_host_pwren_h: usb-host-pwren-h { + rockchip,pins = <0 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>; + }; + + usb_otg_pwren_h: usb-otg-pwren-h { + rockchip,pins = <0 RK_PA5 RK_FUNC_GPIO &pcfg_pull_none>; + }; + }; + + rtc { + pinctrl_externalrtc_reg: externalrtcreggrp { + rockchip,pins = <0 RK_PD3 RK_FUNC_GPIO &pcfg_pull_up>; + }; + }; + + eeprom { + tb3_eeprom: tb3-eeprom { + rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_up>; + }; + }; +}; diff --git a/arch/arm64/boot/dts/rockchip/rk3566-tinker-3s.dts b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3s.dts new file mode 100644 index 000000000000..ba7efd702050 --- /dev/null +++ b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3s.dts @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +/* + * Copyright (C) 2025 Michael Opdenacker <michael.opdenacker@rootcommit.com> + */ + +/dts-v1/; + +#include "rk3566-tinker-3.dtsi" + +/ { + model = "Rockchip RK3566 Asus Tinker Board 3S"; + compatible = "asus,rk3566-tinker-3s", "asus,rk3566-tinker-3", "rockchip,rk3566"; + + aliases { + mmc0 = &sdhci; + }; +}; + +&sdhci { + bus-width = <8>; + cap-mmc-highspeed; + max-frequency = <200000000>; + mmc-hs200-1_8v; + non-removable; + pinctrl-names = "default"; + pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe>; + vmmc-supply = <&vcc_3v3>; + vqmmc-supply = <&vcc_1v8>; + status = "okay"; +}; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree 2025-11-11 17:20 ` [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree michael.opdenacker @ 2025-11-13 22:35 ` Heiko Stübner 2025-11-14 14:31 ` Michael Opdenacker 2025-11-14 2:26 ` Dragan Simic 1 sibling, 1 reply; 9+ messages in thread From: Heiko Stübner @ 2025-11-13 22:35 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, michael.opdenacker Cc: Michael Opdenacker, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel Hi Michael, Am Dienstag, 11. November 2025, 18:20:23 Mitteleuropäische Normalzeit schrieb michael.opdenacker@rootcommit.com: > From: Michael Opdenacker <michael.opdenacker@rootcommit.com> > > Add initial device tree support for Asus Tinkerboard 3 [1] and 3S [2], > which are SBCs based on the Rockchip 3566 SoC. > > The "3S" version ("S" for "storage") just adds a 16 GB eMMC > and a "mask ROM" DIP switch (to mask the eMMC and enter "Mask ROM" > mode for recovery) to the "3" version. > > This adds support for: > - Debug UART (/dev/ttyS2) > - SD card (/dev/mmcblk1) > - eMMC (/dev/mmcblk0, only on Tinkerboard 3S) > - I2C: > - i2c0 (internal bus with a PMIC and regulators) > - i2c2 (internal bus with an at24 eeprom and an RTC device) > - USB 2.0 ports > - 2 GPIO LEDS > > Link: https://tinker-board.asus.com/series/tinker-board-3.html [1] > Link: https://tinker-board.asus.com/series/tinker-board-3s.html [2] > Signed-off-by: Michael Opdenacker <michael.opdenacker@rootcommit.com> > --- please follow the DTS coding style https://docs.kernel.org/devicetree/bindings/dts-coding-style.html > +/ { > + aliases { > + serial2 = &uart2; > + mmc1 = &sdmmc0; > + i2c0 = &i2c0; > + i2c2 = &i2c2; alphabetical property order > + }; > + > + chosen { > + stdout-path = "serial2:1500000n8"; > + }; > + > + vcc3v3_sys: regulator-3v3-vcc-sys { > + compatible = "regulator-fixed"; > + regulator-name = "vcc3v3_sys"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + vin-supply = <&vcc5v0_sys>; > + }; > + > + vcc5v0_sys: regulator-5v0-vcc-sys { > + compatible = "regulator-fixed"; > + regulator-name = "vcc5v0_sys"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + }; > + > + vcc5v0_usb_host: regulator-5v0-vcc-usb-host { > + compatible = "regulator-fixed"; > + enable-active-high; > + gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&usb_host_pwren_h>; > + regulator-name = "vcc5v0_usb_host"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + vin-supply = <&vcc5v0_sys>; > + }; > + > + gpio_leds: gpio-leds { gpio-foo before regulator-bar > + compatible = "gpio-leds"; > + > + act-led { > + gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>; > + linux,default-trigger="mmc1"; > +}; missing indentation > + > + rsv-led { > + gpios = <&gpio0 RK_PD6 GPIO_ACTIVE_HIGH>; > + linux,default-trigger="none"; > + }; > + }; > +}; > + > +&uart2 { alphabetical ordering of phandles please (uart2 definitly somewhere after i2c0) > + status = "okay"; > +}; > + > +&i2c0 { > + status = "okay"; > + > + rk809: pmic@20 { > + compatible = "rockchip,rk809"; > + reg = <0x20>; > + assigned-clocks = <&cru I2S1_MCLKOUT_TX>; > + assigned-clock-parents = <&cru CLK_I2S1_8CH_TX>; > + #clock-cells = <1>; > + clocks = <&cru I2S1_MCLKOUT_TX>; > + clock-names = "mclk"; > + clock-output-names = "rk809-clkout1", "rk809-clkout2"; > + interrupt-parent = <&gpio0>; > + interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_int_l>, <&i2s1m0_mclk>; > + #sound-dai-cells = <0>; > + system-power-controller; > + wakeup-source; > + > + vcc1-supply = <&vcc3v3_sys>; > + vcc2-supply = <&vcc3v3_sys>; > + vcc3-supply = <&vcc3v3_sys>; > + vcc4-supply = <&vcc3v3_sys>; > + vcc5-supply = <&vcc3v3_sys>; > + vcc6-supply = <&vcc3v3_sys>; > + vcc7-supply = <&vcc3v3_sys>; > + vcc8-supply = <&vcc3v3_sys>; > + vcc9-supply = <&vcc3v3_sys>; > + > + regulators { > + vcc_1v8: DCDC_REG5 { > + regulator-name = "vcc_1v8"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc3v3_sd: SWITCH_REG2 { > + regulator-name = "vcc3v3_sd"; > + regulator-always-on; > + regulator-boot-on; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vccio_sd: LDO_REG5 { > + regulator-name = "vccio_sd"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_3v3: SWITCH_REG1 { > + regulator-name = "vcc_3v3"; > + regulator-always-on; > + regulator-boot-on; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + }; > + }; > + > + vdd_cpu: regulator@40 { you probably need &cpu0 phandles to set this regulator-supply? > + compatible = "silergy,syr827"; > + reg = <0x40>; > + fcs,suspend-voltage-selector = <1>; > + regulator-name = "vdd_cpu"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <830000>; > + regulator-max-microvolt = <1200000>; > + regulator-ramp-delay = <2300>; > + vin-supply = <&vcc3v3_sys>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > +}; > + > +&i2c2 { > + status = "okay"; > + > + m24c08@50 { I guess eeprom@50 ? Heiko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree 2025-11-13 22:35 ` Heiko Stübner @ 2025-11-14 14:31 ` Michael Opdenacker 0 siblings, 0 replies; 9+ messages in thread From: Michael Opdenacker @ 2025-11-14 14:31 UTC (permalink / raw) To: Heiko Stübner, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: michael.opdenacker, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel Hi Heiko Thanks a lot for the review! On 11/13/25 23:35, Heiko Stübner wrote: > Hi Michael, > > Am Dienstag, 11. November 2025, 18:20:23 Mitteleuropäische Normalzeit schrieb michael.opdenacker@rootcommit.com: >> From: Michael Opdenacker <michael.opdenacker@rootcommit.com> >> >> Add initial device tree support for Asus Tinkerboard 3 [1] and 3S [2], >> which are SBCs based on the Rockchip 3566 SoC. >> >> The "3S" version ("S" for "storage") just adds a 16 GB eMMC >> and a "mask ROM" DIP switch (to mask the eMMC and enter "Mask ROM" >> mode for recovery) to the "3" version. >> >> This adds support for: >> - Debug UART (/dev/ttyS2) >> - SD card (/dev/mmcblk1) >> - eMMC (/dev/mmcblk0, only on Tinkerboard 3S) >> - I2C: >> - i2c0 (internal bus with a PMIC and regulators) >> - i2c2 (internal bus with an at24 eeprom and an RTC device) >> - USB 2.0 ports >> - 2 GPIO LEDS >> >> Link: https://tinker-board.asus.com/series/tinker-board-3.html [1] >> Link: https://tinker-board.asus.com/series/tinker-board-3s.html [2] >> Signed-off-by: Michael Opdenacker <michael.opdenacker@rootcommit.com> >> --- > please follow the DTS coding style > https://docs.kernel.org/devicetree/bindings/dts-coding-style.html Great, I was looking for such a document :) > > >> +/ { >> + aliases { >> + serial2 = &uart2; >> + mmc1 = &sdmmc0; >> + i2c0 = &i2c0; >> + i2c2 = &i2c2; > alphabetical property order Fixed. > > >> + }; >> + >> + chosen { >> + stdout-path = "serial2:1500000n8"; >> + }; >> + >> + vcc3v3_sys: regulator-3v3-vcc-sys { >> + compatible = "regulator-fixed"; >> + regulator-name = "vcc3v3_sys"; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + vin-supply = <&vcc5v0_sys>; >> + }; >> + >> + vcc5v0_sys: regulator-5v0-vcc-sys { >> + compatible = "regulator-fixed"; >> + regulator-name = "vcc5v0_sys"; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-min-microvolt = <5000000>; >> + regulator-max-microvolt = <5000000>; >> + }; >> + >> + vcc5v0_usb_host: regulator-5v0-vcc-usb-host { >> + compatible = "regulator-fixed"; >> + enable-active-high; >> + gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&usb_host_pwren_h>; >> + regulator-name = "vcc5v0_usb_host"; >> + regulator-min-microvolt = <5000000>; >> + regulator-max-microvolt = <5000000>; >> + vin-supply = <&vcc5v0_sys>; >> + }; >> + >> + gpio_leds: gpio-leds { > gpio-foo before regulator-bar Done > >> + compatible = "gpio-leds"; >> + >> + act-led { >> + gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>; >> + linux,default-trigger="mmc1"; >> +}; > missing indentation Oops, fixed. > >> + >> + rsv-led { >> + gpios = <&gpio0 RK_PD6 GPIO_ACTIVE_HIGH>; >> + linux,default-trigger="none"; >> + }; >> + }; >> +}; >> >> >>> Heiko >>> >>> >> + >> +&uart2 { > alphabetical ordering of phandles please (uart2 definitly somewhere after i2c0) Indeed. Done. > >> + status = "okay"; >> +}; >> + >> +&i2c0 { >> + status = "okay"; >> + >> + rk809: pmic@20 { >> + compatible = "rockchip,rk809"; >> + reg = <0x20>; >> + assigned-clocks = <&cru I2S1_MCLKOUT_TX>; >> + assigned-clock-parents = <&cru CLK_I2S1_8CH_TX>; >> + #clock-cells = <1>; >> + clocks = <&cru I2S1_MCLKOUT_TX>; >> + clock-names = "mclk"; >> + clock-output-names = "rk809-clkout1", "rk809-clkout2"; >> + interrupt-parent = <&gpio0>; >> + interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pmic_int_l>, <&i2s1m0_mclk>; >> + #sound-dai-cells = <0>; >> + system-power-controller; >> + wakeup-source; >> + >> + vcc1-supply = <&vcc3v3_sys>; >> + vcc2-supply = <&vcc3v3_sys>; >> + vcc3-supply = <&vcc3v3_sys>; >> + vcc4-supply = <&vcc3v3_sys>; >> + vcc5-supply = <&vcc3v3_sys>; >> + vcc6-supply = <&vcc3v3_sys>; >> + vcc7-supply = <&vcc3v3_sys>; >> + vcc8-supply = <&vcc3v3_sys>; >> + vcc9-supply = <&vcc3v3_sys>; >> + >> + regulators { >> + vcc_1v8: DCDC_REG5 { >> + regulator-name = "vcc_1v8"; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + >> + regulator-state-mem { >> + regulator-off-in-suspend; >> + }; >> + }; >> + >> + vcc3v3_sd: SWITCH_REG2 { >> + regulator-name = "vcc3v3_sd"; >> + regulator-always-on; >> + regulator-boot-on; >> + >> + regulator-state-mem { >> + regulator-off-in-suspend; >> + }; >> + }; >> + >> + vccio_sd: LDO_REG5 { >> + regulator-name = "vccio_sd"; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3300000>; >> + >> + regulator-state-mem { >> + regulator-off-in-suspend; >> + }; >> + }; >> + >> + vcc_3v3: SWITCH_REG1 { >> + regulator-name = "vcc_3v3"; >> + regulator-always-on; >> + regulator-boot-on; >> + >> + regulator-state-mem { >> + regulator-off-in-suspend; >> + }; >> + }; >> + }; >> + }; >> + >> + vdd_cpu: regulator@40 { > you probably need &cpu0 phandles to set this regulator-supply? Oh, you're the maintainer on this one :) Good catch. I was imitating the Orange Pi 3B DT, and since the board seemed to boot fine, I didn't pay attention to the fact that vdd_cpu was needed elsewhere. Fixed. > >> + compatible = "silergy,syr827"; >> + reg = <0x40>; >> + fcs,suspend-voltage-selector = <1>; >> + regulator-name = "vdd_cpu"; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-min-microvolt = <830000>; >> + regulator-max-microvolt = <1200000>; >> + regulator-ramp-delay = <2300>; >> + vin-supply = <&vcc3v3_sys>; >> + >> + regulator-state-mem { >> + regulator-off-in-suspend; >> + }; >> + }; >> +}; >> + >> +&i2c2 { >> + status = "okay"; >> + >> + m24c08@50 { > I guess eeprom@50 ? Indeed. Fixed too. Thanks again, Michael. -- Michael Opdenacker Root Commit Embedded Linux Training and Consulting https://rootcommit.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree 2025-11-11 17:20 ` [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree michael.opdenacker 2025-11-13 22:35 ` Heiko Stübner @ 2025-11-14 2:26 ` Dragan Simic 2025-11-14 13:54 ` Michael Opdenacker 1 sibling, 1 reply; 9+ messages in thread From: Dragan Simic @ 2025-11-14 2:26 UTC (permalink / raw) To: michael.opdenacker Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel Hello Michael, Thanks for this patch! Please, see some comments below. On Tuesday, November 11, 2025 18:20 CET, michael.opdenacker@rootcommit.com wrote: > From: Michael Opdenacker <michael.opdenacker@rootcommit.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/. > The "3S" version ("S" for "storage") just adds a 16 GB eMMC > and a "mask ROM" DIP switch (to mask the eMMC and enter "Mask ROM" > mode for recovery) to the "3" version. > > This adds support for: > - Debug UART (/dev/ttyS2) > - SD card (/dev/mmcblk1) > - eMMC (/dev/mmcblk0, only on Tinkerboard 3S) > - I2C: > - i2c0 (internal bus with a PMIC and regulators) > - i2c2 (internal bus with an at24 eeprom and an RTC device) > - USB 2.0 ports > - 2 GPIO LEDS > > Link: https://tinker-board.asus.com/series/tinker-board-3.html [1] > Link: https://tinker-board.asus.com/series/tinker-board-3s.html [2] These two lines should read like this, to serve as references, with an empty line afterwards, of course: [1] https://tinker-board.asus.com/series/tinker-board-3.html [2] https://tinker-board.asus.com/series/tinker-board-3s.html > Signed-off-by: Michael Opdenacker <michael.opdenacker@rootcommit.com> > --- > arch/arm64/boot/dts/rockchip/Makefile | 2 + > .../boot/dts/rockchip/rk3566-tinker-3.dts | 14 + > .../boot/dts/rockchip/rk3566-tinker-3.dtsi | 264 ++++++++++++++++++ > .../boot/dts/rockchip/rk3566-tinker-3s.dts | 30 ++ > 4 files changed, 310 insertions(+) > create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts > create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dtsi > create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-tinker-3s.dts > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile > index ad684e3831bc..381831cab20c 100644 > --- a/arch/arm64/boot/dts/rockchip/Makefile > +++ b/arch/arm64/boot/dts/rockchip/Makefile > @@ -130,6 +130,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-lubancat-1.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-nanopi-r3s.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-bigtreetech-cb2-manta.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-bigtreetech-pi2.dtb > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-tinker-3.dtb > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-tinker-3s.dtb 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. > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-fastrhino-r66s.dtb > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts > new file mode 100644 > index 000000000000..938af35b9004 > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts > @@ -0,0 +1,14 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > +/* > + * Copyright (C) 2025 Michael Opdenacker <michael.opdenacker@rootcommit.com> > + */ > + > +/dts-v1/; > + > +#include "rk3566-tinker-3.dtsi" The same comment from above about the naming applies here (as well as in other places), so we should have "rk3566-tinker-board-3.dtsi" here instead. > + > +/ { > + model = "Rockchip RK3566 Asus Tinker Board 3"; For consistency and to avoid redundancy, the "Rockchip RK3566" part should be removed. > + 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. > +}; > + > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dtsi > new file mode 100644 > index 000000000000..45269d33b0cb > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dtsi This .dtsi file should be named "rk3566-tinker-board-3.dtsi" instead, because there's no need for shortening. Please see also a comment above, regarding the naming. [snip] > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-tinker-3s.dts b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3s.dts > new file mode 100644 > index 000000000000..ba7efd702050 > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3s.dts This .dts file should be named "rk3566-tinker-board-3s.dts" instead, because there's no need for shortening. > @@ -0,0 +1,30 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > +/* > + * Copyright (C) 2025 Michael Opdenacker <michael.opdenacker@rootcommit.com> > + */ > + > +/dts-v1/; > + > +#include "rk3566-tinker-3.dtsi" > + > +/ { > + model = "Rockchip RK3566 Asus Tinker Board 3S"; For consistency and to avoid redundancy, the "Rockchip RK3566" part should be removed. > + 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" Though, perhaps it would be better to not include the "asus,rk3566- tinker-board-3" part, because I think it's pretty much redundant. [snip] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree 2025-11-14 2:26 ` Dragan Simic @ 2025-11-14 13:54 ` Michael Opdenacker 2025-11-14 14:30 ` Dragan Simic 0 siblings, 1 reply; 9+ messages in thread From: Michael Opdenacker @ 2025-11-14 13:54 UTC (permalink / raw) To: Dragan Simic Cc: michael.opdenacker, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel Hi Dragan, Thanks a lot for your review and feedback! On 11/14/25 03:26, Dragan Simic wrote: > Hello Michael, > > Thanks for this patch! Please, see some comments below. > > On Tuesday, November 11, 2025 18:20 CET, michael.opdenacker@rootcommit.com wrote: >> From: Michael Opdenacker <michael.opdenacker@rootcommit.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. > > 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). > >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-fastrhino-r66s.dtb >> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts >> new file mode 100644 >> index 000000000000..938af35b9004 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/rockchip/rk3566-tinker-3.dts >> @@ -0,0 +1,14 @@ >> +// SPDX-License-Identifier: (GPL-2.0 OR MIT) >> +/* >> + * Copyright (C) 2025 Michael Opdenacker <michael.opdenacker@rootcommit.com> >> + */ >> + >> +/dts-v1/; >> + >> +#include "rk3566-tinker-3.dtsi" > The same comment from above about the naming applies here (as well > as in other places), so we should have "rk3566-tinker-board-3.dtsi" > here instead. > >> + >> +/ { >> + model = "Rockchip RK3566 Asus Tinker Board 3"; > For consistency and to avoid redundancy, the "Rockchip RK3566" > part should be removed. Done. > >> + 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. >> + 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. What do you think? Thanks again, Cheers Michael. -- Michael Opdenacker Root Commit Embedded Linux Training and Consulting https://rootcommit.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree 2025-11-14 13:54 ` Michael Opdenacker @ 2025-11-14 14:30 ` Dragan Simic 0 siblings, 0 replies; 9+ messages in thread From: Dragan Simic @ 2025-11-14 14:30 UTC (permalink / raw) To: Michael Opdenacker Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel Hello Michael, On Friday, November 14, 2025 14:54 CET, Michael Opdenacker <michael.opdenacker@rootcommit.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@rootcommit.com wrote: > >> From: Michael Opdenacker <michael.opdenacker@rootcommit.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". ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-11-14 14:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20251111172003.2324525-1-michael.opdenacker@rootcommit.com>
2025-11-11 17:20 ` [PATCH 1/2] dt-bindings: arm: rockchip: Asus Tinkerboard 3 and 3S michael.opdenacker
2025-11-13 8:35 ` Krzysztof Kozlowski
2025-11-14 2:00 ` Dragan Simic
2025-11-11 17:20 ` [PATCH 2/2] arm64: dts: rockchip: add Tinkerboard 3 and 3S device tree michael.opdenacker
2025-11-13 22:35 ` Heiko Stübner
2025-11-14 14:31 ` Michael Opdenacker
2025-11-14 2:26 ` Dragan Simic
2025-11-14 13:54 ` Michael Opdenacker
2025-11-14 14:30 ` Dragan Simic
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).