riscv: dts: spacemit: Add PMIC regulators usb pcie
Subsystems (3)
cover riscv: dts: spacemit: Add PMIC regulators usb pcie
This patch series enables the PMIC, voltage regulators, usb and pcie for the OrangePi rv2 board.
Han Gao (7):
riscv: dts: spacemit: Enable i2c8 adapter for OrangePi RV2
riscv: dts: spacemit: Define fixed regulators for OrangePi RV2
riscv: dts: spacemit: Define the P1 PMIC regulators for OrangePi RV2
riscv: dts: spacemit: Enable USB3.0 on OrangePi RV2
riscv: dts: spacemit: Update PMIC supply properties for OrangePi RV2
riscv: dts: spacemit: Add a PCIe regulator for OrangePi RV2
riscv: dts: spacemit: PCIe and PHY-related updates for OrangePi RV2
.../boot/dts/spacemit/k1-orangepi-rv2.dts | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+)
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
prerequisite-patch-id: 32d6acfcb4801407e38510b391407943e9a8ec82
prerequisite-patch-id: ae644485e1a8ead3fa7a087c8db9062c7acc14ff
prerequisite-patch-id: 605d537aa0f8387b3523786a875c76b9edfbcbb4
prerequisite-patch-id: faab9c820ac67de42c40f2c4578bb5b8753fcb71
prerequisite-patch-id: f9c4643cc99e5b9f1acc9daf1e679b95f81cee73
prerequisite-patch-id: 50138c8918218ea3b8650999bd558ed6350ed2f2
prerequisite-patch-id: f8f91e32361cc99c531170c39ed54cd1b3c5997d
prerequisite-patch-id: 475d1968c0c41895578809410c9bd8fe24997de4
prerequisite-patch-id: c40f1562f712f04578ef58472840d257a19919fe
prerequisite-patch-id: e9ef3af6c80187e6f05444704d13212d3beaf40a
prerequisite-patch-id: 7076909bb4103588b4761d175eca40bce470230f
prerequisite-patch-id: 0c859b4d131b3360875c795c6148c6176b55fb91
prerequisite-patch-id: 2ed98dc1ab0f5ed923cc252415c345dc8caf6f17
prerequisite-patch-id: 1be1a031763fac029076a768f012af31e455be66
prerequisite-patch-id: 21bb8387c946e050910440e7a7622305d46d946d
prerequisite-patch-id: f3bdc2c74b230663710086bd770a755d56cb8b9c
prerequisite-patch-id: ba6a7df5c7a2fa9ce0b8a72a9177e42a4dcdab01
prerequisite-patch-id: 33fd23112b55b5e6b1e276594826ee454afc1c7e
prerequisite-patch-id: 009a5aaff0768d90d148eb9d5d999c5098fd6d93
prerequisite-patch-id: 7e41df07cbce163a689f5dca246b98a0122d1610
Signature · 1 line
2.47.3
PATCH 1/7 riscv: dts: spacemit: Enable i2c8 adapter for OrangePi RV2
The adapter is used to access the SpacemiT P1 PMIC present in this board.
--- arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts | 6 ++++++ 1 file changed, 6 insertions(+)
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +6 -0
@@ -87,6 +87,12 @@ &pdma { status = "okay"; }; +&i2c8 { + pinctrl-0 = <&i2c8_cfg>; + pinctrl-names = "default"; + status = "okay"; +}; + &uart0 { pinctrl-names = "default"; pinctrl-0 = <&uart0_2_cfg>;
Signature · 1 line
2.47.3
PATCH 2/7 riscv: dts: spacemit: Define fixed regulators for OrangePi RV2
In reply to Han GaoDefine the DC power input and the 4v power as fixed regulator supplies.
--- .../boot/dts/spacemit/k1-orangepi-rv2.dts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +19 -0
@@ -23,6 +23,25 @@ chosen { stdout-path = "serial0"; }; + reg_dc_in: dc-in-12v { + compatible = "regulator-fixed"; + regulator-name = "dc_in_12v"; + regulator-min-microvolt = <12000000>; + regulator-max-microvolt = <12000000>; + regulator-boot-on; + regulator-always-on; + }; + + reg_vcc_4v: vcc-4v { + compatible = "regulator-fixed"; + regulator-name = "vcc_4v"; + regulator-min-microvolt = <4000000>; + regulator-max-microvolt = <4000000>; + regulator-boot-on; + regulator-always-on; + vin-supply = <®_dc_in>; + }; + leds { compatible = "gpio-leds";
Signature · 1 line
2.47.3
PATCH 3/7 riscv: dts: spacemit: Define the P1 PMIC regulators for OrangePi RV2
In reply to Han GaoDefine the SpacemiT P1 PMIC voltage regulators and their constraints.
The power management hardware design on the OrangePi RV2 is identical to the Banana Pi BPI-F3, so the DT Nodes were taken from k1-bananapi-f3.dts.
--- .../boot/dts/spacemit/k1-orangepi-rv2.dts | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+)
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +110 -0
@@ -110,6 +110,116 @@ &i2c8 { pinctrl-0 = <&i2c8_cfg>; pinctrl-names = "default"; status = "okay"; + + pmic@41 { + compatible = "spacemit,p1"; + reg = <0x41>; + interrupts = <64>; + vin-supply = <®_vcc_4v>; + + regulators { + buck1 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3450000>; + regulator-ramp-delay = <5000>; + regulator-always-on; + }; + + buck2 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3450000>; + regulator-ramp-delay = <5000>; + regulator-always-on; + }; + + buck3_1v8: buck3 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <1800000>; + regulator-ramp-delay = <5000>; + regulator-always-on; + }; + + buck4 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3300000>; + regulator-ramp-delay = <5000>; + regulator-always-on; + }; + + buck5 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3450000>; + regulator-ramp-delay = <5000>; + regulator-always-on; + }; + + buck6 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3450000>; + regulator-ramp-delay = <5000>; + regulator-always-on; + }; + + aldo1 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + regulator-boot-on; + }; + + aldo2 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + }; + + aldo3 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + }; + + aldo4 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + }; + + dldo1 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + regulator-boot-on; + }; + + dldo2 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + }; + + dldo3 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + }; + + dldo4 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + regulator-always-on; + }; + + dldo5 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + }; + + dldo6 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + regulator-always-on; + }; + + dldo7 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <3400000>; + }; + }; + }; }; &uart0 {
Signature · 1 line
2.47.3
PATCH 4/7 riscv: dts: spacemit: Enable USB3.0 on OrangePi RV2
In reply to Han GaoEnable the DWC3 USB 3.0 controller and its associated usbphy2 on the OrangePi RV2 board.
The board utilizes a Genesys Logic GL3523 hub, which requires two separate power supplies: one VBUS and one for hub itself. Add two GPIO-controlled fixed-regulators to manage this.
--- .../boot/dts/spacemit/k1-orangepi-rv2.dts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+)
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +46 -0
@@ -42,6 +42,25 @@ reg_vcc_4v: vcc-4v { vin-supply = <®_dc_in>; }; + usb3-vbus-5v { + compatible = "regulator-fixed"; + regulator-name = "USB30_VBUS"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + regulator-always-on; + gpio = <&gpio K1_GPIO(97) GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + + usb3_hub_5v: usb3-hub-5v { + compatible = "regulator-fixed"; + regulator-name = "USB30_HUB"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpio K1_GPIO(123) GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + leds { compatible = "gpio-leds";
@@ -227,3 +246,30 @@ &uart0 { pinctrl-0 = <&uart0_2_cfg>; status = "okay"; }; + +&usbphy2 { + status = "okay"; +}; + +&usb_dwc3 { + dr_mode = "host"; + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + hub_2_0: hub@1 { + compatible = "usb5e3,610"; + reg = <0x1>; + vdd-supply = <&usb3_hub_5v>; + peer-hub = <&hub_3_0>; + reset-gpios = <&gpio K1_GPIO(124) GPIO_ACTIVE_LOW>; + }; + + hub_3_0: hub@2 { + compatible = "usb5e3,620"; + reg = <0x2>; + vdd-supply = <&usb3_hub_5v>; + peer-hub = <&hub_2_0>; + reset-gpios = <&gpio K1_GPIO(124) GPIO_ACTIVE_LOW>; + }; +};
Signature · 1 line
2.47.3
PATCH 5/7 riscv: dts: spacemit: Update PMIC supply properties for OrangePi RV2
In reply to Han GaoUpdate individual supply properties in pmic "spacemit,p1" node to specify the board's power tree topology for OrangePi RV2.
--- arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +10 -2
@@ -134,7 +134,15 @@ pmic@41 { compatible = "spacemit,p1"; reg = <0x41>; interrupts = <64>; - vin-supply = <®_vcc_4v>; + vin1-supply = <®_vcc_4v>; + vin2-supply = <®_vcc_4v>; + vin3-supply = <®_vcc_4v>; + vin4-supply = <®_vcc_4v>; + vin5-supply = <®_vcc_4v>; + vin6-supply = <®_vcc_4v>; + aldoin-supply = <®_vcc_4v>; + dldoin1-supply = <&buck5>; + dldoin2-supply = <&buck5>; regulators { buck1 {
@@ -165,7 +173,7 @@ buck4 { regulator-always-on; }; - buck5 { + buck5: buck5 { regulator-min-microvolt = <500000>; regulator-max-microvolt = <3450000>; regulator-ramp-delay = <5000>;
Signature · 1 line
2.47.3
PATCH 6/7 riscv: dts: spacemit: Add a PCIe regulator for OrangePi RV2
In reply to Han GaoDefine a 3.3v fixed voltage regulator to be used by PCIe on the OrangePi RV2. On this platform, this regulator is always on.
--- arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts | 10 ++++++++++ 1 file changed, 10 insertions(+)
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +10 -0
@@ -23,6 +23,16 @@ chosen { stdout-path = "serial0"; }; + pcie_vcc_3v3: pcie-vcc3v3 { + compatible = "regulator-fixed"; + regulator-name = "PCIE_VCC3V3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + gpio = <&gpio K1_GPIO(116) GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + reg_dc_in: dc-in-12v { compatible = "regulator-fixed"; regulator-name = "dc_in_12v";
Signature · 1 line
2.47.3
PATCH 7/7 riscv: dts: spacemit: PCIe and PHY-related updates for OrangePi RV2
In reply to Han GaoDefine PCIe and PHY-related Device Tree nodes for the OrangePi RV2.
--- .../boot/dts/spacemit/k1-orangepi-rv2.dts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+)
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +36 -0
@@ -83,6 +83,12 @@ led1 { }; }; +&combo_phy { + pinctrl-names = "default"; + pinctrl-0 = <&pcie0_3_cfg>; + status = "okay"; +}; + ð0 { phy-handle = <&rgmii0>; phy-mode = "rgmii-id";
@@ -259,6 +265,36 @@ dldo7 { }; }; +&pcie1_phy { + pinctrl-names = "default"; + pinctrl-0 = <&pcie1_3_cfg>; + status = "okay"; +}; + +&pcie1_port { + phys = <&pcie1_phy>; +}; + +&pcie1 { + vpcie3v3-supply = <&pcie_vcc_3v3>; + status = "okay"; +}; + +&pcie2_phy { + pinctrl-names = "default"; + pinctrl-0 = <&pcie2_4_cfg>; + status = "okay"; +}; + +&pcie2_port { + phys = <&pcie2_phy>; +}; + +&pcie2 { + vpcie3v3-supply = <&pcie_vcc_3v3>; + status = "okay"; +}; + &uart0 { pinctrl-names = "default"; pinctrl-0 = <&uart0_2_cfg>;
Signature · 1 line
2.47.3
Quoted 2 lines · Han Gao
This patch series enables the PMIC, voltage regulators, usb and pcie for the OrangePi rv2 board.
Hi:
Thank you for sending this in. I am working on "b4 review" functionality and I am using arbitrary series to perform agent-assisted reviews to find bugs in the process.
This review was done by Claude Opus 4.5. I do not know enough about the subsystem to make any judgment about the quality of these reviews, but they look valid enough.
Hopefully, these were useful. If not, I am sorry for wasting your time.
Signature · 1 line
Konstantin Ryabitsev <[email protected]>
Re: [PATCH 3/7] riscv: dts: spacemit: Define the P1 PMIC regulators for OrangePi RV2
In reply to Han GaoQuoted 6 lines · Han Gao
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +0
-0
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts &i2c8 { · line 119
Quoted 10 lines
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +7
-0@@ -110,6 +110,116 @@ &i2c8 { pinctrl-0 = <&i2c8_cfg>; pinctrl-names = "default"; status = "okay"; + + pmic@41 { + compatible = "spacemit,p1"; + reg = <0x41>; + interrupts = <64>; + vin-supply = <®_vcc_4v>; +
This vin-supply is replaced by per-rail supplies (vin1-supply through vin6-supply, aldoin-supply, dldoin1-supply, dldoin2-supply) in patch 5/7. Consider using the final supply properties here directly to avoid an intermediate state that is immediately superseded.
Signature · 1 line
Konstantin Ryabitsev <[email protected]>
Re: [PATCH 5/7] riscv: dts: spacemit: Update PMIC supply properties for OrangePi RV2
In reply to Han Gaoarch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts pmic@41 { · line 138
Quoted 12 lines · Han Gao
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +2
-1@@ -134,7 +134,15 @@ pmic@41 { compatible = "spacemit,p1"; reg = <0x41>; interrupts = <64>; - vin-supply = <®_vcc_4v>; + vin1-supply = <®_vcc_4v>; + vin2-supply = <®_vcc_4v>;
These per-rail supply properties (vin1-supply through dldoin2-supply) are not declared in the spacemit,p1.yaml binding, which only has vin-supply. With unevaluatedProperties: false on the PMIC node, these will cause dtbs_check failures. The binding needs to be updated to match, or these should remain as the single vin-supply.
Signature · 1 line
Konstantin Ryabitsev <[email protected]>
Quoted 5 lines · Konstantin Ryabitsev
Quoted 2 lines · Han Gao
This patch series enables the PMIC, voltage regulators, usb and pcie for the OrangePi rv2 board.
Hi:
Quoted 7 lines
Thank you for sending this in. I am working on "b4 review" functionality and I am using arbitrary series to perform agent-assisted reviews to find bugs in the process.
This review was done by Claude Opus 4.5. I do not know enough about the subsystem to make any judgment about the quality of these reviews, but they look valid enough.
I'm not sure if you see this, but your bot gave two contradictory recommendations.
On patch 3 it says:
Quoted 4 lines
This vin-supply is replaced by per-rail supplies (vin1-supply through vin6-supply, aldoin-supply, dldoin1-supply, dldoin2-supply) in patch 5/7. Consider using the final supply properties here directly to avoid an intermediate state that is immediately superseded.
Whereas on patch 5 it says
Quoted 5 lines
These per-rail supply properties (vin1-supply through dldoin2-supply) are not declared in the spacemit,p1.yaml binding, which only has vin-supply. With unevaluatedProperties: false on the PMIC node, these will cause
dtbs_checkfailures. The binding needs to be updated to match, or these should remain as the single vin-supply.
"Use vin1-supply through dldoin2-supply, not vin-supply" and "use vin-supply only, not vin1-supply etc" can't both be correct, so obviously something weird is going on.
So what on earth is going on? Granted, Han could have described it better in his cover letter, but there's a proposed update to the spacemit,p1 bindings [1] that would have deprecated vin-supply and allowed vin1-supply etc. So there's a dependency in this series of something like:
patch 1,2 -> patch 3 -> bindings update [1] -> patch 5
If/when the bindings update is accepted, patch 5 can be accepted into the DTS tree. Until then, patch 5 should be withheld.
This isn't obvious from the patch itself, but still, the two recommendations are contradictory on their own, and make dtbs_check (mentioned in the patch 5 comment!) would have caught the problem with the patch 3 comment.
Quoted 1 line
Hopefully, these were useful. If not, I am sorry for wasting your time.
I acknowledge that this is a sample size of 1, but this is not making me feel inclined to trust the future of whatever "agent-assisted review" is going to be happening. What good is an "agent" if it knows about make dtbs_check but doesn't even run it to see what would happen to its suggested fixes?
I don't know what you have in mind for "b4 review", but I think at the very least, "b4 review" should not encourage "I do not know much about the subsystem but the LLM comments look valid" replies.
Regards,
Vivian "dramforever" Wang
Hi Han,
Quoted 3 lines · Han Gao
This patch series enables the PMIC, voltage regulators, usb and pcie for the OrangePi rv2 board.
It's obvious too late for v6.20, besides this series depend on P1 PMIC patch which haven't settled down.. Guodong is working on v3, see
https://lore.kernel.org/all/CAH1PCMYmK=cenJAvhGdZuh68fwNu-c_JJFpyztZUXvcOTv0YSg@mail.gmail.com/
Quoted 37 lines
Han Gao (7):
riscv: dts: spacemit: Enable i2c8 adapter for OrangePi RV2
riscv: dts: spacemit: Define fixed regulators for OrangePi RV2
riscv: dts: spacemit: Define the P1 PMIC regulators for OrangePi RV2
riscv: dts: spacemit: Enable USB3.0 on OrangePi RV2
riscv: dts: spacemit: Update PMIC supply properties for OrangePi RV2
riscv: dts: spacemit: Add a PCIe regulator for OrangePi RV2
riscv: dts: spacemit: PCIe and PHY-related updates for OrangePi RV2.../boot/dts/spacemit/k1-orangepi-rv2.dts | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+)base-commit:
8f0b4cce4481fb22653697cced8d0d04027cb1e8
prerequisite-patch-id:32d6acfcb4801407e38510b391407943e9a8ec82
prerequisite-patch-id:ae644485e1a8ead3fa7a087c8db9062c7acc14ff
prerequisite-patch-id:605d537aa0f8387b3523786a875c76b9edfbcbb4
prerequisite-patch-id:faab9c820ac67de42c40f2c4578bb5b8753fcb71
prerequisite-patch-id:f9c4643cc99e5b9f1acc9daf1e679b95f81cee73
prerequisite-patch-id:50138c8918218ea3b8650999bd558ed6350ed2f2
prerequisite-patch-id:f8f91e32361cc99c531170c39ed54cd1b3c5997d
prerequisite-patch-id:475d1968c0c41895578809410c9bd8fe24997de4
prerequisite-patch-id:c40f1562f712f04578ef58472840d257a19919fe
prerequisite-patch-id:e9ef3af6c80187e6f05444704d13212d3beaf40a
prerequisite-patch-id:7076909bb4103588b4761d175eca40bce470230f
prerequisite-patch-id:0c859b4d131b3360875c795c6148c6176b55fb91
prerequisite-patch-id:2ed98dc1ab0f5ed923cc252415c345dc8caf6f17
prerequisite-patch-id:1be1a031763fac029076a768f012af31e455be66
prerequisite-patch-id:21bb8387c946e050910440e7a7622305d46d946d
prerequisite-patch-id:f3bdc2c74b230663710086bd770a755d56cb8b9c
prerequisite-patch-id:ba6a7df5c7a2fa9ce0b8a72a9177e42a4dcdab01
prerequisite-patch-id:33fd23112b55b5e6b1e276594826ee454afc1c7e
prerequisite-patch-id:009a5aaff0768d90d148eb9d5d999c5098fd6d93
prerequisite-patch-id:7e41df07cbce163a689f5dca246b98a0122d1610Signature · 1 line
2.47.3
Signature · 1 line
Yixun Lan (dlan)
Re: [PATCH 2/7] riscv: dts: spacemit: Define fixed regulators for OrangePi RV2
In reply to Han GaoHi Han,
Quoted 12 lines · Han Gao
Define the DC power input and the 4v power as fixed regulator supplies.
Signed-off-by Han Gao <[email protected]>--- .../boot/dts/spacemit/k1-orangepi-rv2.dts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +0
-0
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts chosen { · line 26
Quoted 4 lines
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +1
-0@@ -23,6 +23,25 @@ chosen { stdout-path = "serial0"; }; + reg_dc_in: dc-in-12v {
this is copy & paste.. per discussion with Chukun, I'd suggest to add 'regulator-' prefix, see
Quoted 24 lines
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +18
-0+ compatible = "regulator-fixed"; + regulator-name = "dc_in_12v"; + regulator-min-microvolt = <12000000>; + regulator-max-microvolt = <12000000>; + regulator-boot-on; + regulator-always-on; + }; + + reg_vcc_4v: vcc-4v { + compatible = "regulator-fixed"; + regulator-name = "vcc_4v"; + regulator-min-microvolt = <4000000>; + regulator-max-microvolt = <4000000>; + regulator-boot-on; + regulator-always-on; + vin-supply = <®_dc_in>; + }; + leds { compatible = "gpio-leds";Signature · 1 line
2.47.3
Signature · 1 line
Yixun Lan (dlan)
Quoted 6 lines · Vivian Wang
Quoted 3 lines
This review was done by Claude Opus 4.5. I do not know enough about the subsystem to make any judgment about the quality of these reviews, but they look valid enough.
I'm not sure if you see this, but your bot gave two contradictory recommendations.
Criticism accepted. For the record, it's not a bot -- I'm writing a review tool that *may*, should the reviewer wish it, incorporate an LLM agent as a manual step.
My goal was to test end-to-end review functionality, but from now on I will focus on subsystems where I can actually discern actual useful reviews from "looks reasonable" reviews.
Thanks and my apologies,
-K
Re: [PATCH 6/7] riscv: dts: spacemit: Add a PCIe regulator for OrangePi RV2
In reply to Han GaoHi,
Quoted 2 lines
Define a 3.3v fixed voltage regulator to be used by PCIe on the OrangePi RV2. On this platform, this regulator is always on.
(...)
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts chosen { · line 32
Quoted 2 lines
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +2
-0+ regulator-always-on; + gpio = <&gpio K1_GPIO(116) GPIO_ACTIVE_HIGH>;
Why is this regulator always on?
It is clearly controlled by gpio.
Thanks,
Chukun
Re: [PATCH 7/7] riscv: dts: spacemit: PCIe and PHY-related updates for OrangePi RV2
In reply to Han GaoHi,
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts led1 { · line 90
Quoted 4 lines
+&combo_phy {
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +3
-0+ pinctrl-names = "default"; + pinctrl-0 = <&pcie0_3_cfg>; +};
This pinctrl is clearly incorrect (including BPI-F3). Checking the schematic reveals that these pins are for CAM_I2C. By the way, combo_phy connects to USB3, so it should be enabled in the USB3 patch.
Thanks,
Chukun
Yixun Lan
maintainerRe: [PATCH 7/7] riscv: dts: spacemit: PCIe and PHY-related updates for OrangePi RV2
Hi Chukun,
Quoted 9 lines · Chukun Pan
Hi,
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts led1 { · line 90
Quoted 4 lines
+&combo_phy {
arch/riscv/boot/dts/spacemit/k1-orangepi-rv2.dts +3
-0+ pinctrl-names = "default"; + pinctrl-0 = <&pcie0_3_cfg>; +};This pinctrl is clearly incorrect (including BPI-F3). Checking the schematic reveals that these pins are for
CAM_I2C.
I agree, if the phy is used for USB, the pinctrl info can be dropped Do you want to submit a patch for this?
Quoted 3 lines
By the way,
combo_phyconnects to USB3, so it should be enabled in the USB3 patch.
I think this is already done, check usb_dwc3 node in k1.dtsi?
https://github.com/torvalds/linux/blob/v7.0-rc3/arch/riscv/boot/dts/spacemit/k1.dtsi#L1180
Signature · 1 line
Yixun Lan (dlan)
Re: [PATCH 7/7] riscv: dts: spacemit: PCIe and PHY-related updates for OrangePi RV2
Hi,
Quoted 2 lines
I agree, if the phy is used for USB, the pinctrl info can be dropped Do you want to submit a patch for this?
I will submit a patch to drop the pinctrl.
Thanks,
Chukun