Message-ID: <54a76b31-5f69-4004-a030-0096a18a0de4@kwiboo.se>
Date: Mon, 14 Jul 2025 12:49:24 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Cc: Heiko Stuebner <heiko@...ech.de>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, linux-rockchip@...ts.infradead.org,
 Yao Zi <ziyao@...root.org>, Chukun Pan <amadeus@....edu.cn>,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/6] arm64: dts: rockchip: Add Radxa ROCK 2A/2F

Hi Nicolas,

On 7/14/2025 11:44 AM, Nicolas Frattaroli wrote:
> Hi Jonas,
> 
> see comments in-line.
> 
> On Saturday, 12 July 2025 19:37:44 Central European Summer Time Jonas Karlman wrote:
>> The ROCK 2A and ROCK 2F is a high-performance single board computer
>> developed by Radxa, based on the Rockchip RK3528A SoC.
>>
>> Add initial device tree for the Radxa ROCK 2A and ROCK 2F boards.
>>
>> Signed-off-by: Jonas Karlman <jonas@...boo.se>
>> ---
>> v3: Rename led nodes to led-0/led-1 (Chukun Pan)
>> v2: Limit sdmmc max-frequency to 100 MHz (Yao Zi)
>>
>> Schematics:
>> - https://dl.radxa.com/rock2/2a/v1.2/radxa_rock_2a_v1.2_schematic.pdf
>> - https://dl.radxa.com/rock2/2f/radxa_rock2f_v1.01_schematic.pdf
>> ---
>>  arch/arm64/boot/dts/rockchip/Makefile         |   2 +
>>  .../boot/dts/rockchip/rk3528-rock-2.dtsi      | 293 ++++++++++++++++++
>>  .../boot/dts/rockchip/rk3528-rock-2a.dts      |  82 +++++
>>  .../boot/dts/rockchip/rk3528-rock-2f.dts      |  10 +
>>  4 files changed, 387 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-rock-2.dtsi
>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-rock-2f.dts
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
>> index 099520962ffb..4cb6106b16f2 100644
>> --- a/arch/arm64/boot/dts/rockchip/Makefile
>> +++ b/arch/arm64/boot/dts/rockchip/Makefile
>> @@ -90,6 +90,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-sapphire.dtb
>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-sapphire-excavator.dtb
>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399pro-rock-pi-n10.dtb
>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3528-radxa-e20c.dtb
>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3528-rock-2a.dtb
>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3528-rock-2f.dtb
>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3562-evb2-v10.dtb
>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg-arc-d.dtb
>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg-arc-s.dtb
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-rock-2.dtsi b/arch/arm64/boot/dts/rockchip/rk3528-rock-2.dtsi
>> new file mode 100644
>> index 000000000000..aedc7ee9ee46
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3528-rock-2.dtsi
>> [...]
>> +
>> +&pinctrl {
>> +	bluetooth {
>> +		bt_wake_host_h: bt-wake-host-h {
>> +			rockchip,pins = <1 RK_PC2 RK_FUNC_GPIO &pcfg_pull_down>;
>> +		};
>> +
>> +		host_wake_bt_h: host-wake-bt-h {
>> +			rockchip,pins = <1 RK_PA5 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +	};
>> +
>> +	leds {
>> +		state_led_b: state-led-b {
>> +			rockchip,pins = <1 RK_PA3 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +	};
>> +
>> +	sdmmc {
>> +		sdmmc_vol_ctrl_h: sdmmc-vol-ctrl-h {
>> +			rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +	};
>> +
>> +	usb {
>> +		usb_host_en: usb-host-en {
>> +			rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +	};
>> +
>> +	wifi {
>> +		usb_wifi_pwr: usb-wifi-pwr {
>> +			rockchip,pins = <4 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +
>> +		wifi_reg_on_h: wifi-reg-on-h {
>> +			rockchip,pins = <1 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>;
> 
> I don't see an external pull-up or pull-down in the schematic. I'm not
> sure what the recommended practice is; should we have a pull direction
> set in the SoC's pin controller in these cases, or leave it floating?

This is at least what I have done for gpio output pins in the past, see
other defines for a wifi_reg_on_h in tree.

My (possible wrong) reasoning is that this pin is used as a gpio output
pin and the output value should determin high/low. For this perticiular
board the wifi_reg_on_h pin is referenced as shutdown-gpios in the
rfkill-gpio node, so when this is initialized the output value should be
set high/low and the internal pull value no longer has a real purpose.

Please correct me if my reasoning is wrong :-)

> 
>> +		};
>> +
>> +		wifi_wake_host_h: wifi-wake-host-h {
>> +			rockchip,pins = <1 RK_PA7 RK_FUNC_GPIO &pcfg_pull_down>;
>> +		};
>> +	};
>> +};
>> +
>> +&pwm1 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pwm1m0_pins>;
>> +	status = "okay";
>> +};
>> +
>> +&pwm2 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pwm2m0_pins>;
>> +	status = "okay";
>> +};
>> +
>> +&saradc {
>> +	vref-supply = <&vcc_1v8>;
>> +	status = "okay";
>> +};
>> +
>> +&sdhci {
>> +	bus-width = <8>;
>> +	cap-mmc-highspeed;
>> +	mmc-hs200-1_8v;
>> +	no-sd;
>> +	no-sdio;
>> +	non-removable;
>> +	vmmc-supply = <&vcc_3v3>;
>> +	vqmmc-supply = <&vcc_1v8>;
>> +	status = "okay";
>> +};
>> +
>> +&sdmmc {
>> +	bus-width = <4>;
>> +	cap-mmc-highspeed;
>> +	cap-sd-highspeed;
>> +	disable-wp;
>> +	max-frequency = <100000000>;
> 
> Any reason why we reduce this from the 150000000 in the SoC .dtsi?

Please see v1 of this series, Yao Zi reported issues when 150 MHz was
usued with a SD-card that works with 150 MHz on the Radxa E20C.

Vendor kernel seem to change drive strenght of clk pin, but that did not
improve the situation, so I changed it to use a lower rate for now.

> Using the frequency the SoC .dtsi uses seems to work for me on the
> ROCK 2A:
> 
>   [    0.347556] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
>   [    0.547362] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
>   [    0.814405] dwmmc_rockchip ffc30000.mmc: Successfully tuned phase to 248
>   [    0.815407] mmc1: new UHS-I speed SDR104 SDXC card at address aaaa
>   [    0.817030] mmcblk1: mmc1:aaaa SH64G 59.5 GiB
>   [    0.820943]  mmcblk1: p1 p2

Using 150 MHz also worked with my 2A/3F boards and the two SD-cards I
could test.

> 
>> +	sd-uhs-sdr104;
>> +	vmmc-supply = <&vcc_3v3>;
>> +	vqmmc-supply = <&vccio_sd>;
>> +	status = "okay";
>> +};
>> +
>> +&uart0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&uart0m0_xfer>;
>> +	status = "okay";
>> +};
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts b/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
>> new file mode 100644
>> index 000000000000..c03ae1dd3456
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3528-rock-2a.dts
>> [...]
>> +
>> +&pinctrl {
>> +	ethernet {
>> +		gmac1_rstn_l: gmac1-rstn-l {
>> +			rockchip,pins = <4 RK_PC2 RK_FUNC_GPIO &pcfg_pull_none>;
> 
> Same concern as with wifi-reg-on-h, there's no external pull resistor
> in the schematic.

Same resoning as above, this pin is used as the Ethernet PHY reset-gpios
pin. Here the bootloader (U-Boot) will also configure this pin with a
gpio output value to trigger a PHY reset.

I can change these to pull-down as that seem to be the reset value for
these two pins, if I understand the schematic and PinOut correctly, if
you have a strong opinion on that these defines are wrong.

Regards,
Jonas

> 
>> +		};
>> +	};
>> +
>> +	leds {
>> +		sys_led_g: sys-led-g {
>> +			rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +	};
>> +
>> +	usb {
>> +		usb_otg_en: usb-otg-en {
>> +			rockchip,pins = <1 RK_PC3 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +	};
>> +};
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-rock-2f.dts b/arch/arm64/boot/dts/rockchip/rk3528-rock-2f.dts
>> new file mode 100644
>> index 000000000000..3e2b9b685cb2
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3528-rock-2f.dts
>> @@ -0,0 +1,10 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +
>> +/dts-v1/;
>> +
>> +#include "rk3528-rock-2.dtsi"
>> +
>> +/ {
>> +	model = "Radxa ROCK 2F";
>> +	compatible = "radxa,rock-2f", "rockchip,rk3528";
>> +};
>>
> 
> Other than the indicated comments,
> 
> Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
> 
> ensured the pinctrls were correct, checked that they correspond to the
> GPIOs being used in the devices associated with them. Also checked the
> rock 2a schematic to verify that the vccio_sd switching setup is
> correct, i.e. that high is indeed 3.3V and low is 1.8V.
> 
> And:
> 
> Tested-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
> 
> test-booted on a ROCK 2A, tested that SD card shows up, made sure
> ethernet works as well, also ensured that the adc key works.
> 
> 

Powered by blists - more mailing lists