linux-kernel - Re: [PATCH 3/3] arm64: dts: rockchip: Add Radxa E24C

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: <20250728125015.988357-1-amadeus@jmu.edu.cn>
Date: Mon, 28 Jul 2025 20:50:15 +0800
From: Chukun Pan <amadeus@....edu.cn>
To: jonas@...boo.se
Cc: amadeus@....edu.cn,
	conor+dt@...nel.org,
	devicetree@...r.kernel.org,
	heiko@...ech.de,
	krzk+dt@...nel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	linux-rockchip@...ts.infradead.org,
	robh@...nel.org,
	ziyao@...root.org
Subject: Re: [PATCH 3/3] arm64: dts: rockchip: Add Radxa E24C

Hi,

> +	avddl_1v1: avddh_3v3: avdd_rtl8367rb: regulator-avdd-rtl8367rb {
> +		compatible = "regulator-fixed";
> +		enable-active-high;
> +		gpios = <&gpio1 RK_PC3 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&gpio_8367_en>;
> +		regulator-name = "avdd_rtl8367rb";

I don't see the avdd_rtl8367rb regulator in the schematics. It looks like
DVDDIO (RTL8367RB power) is connected to AVDDH_3V3 via a magnetic bead.

> +&gmac1 {
> +	clock_in_out = "output";
> +	phy-mode = "rgmii-id";
> +	phy-supply = <&avdd_rtl8367rb>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&rgmii_miim>, <&rgmii_tx_bus2>, <&rgmii_rx_bus2>,
> +		    <&rgmii_rgmii_clk>, <&rgmii_rgmii_bus>, <&gmac1_rstn_l>;

Should the pinctrl of gmac1_rstn_l be written together with the
reset-gpios of the rtl8367rb switch?

```
reset-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_LOW>;
pinctrl-0 = <&gmac1_rstn_l>;
```

> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0m0_xfer>;
> +	status = "okay";
> +
> +	rk805: pmic@18 {
> +		compatible = "rockchip,rk805";
> +		reg = <0x18>;
> +		interrupt-parent = <&gpio4>;
> +		interrupts = <RK_PB2 IRQ_TYPE_LEVEL_LOW>;
> +		#clock-cells = <1>;
> +		clock-output-names = "rk805-clkout1", "rk805-clkout2";

The clkout pin is not connected, but the dt-bindings require it.
Maybe clock-output-names could be made optional?

+&mdio1 {
+	reset-delay-us = <25000>;
+	reset-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_LOW>;
+	reset-post-delay-us = <100000>;
+};

I don't think this is correct, reset-gpios should be written on the
rtl8365mb switch node. The switch driver has defined the reset time.

```
&mdio1 {
	switch@29 {
		compatible = "realtek,rtl8365mb";
		reg = <29>;
		reset-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_LOW>;
```

Thanks,
Chukun

--
2.25.1