Re: [PATCH v5 3/3] riscv: dts: spacemit: Enable USB3.0/PCIe on OrangePi RV2

From: Yixun Lan <dlan@kernel.org>
To: Chukun Pan <amadeus@jmu.edu.cn>
Cc: gaohan@iscas.ac.cn, alex@ghiti.fr, aou@eecs.berkeley.edu,
	conor+dt@kernel.org, devicetree@vger.kernel.org,
	krzk+dt@kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	pjw@kernel.org, rabenda.cn@gmail.com, robh@kernel.org,
	spacemit@lists.linux.dev
Subject: Re: [PATCH v5 3/3] riscv: dts: spacemit: Enable USB3.0/PCIe on OrangePi RV2
Date: Fri, 3 Apr 2026 22:16:37 +0800	[thread overview]
Message-ID: <20260403141637-GKA1042809@kernel.org> (raw)
In-Reply-To: <20260403095036.231761-1-amadeus@jmu.edu.cn>

Hi Chukun,

On 17:50 Fri 03 Apr     , Chukun Pan wrote:
> Hi,
> 
> > +	pcie_vcc_3v3: regulator-pcie-vcc3v3 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "pcie_vcc3v3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		gpio = <&gpio K1_GPIO(116) GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> > +	};
> 
> I would like to maintain the same property order as the
> regulator below,
..
> and add vin-supply:
> 
I think Krzysztof will have objection on this, which isn't used by any
device, so not really useful, see similar comment for "reg_dc_in" here
https://lore.kernel.org/all/6530526f-59ca-4753-a068-46c62a1a1fed@kernel.org/

> ```
> 	pcie_vcc3v3: pcie-vcc3v3 {
please keep "regulator-" prefix, which is
 	pcie_vcc_3v3: regulator-pcie-vcc3v3 {

> 		compatible = "regulator-fixed";
> 		enable-active-high;
> 		gpios = <&gpio K1_GPIO(116) GPIO_ACTIVE_HIGH>;
> 		regulator-name = "pcie_vcc3v3";
> 		regulator-min-microvolt = <3300000>;
> 		regulator-max-microvolt = <3300000>;
> 		vin-supply = <&vcc_5v0>;
I'm not sure if there is any enforced rules on this? I can understand you
are trying to sort them in alphabet order.. but I would personally
prefer old way - in slightly logical order.. but I do have no strong
preference..

> 	};
> ```
> 
> > +&pcie1 {
> > +	vpcie3v3-supply = <&pcie_vcc_3v3>;
> 
> > +&pcie2 {
> > +	vpcie3v3-supply = <&pcie_vcc_3v3>;
> 
> I think vpcie3v3-supply is not needed here. [1]
You right, can you send a patch to fix the same issue in tree?
I was trying to fix vpcie3v3 warning, but realized not fixed all..

> 
> > +	hub_2_0: hub@1 {
> > +		compatible = "usb5e3,610";
> > +		reg = <0x1>;
> > +		peer-hub = <&hub_3_0>;
> > +		vdd-supply = <&vcc5v0_usb30>;
> 
> vdd-supply = <&vcc_5v0>;
right, I agree
> 
> Please refer to the schematic. [2]
> 
> [1] https://lore.kernel.org/linux-pci/u53qfrubgrcamiz35ox6lcdpp5bbzfwcsic466z5r6yyx6xz3n@c64nw2pegtfe/
> [2] https://drive.google.com/drive/folders/1pcI_U0C3VJKTCg8A1zj08CwNbohnONSR
> 
> Thanks,
> Chukun

-- 
Yixun Lan (dlan)