linux-kernel - Re: [PATCH v1 2/2] arm64: dts: rockchip: add DTs for 100ASK DShanPi A1

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] [day] [month] [year] [list]

Message-Id: <20250823105001.970560-1-amadeus@jmu.edu.cn>
Date: Sat, 23 Aug 2025 18:50:01 +0800
From: Chukun Pan <amadeus@....edu.cn>
To: i@...insx.cn
Cc: 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,
	Chukun Pan <amadeus@....edu.cn>
Subject: Re: [PATCH v1 2/2] arm64: dts: rockchip: add DTs for 100ASK DShanPi A1

Hi,

> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/pwm/pwm.h>

It seems that pwm is not used?

> +	vcc_12v0_dcin: regulator-vcc-12v0-dcin {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_12v0_dcin";
> +		regulator-always-on;
> +		regulator-boot-on;

> +	vcc_1v1_nldo_s3: regulator-vcc-1v1-nldo-s3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_1v1_nldo_s3";
> +		regulator-boot-on;
> +		regulator-always-on;

The order of properties should be consistent, e.g.
```
		regulator-name = ...;
		regulator-always-on;
		regulator-boot-on;
```

> +	vcc3v3_pcie: regulator-vcc3v3-pcie {
> +		compatible = "regulator-fixed";
> +		enable-active-high;
> +		gpio = <&gpio1 RK_PB7 GPIO_ACTIVE_HIGH>;

`gpios = ...`

> +&combphy1_psu {
> +	status = "okay";
> +};
> +
> +&combphy0_ps {
> +	status = "okay";
> +};

combphy0 should be above combphy1

> +&gmac0 {
> +	phy-mode = "rgmii-id";
> +	clock_in_out = "output";

clock_in_out should be above phy-mode

> +&gmac1 {
> +	phy-mode = "rgmii-id";
> +	clock_in_out = "output";

Same here.

> ...
> +&i2c1 {
> +	status = "okay";
> +
> +	pmic@23 {
> ...
> +		gpio-controller;
> +		#gpio-cells = <2>;

gpio-xxx should be above interrupt

> +		regulators {
> +			vdd_cpu_big_s0: dcdc-reg1 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <950000>;
> +				regulator-ramp-delay = <12500>;
> +				regulator-name = "vdd_cpu_big_s0";
> +				regulator-enable-ramp-delay = <400>;

It would be better to add a blank line here.

> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> ...

> +&i2c4 {
> ...
> +		#sound-dai-cells = <0>;

This should be put last.

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sai1m0_mclk>;
> +	};
> +};
> ...

> +&pinctrl {
> +	gmac {
> +		gmac0_rst: gmac0-rst {
> +			rockchip,pins = <0 RK_PC2 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};

It would be better to add a blank line here.

> +		gmac1_rst: gmac1-rst {
> +			rockchip,pins = <0 RK_PC5 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> ...

> +&sdmmc {
> +	bus-width = <4>;
> +	cap-mmc-highspeed;
> +	cap-sd-highspeed;
> +	disable-wp;
> +	max-frequency = <200000000>;
> +	no-sdio;
> +	no-mmc;

cap-mmc-highspeed and no-mmc are mutually exclusive.
And the sdmmc controller supports sdio devices.

> +&u2phy0_otg {
> +	status = "okay";

Maybe lack of phy-supply?
And the usb node is not enabled?

--
2.25.1