linux-kernel - Re: [PATCH v6 3/3] arm64: dts: rockchip: add dts for Ariaboard Photonicat RK3568

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 for Android: free password hash cracker in your pocket

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

Message-ID: <9a8237c4-1ca2-4cce-b836-a3394692a144@classfun.cn>
Date: Mon, 13 Jan 2025 06:44:14 +0800
From: Junhao Xie <bigfoot@...ssfun.cn>
To: Chukun Pan <amadeus@....edu.cn>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
 Junhao Xie <bigfoot@...ssfun.cn>
Subject: Re: [PATCH v6 3/3] arm64: dts: rockchip: add dts for Ariaboard
 Photonicat RK3568

On 2025/1/12 16:20, Chukun Pan wrote:
> Hi,
>> +	/* pi6c pcie clock generator */
>> +	vcc3v3_pi6c: regulator-vcc3v3-pi6c {
>> +		compatible = "regulator-fixed";
>> +		enable-active-high;
>> +		gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
> 
> Although both gpio and gpios can be used here, I tend 
> to use gpio to keep it consistent with other parts.
> (vcc3v3_sd, vcc3v4_rf, vcc5v0_usb30_otg0 ...)

Yes, I overlooked it, I will change it to this:
gpio = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;

> 
>> +&mdio1 {
>> +	rgmii_phy1: ethernet-phy@0 {
> 
> Only one rgmii phy I think '1' is not needed?
> Or `phy@...ress: ethernet-phy@...ress {`

I used sgmii_phy0 rgmii_phy1 to name them, they are
indeed redundant, and sgmii is not in mainline yet,
I will change rgmii_phy1 to rgmii_phy.

> 
>> +		compatible = "ethernet-phy-ieee802.3-c22";
>> +		reg = <0x0>;
> 
> The phy address should be 3. Can you try it?

It seems to work fine when using address 0x3, I will modify it.

> 
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&eth1_phy_rst>;
> 
> Is pinctrl necessary here?

I will remove it.

The node now looks like this:
&mdio1 {
	rgmii_phy: ethernet-phy@3 {
		compatible = "ethernet-phy-ieee802.3-c22";
		reg = <0x3>;
		reset-assert-us = <20000>;
		reset-deassert-us = <100000>;
		reset-gpios = <&gpio4 RK_PC0 GPIO_ACTIVE_LOW>;
		rx-internal-delay-ps = <1500>;
		tx-internal-delay-ps = <1500>;
	};
};

> 
>> +&sdmmc1 {
>> +   ...
>> +	wifi: wifi@1 {
>> +   ...
>> +&uart1 {
>> +   ...
>> +	bluetooth: bluetooth {
> 
> No aliases needed.

I will remove it.

> 

Thanks for your review, I will fix all problems in next version!

Best regards,
Junhao