[PATCH v2 0/6] arm64: dts: rockchip: fix GMAC clocks and USB OTG power on RK3568 boards

[PATCH v2 0/6] arm64: dts: rockchip: fix GMAC clocks and USB OTG power on RK3568 boards

[Coia Prant]

Changes since v1:
- Patch 5 (USB OTG power): Reworked the power tree based on the schematic.
  Instead of tying usb2phy0_otg directly to the boost, model the OR-ed
  5V_OUT rail explicitly. Add vbat, 5v_boost, and 5v_out nodes, and fix
  the vin-supply of vcc5v0_usb30_otg0 to depend on 5v_out. Drop the
  incorrect phy-supply change on usb2phy0_otg.
- Patch 6 (modem power): New patch. Fix vcc3v4_rf vin-supply from
  vccin_5v to vcc_syson so the M.2 B-Key modem does not lose power
  when the board runs on battery.

===

This series fixes three classes of board-level issues found while
bringing up several RK3568 boards.

The first four patches address GMAC clock parents. On these boards the
DTS lists SCLK_GMACx in assigned-clocks but only provides a parent for
SCLK_GMACx_RX_TX, so SCLK_GMACx keeps whatever parent the bootloader or
the hardware reset left. Add CLK_MACx_2TOP explicitly so the GMAC main
clock always comes from the internal source. BPI-R2 Pro additionally
describes clock_in_out wrongly ("input" while the port is a fixed-link
with no external 125 MHz clock); switch it to "output" to match the
hardware. On RK3568 clock_in_out does not change the hardware clock
routing today, so that part is a description fix.

Patch 5 fixes USB OTG power on the Ariaboard Photonicat. On battery
power the USB port is dead because the 5V_OUT rail (which feeds the USB
load switch) relies on a boost converter that is not enabled. Add the
vbat, 5v_boost, and 5v_out regulator nodes to model the OR-ed power
path from the schematic, restoring USB host functionality on battery.

Patch 6 fixes the M.2 B-Key modem power supply on the same board. The
vcc3v4_rf regulator was incorrectly modeled as supplied by vccin_5v
(external DC), which means the modem would lose power on battery.
Correct it to vcc_syson per the schematic.
---
Coia Prant (6):
  arm64: dts: rockchip: fix GMAC clock parents on 9Tripod X3568 v4
  arm64: dts: rockchip: fix GMAC clock parents on Graperain G3568 v2
  arm64: dts: rockchip: fix GMAC clock setup on BPI-R2 Pro
  arm64: dts: rockchip: fix GMAC clock parent on Photonicat
  arm64: dts: rockchip: fix USB OTG power on Photonicat in battery mode
  arm64: dts: rockchip: fix modem power supply on Photonicat

 .../dts/rockchip/rk3568-9tripod-x3568-v4.dts  |  4 +-
 .../boot/dts/rockchip/rk3568-bpi-r2-pro.dts   |  4 +-
 .../rockchip/rk3568-graperain-g3568-v2.dts    |  4 +-
 .../boot/dts/rockchip/rk3568-photonicat.dts   | 53 +++++++++++++++++--
 4 files changed, 57 insertions(+), 8 deletions(-)

-- 
2.47.3

[PATCH v2 1/6] arm64: dts: rockchip: fix GMAC clock parents on 9Tripod X3568 v4

[Coia Prant]

Both GMAC nodes list SCLK_GMACx in assigned-clocks, but only
SCLK_GMACx_RX_TX gets a parent in assigned-clock-parents. SCLK_GMACx
therefore keeps whatever parent the bootloader or the hardware reset
left, which may or may not be the internal CLK_MACx_2TOP.

Add CLK_MACx_2TOP as the parent of SCLK_GMACx explicitly, so the GMAC
main clock always comes from the internal clock source and does not
depend on the bootloader state. The rate assignment is unchanged:
SCLK_GMACx_RX_TX stays at 0 (managed at runtime by
rk_set_clk_tx_rate()), SCLK_GMACx stays at 125 MHz.

Fixes: 156a530ed5ed ("arm64: dts: rockchip: Add devicetree for the 9Tripod X3568 v4")
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-9tripod-x3568-v4.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-9tripod-x3568-v4.dts b/arch/arm64/boot/dts/rockchip/rk3568-9tripod-x3568-v4.dts
index 4903e32d0ead0..d94bc34a31e67 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-9tripod-x3568-v4.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-9tripod-x3568-v4.dts
@@ -268,7 +268,7 @@ &cpu3 {
 
 &gmac0 {
 	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
-	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>;
 	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	pinctrl-names = "default";
@@ -284,7 +284,7 @@ &gmac0_rgmii_clk
 
 &gmac1 {
 	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
-	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
+	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
 	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	pinctrl-names = "default";
-- 
2.47.3

[PATCH v2 2/6] arm64: dts: rockchip: fix GMAC clock parents on Graperain G3568 v2

[Coia Prant]

Both GMAC nodes list SCLK_GMACx in assigned-clocks, but only
SCLK_GMACx_RX_TX gets a parent in assigned-clock-parents. SCLK_GMACx
therefore keeps whatever parent the bootloader or the hardware reset
left, which may or may not be the internal CLK_MACx_2TOP.

Add CLK_MACx_2TOP as the parent of SCLK_GMACx explicitly, so the GMAC
main clock always comes from the internal clock source and does not
depend on the bootloader state. The rate assignment is unchanged:
SCLK_GMACx_RX_TX stays at 0 (managed at runtime by
rk_set_clk_tx_rate()), SCLK_GMACx stays at 125 MHz.

Fixes: f296bb503a24 ("arm64: dts: rockchip: Add devicetree for the Graperain G3568 v2")
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-graperain-g3568-v2.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-graperain-g3568-v2.dts b/arch/arm64/boot/dts/rockchip/rk3568-graperain-g3568-v2.dts
index 4c55c7d211599..07cfc68b5f91b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-graperain-g3568-v2.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-graperain-g3568-v2.dts
@@ -280,7 +280,7 @@ &cpu3 {
 
 &gmac0 {
 	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
-	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>;
 	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	pinctrl-names = "default";
@@ -296,7 +296,7 @@ &gmac0_rgmii_clk
 
 &gmac1 {
 	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
-	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
+	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
 	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	pinctrl-names = "default";
-- 
2.47.3

[PATCH v2 3/6] arm64: dts: rockchip: fix GMAC clock setup on BPI-R2 Pro

[Coia Prant]

Both GMAC nodes rely on the internal 125 MHz GMAC main clock, but
neither pins its rate explicitly. Add assigned-clock-rates = <0>,
<125000000> so SCLK_GMACx is fixed at 125 MHz instead of whatever the
bootloader or the hardware reset left. The RGMII speed clock
(SCLK_GMACx_RX_TX) stays at 0, since it is managed at runtime by
rk_set_clk_tx_rate().

For gmac0, also switch clock_in_out from "input" to "output". The port
uses a fixed-link, and there is no external 125 MHz clock feeding the
MAC, so "input" described a clock source that is not wired up. Note
that on RK3568 clock_in_out does not change the hardware clock routing
today (rk3568_ops has no io_clksel masks), so this corrects the
description rather than the behaviour.

Fixes: f901aaadaa2a ("arm64: dts: rockchip: Add Bananapi R2 Pro")
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
index 4d3ebe50b90ba..677b6261a3a1e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
@@ -206,7 +206,8 @@ &combphy2 {
 &gmac0 {
 	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
 	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>;
-	clock_in_out = "input";
+	assigned-clock-rates = <0>, <125000000>;
+	clock_in_out = "output";
 	phy-mode = "rgmii";
 	pinctrl-names = "default";
 	pinctrl-0 = <&gmac0_miim
@@ -232,6 +233,7 @@ fixed-link {
 &gmac1 {
 	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
 	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
+	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	phy-handle = <&rgmii_phy1>;
 	phy-mode = "rgmii";
-- 
2.47.3

[PATCH v2 4/6] arm64: dts: rockchip: fix GMAC clock parent on Photonicat

[Coia Prant]

The RGMII WAN port on the Ariaboard Photonicat uses SCLK_GMAC1 as the
GMAC main clock and SCLK_GMAC1_RX_TX as the RGMII speed clock. Only the
parent of SCLK_GMAC1_RX_TX is set, so SCLK_GMAC1 keeps whatever parent
the bootloader or the hardware reset left.

Set the parent of SCLK_GMAC1 to CLK_MAC1_2TOP explicitly, so the GMAC
main clock always uses the internal source and does not depend on the
bootloader state. The rate assignment is unchanged: SCLK_GMAC1_RX_TX
keeps its rate unset, SCLK_GMAC1 stays at 125 MHz.

Fixes: ba124e06ce7b ("arm64: dts: rockchip: add dts for Ariaboard Photonicat RK3568")
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
index b719d5b661c43..7e0a2ab829209 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
@@ -285,7 +285,7 @@ &gmac0 {
 /* Motorcomm YT8521SC WAN port */
 &gmac1 {
 	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
-	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
+	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
 	assigned-clock-rates = <0>, <125000000>;
 	clock_in_out = "output";
 	phy-handle = <&rgmii_phy>;
-- 
2.47.3

[PATCH v2 5/6] arm64: dts: rockchip: fix USB OTG power on Photonicat in battery mode

[Coia Prant]

When the Ariaboard Photonicat runs on battery, the USB port has no power
output and attached devices are not detected. With the external supply
connected, the same port works fine.

Per the schematic, the 5V_OUT rail that feeds the USB load switch (U12,
enabled by USB_HOST_PWREN_H) is the OR-ed combination of two sources:
- VCCIN_5V (external DC input), OR-ed via D16.
- 5V_BOOST from the VBAT boost converter (U16, enabled by BOOST_EN /
  GPIO4_D2), OR-ed via D5002.

The 5V_OUT rail has no enable pin of its own. Since the regulator
framework can only model a single power supply dependency per
fixed-regulator, add a v5v_boost node for the VBAT boost converter, and
a v5v_out node to represent the OR-ed 5V_OUT rail. Bind the vin-supply
of v5v_out to v5v_boost to ensure VBAT is boosted when the system runs
on battery. When VCCIN_5V is present, D16 takes over.

The USB load switch (vcc5v0_usb30_otg0) is then supplied by v5v_out.
This restores USB host functionality on battery power, matching the
vendor firmware behaviour.

Link: https://github.com/immortalwrt/immortalwrt/issues/1534
Link: https://github.com/photonicat/photonicat-hardware/blob/main/Photonicat_rk3568_sch_20221116.pdf (Page 15,18)
Fixes: ba124e06ce7b ("arm64: dts: rockchip: add dts for Ariaboard Photonicat RK3568")
Cc: stable@vger.kernel.org
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 .../boot/dts/rockchip/rk3568-photonicat.dts   | 49 ++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
index 7e0a2ab829209..11ac78acca3be 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
@@ -59,6 +59,47 @@ hdmi_con_in: endpoint {
 		};
 	};
 
+	regulator_5v_boost: regulator-5v-boost {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&boost_en>;
+		regulator-name = "5v_boost";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vbat>;
+	};
+
+	/*
+	 * 5V_OUT is the OR-ed combination of the VBAT boost (via D5002)
+	 * and VCCIN_5V (via D16).
+	 * It has no enable pin of its own.
+	 * Model it as a fixed regulator depending on the boost converter,
+	 * so that VBAT is boosted when running on battery.
+	 *
+	 * 5V_OUT supplies:
+	 * - VCC5V0_USB30_OTG0 (USB VBUS via U12 load switch)
+	 * - VCC5V_HDMI (HDMI 5V via D5000)
+	 * - J53 expansion header (Pin 1)
+	 */
+	regulator_5v_out: regulator-5v-out {
+		compatible = "regulator-fixed";
+		regulator-name = "5v_out";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&regulator_5v_boost>;
+	};
+
+	vbat: regulator-vbat {
+		compatible = "regulator-fixed";
+		regulator-name = "vbat";
+		regulator-min-microvolt = <3400000>;
+		regulator-max-microvolt = <4200000>;
+		regulator-always-on;
+		regulator-boot-on;
+	};
+
 	vcc_1v8: regulator-vcc-1v8 {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc_1v8";
@@ -148,7 +189,7 @@ vcc5v0_usb30_otg0: regulator-vcc5v0-usb30-otg0 {
 		regulator-name = "vcc5v0_usb30_otg0";
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
-		vin-supply = <&vccin_5v>;
+		vin-supply = <&regulator_5v_boost>;
 	};
 
 	vccin_5v: regulator-vccin-5v {
@@ -446,6 +487,12 @@ pcie_pwren_h: pcie-pwren-h {
 		};
 	};
 
+	power {
+		boost_en: boost-en {
+			rockchip,pins = <4 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	sdmmc0 {
 		sdmmc0_pwren: sdmmc0-pwren {
 			rockchip,pins = <0 RK_PB6 RK_FUNC_GPIO &pcfg_pull_none>;
-- 
2.47.3

Re: [PATCH v2 5/6] arm64: dts: rockchip: fix USB OTG power on Photonicat in battery mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] arm64: dts: rockchip: unequal min/max microvolt on fixed regulator vbat
- [Medium] arm64: dts: rockchip: vcc5v0_usb30_otg0 bypasses unused regulator_5v_out
--

commit cf017fdefcec389cc0bb2c84974cfce9207e4762
Author: Coia Prant <coiaprant@gmail.com>

arm64: dts: rockchip: fix USB OTG power on Photonicat in battery mode

This commit adds fixed regulator nodes for the VBAT supply, the 5V boost
converter, and the 5V_OUT rail on the Ariaboard Photonicat RK3568, and
updates the USB OTG VBUS regulator input supply so USB host power works on
battery.

Link: https://github.com/immortalwrt/immortalwrt/issues/1534
Link: https://github.com/photonicat/photonicat-hardware/blob/main/Photonicat_rk3568_sch_20221116.pdf (Page 15,18)

> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> index e69f5e28e9e64..4d362f665031c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> @@ -51,6 +51,47 @@ hdmi_con_in: endpoint {
>  		};
>  	};
>  
> +	regulator_5v_boost: regulator-5v-boost {
> +		compatible = "regulator-fixed";
> +		enable-active-high;
> +		gpio = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&boost_en>;
> +		regulator-name = "5v_boost";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		vin-supply = <&vbat>;
> +	};
> +
> +	/*
> +	 * 5V_OUT is the OR-ed combination of the VBAT boost (via D5002)
> +	 * and VCCIN_5V (via D16).
> +	 * It has no enable pin of its own.
> +	 * Model it as a fixed regulator depending on the boost converter,
> +	 * so that VBAT is boosted when running on battery.
> +	 *
> +	 * 5V_OUT supplies:
> +	 * - VCC5V0_USB30_OTG0 (USB VBUS via U12 load switch)
> +	 * - VCC5V_HDMI (HDMI 5V via D5000)
> +	 * - J53 expansion header (Pin 1)
> +	 */
> +	regulator_5v_out: regulator-5v-out {
> +		compatible = "regulator-fixed";
> +		regulator-name = "5v_out";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		vin-supply = <&regulator_5v_boost>;
> +	};
> +
> +	vbat: regulator-vbat {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vbat";
> +		regulator-min-microvolt = <3400000>;
> +		regulator-max-microvolt = <4200000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};

[Severity: High]
Will the vbat regulator fail to probe here because regulator-min-microvolt
and regulator-max-microvolt do not match?

In drivers/regulator/fixed.c, reg_fixed_voltage_probe() calls
of_get_fixed_voltage_config(), which requires min_uV and max_uV to be equal
for a fixed regulator:

	if (init_data->constraints.min_uV == init_data->constraints.max_uV) {
		config->microvolts = init_data->constraints.min_uV;
	} else {
		dev_err(dev,
			 "Fixed regulator specified with variable voltages\n");
		return ERR_PTR(-EINVAL);
	}

When vbat fails with -EINVAL, regulator_resolve_supply() ->
of_regulator_dev_lookup() sees the regulator-vbat node present in the Device
Tree without a registered regulator device and permanently returns
-EPROBE_DEFER when resolving vin-supply = <&vbat> on regulator_5v_boost,
which cascades to vcc5v0_usb30_otg0 and the USB2 OTG PHYs (usb2phy0_otg and
usb2phy1_otg).

> +
>  	vcc_1v8: regulator-vcc-1v8 {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vcc_1v8";
> @@ -140,7 +181,7 @@ vcc5v0_usb30_otg0: regulator-vcc5v0-usb30-otg0 {
>  		regulator-name = "vcc5v0_usb30_otg0";
>  		regulator-min-microvolt = <5000000>;
>  		regulator-max-microvolt = <5000000>;
> -		vin-supply = <&vccin_5v>;
> +		vin-supply = <&regulator_5v_boost>;
>  	};

[Severity: Medium]
Should vcc5v0_usb30_otg0 reference <&regulator_5v_out> for its vin-supply
instead of <&regulator_5v_boost>, or is regulator_5v_out unnecessary?

Both the commit message and the comment above regulator_5v_out state that
5V_OUT supplies vcc5v0_usb30_otg0, but vcc5v0_usb30_otg0 points directly to
regulator_5v_boost, leaving regulator_5v_out without any consumers in
rk3568-photonicat.dts. (The commit message also refers to the nodes as
v5v_boost and v5v_out rather than regulator_5v_boost and regulator_5v_out.)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102171838.2958552-1-coiaprant@gmail.com?part=5

[PATCH v2 6/6] arm64: dts: rockchip: fix modem power supply on Photonicat

[Coia Prant]

The vcc3v4_rf regulator supplies the M.2 B-Key 5G modem. It was
incorrectly modeled as being supplied by vccin_5v (the external DC
input). As a result, the modem loses power when the board runs on
battery.

Per the schematic, VCC3V4_RF is actually derived from VCC_SYSON (the
main system power rail after the power switch) via a ferrite bead
(B1). Change the vin-supply of vcc3v4_rf to vcc_syson to match the
hardware topology. This ensures the modem remains powered in both DC
and battery modes.

Link: https://github.com/photonicat/photonicat-hardware/blob/main/Photonicat_rk3568_sch_20221116.pdf (Page 18)
Fixes: ba124e06ce7b ("arm64: dts: rockchip: add dts for Ariaboard Photonicat RK3568")
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
index 11ac78acca3be..02d445d7968fe 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
@@ -177,7 +177,7 @@ vcc3v4_rf: regulator-vcc3v4-rf {
 		regulator-name = "vcc3v4_rf";
 		regulator-min-microvolt = <3400000>;
 		regulator-max-microvolt = <3400000>;
-		vin-supply = <&vccin_5v>;
+		vin-supply = <&vcc_syson>;
 	};
 
 	vcc5v0_usb30_otg0: regulator-vcc5v0-usb30-otg0 {
-- 
2.47.3

Re: [PATCH v2 6/6] arm64: dts: rockchip: fix modem power supply on Photonicat

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] arm64: dts: rockchip: vbat min/max microvolt mismatch fails probe
- [Medium] arm64: dts: rockchip: vcc3v4_rf vin-supply change has no runtime effect
- [Medium] arm64: dts: rockchip: vcc5v0_usb30_otg0 leaves regulator_5v_out unused
--

commit e4c4248e8e73884f2860befcec1dafef98cd711c
Author: Coia Prant <coiaprant@gmail.com>

arm64: dts: rockchip: fix modem power supply on Photonicat

This commit changes the vin-supply of the vcc3v4_rf regulator from vccin_5v
to vcc_syson in the Ariaboard Photonicat RK3568 device tree to match the
hardware schematic and keep the M.2 5G modem powered on battery.

Link: https://github.com/photonicat/photonicat-hardware/blob/main/Photonicat_rk3568_sch_20221116.pdf (Page 18)

> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> index 4d362f665031c..6a51f97395741 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> @@ -169,7 +169,7 @@ vcc3v4_rf: regulator-vcc3v4-rf {
>  		regulator-name = "vcc3v4_rf";
>  		regulator-min-microvolt = <3400000>;
>  		regulator-max-microvolt = <3400000>;
> -		vin-supply = <&vccin_5v>;
> +		vin-supply = <&vcc_syson>;
>  	};

[Severity: Medium]
Does reparenting vcc3v4_rf from vccin_5v to vcc_syson actually change the
hardware power state when running on battery, as described in the commit
message?

In arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts, vccin_5v, vcc_sysin,
and vcc_syson are all regulator-fixed nodes marked regulator-always-on and
regulator-boot-on with no GPIO, clock, or power-domain control, and vcc_syson
chains directly back to vccin_5v:

	vccin_5v: regulator-vccin-5v {
		compatible = "regulator-fixed";
		regulator-name = "vccin_5v";
		regulator-always-on;
		regulator-boot-on;
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
	};

	vcc_sysin: regulator-vcc-sysin {
		compatible = "regulator-fixed";
		regulator-name = "vcc_sysin";
		regulator-always-on;
		regulator-boot-on;
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
		vin-supply = <&vccin_5v>;
	};

	vcc_syson: regulator-vcc-syson {
		compatible = "regulator-fixed";
		regulator-name = "vcc_syson";
		regulator-always-on;
		regulator-boot-on;
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
		vin-supply = <&vcc_sysin>;
	};

In reg_fixed_voltage_probe() in drivers/regulator/fixed.c, a fixed regulator
without a GPIO, clock, or PM domain uses empty fixed_voltage_ops and performs
no hardware switching.

Should the commit message clarify that this only updates the DT supply
hierarchy to match the schematic, or is there a controllable switch or boost
regulator needed to power the modem on battery?

>  
>  	vcc5v0_usb30_otg0: regulator-vcc5v0-usb30-otg0 {

[Severity: High]
In commit cf017fdefcec ("arm64: dts: rockchip: fix USB OTG power on Photonicat
in battery mode") earlier in this series, vcc5v0_usb30_otg0 was updated to
depend on regulator_5v_boost, which in turn depends on the newly added vbat
fixed regulator:

	regulator_5v_boost: regulator-5v-boost {
		...
		vin-supply = <&vbat>;
	};
	...
	vbat: regulator-vbat {
		compatible = "regulator-fixed";
		regulator-name = "vbat";
		regulator-min-microvolt = <3400000>;
		regulator-max-microvolt = <4200000>;
		regulator-always-on;
		regulator-boot-on;
	};

However, reg_fixed_voltage_probe() -> of_get_fixed_voltage_config() in
drivers/regulator/fixed.c requires regulator-min-microvolt and
regulator-max-microvolt to be equal for a regulator-fixed node:

	if (init_data->constraints.min_uV == init_data->constraints.max_uV) {
		config->microvolts = init_data->constraints.min_uV;
	} else {
		dev_err(dev,
			 "Fixed regulator specified with variable voltages\n");
		return ERR_PTR(-EINVAL);
	}

Won't vbat fail to probe with -EINVAL, causing regulator_5v_boost,
regulator_5v_out, vcc5v0_usb30_otg0, and the consumer USB2 OTG PHY nodes
(usb2phy0_otg and usb2phy1_otg) to permanently defer with -EPROBE_DEFER?

[Severity: Medium]
Also in arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts, the comment above
regulator_5v_out states that 5V_OUT supplies VCC5V0_USB30_OTG0:

	/*
	 * ...
	 * 5V_OUT supplies:
	 * - VCC5V0_USB30_OTG0 (USB VBUS via U12 load switch)
	 * - VCC5V_HDMI (HDMI 5V via D5000)
	 * - J53 expansion header (Pin 1)
	 */
	regulator_5v_out: regulator-5v-out {
		compatible = "regulator-fixed";
		regulator-name = "5v_out";
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
		vin-supply = <&regulator_5v_boost>;
	};

However, vcc5v0_usb30_otg0 sets vin-supply to regulator_5v_boost directly,
leaving regulator_5v_out completely unreferenced in the device tree:

	vcc5v0_usb30_otg0: regulator-vcc5v0-usb30-otg0 {
		...
		vin-supply = <&regulator_5v_boost>;
	};

Should vcc5v0_usb30_otg0 use vin-supply = <&regulator_5v_out> instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102171838.2958552-1-coiaprant@gmail.com?part=6

Re: [PATCH v2 6/6] arm64: dts: rockchip: fix modem power supply on Photonicat

[Coia Prant]

<sashiko-bot@kernel.org> 于2026年11月3日周二 01:26写道：
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] arm64: dts: rockchip: vbat min/max microvolt mismatch fails probe
> - [Medium] arm64: dts: rockchip: vcc3v4_rf vin-supply change has no runtime effect
> - [Medium] arm64: dts: rockchip: vcc5v0_usb30_otg0 leaves regulator_5v_out unused
> --
>
> commit e4c4248e8e73884f2860befcec1dafef98cd711c
> Author: Coia Prant <coiaprant@gmail.com>
>
> arm64: dts: rockchip: fix modem power supply on Photonicat
>
> This commit changes the vin-supply of the vcc3v4_rf regulator from vccin_5v
> to vcc_syson in the Ariaboard Photonicat RK3568 device tree to match the
> hardware schematic and keep the M.2 5G modem powered on battery.
>
> Link: https://github.com/photonicat/photonicat-hardware/blob/main/Photonicat_rk3568_sch_20221116.pdf (Page 18)
>
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> > index 4d362f665031c..6a51f97395741 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
> > @@ -169,7 +169,7 @@ vcc3v4_rf: regulator-vcc3v4-rf {
> >               regulator-name = "vcc3v4_rf";
> >               regulator-min-microvolt = <3400000>;
> >               regulator-max-microvolt = <3400000>;
> > -             vin-supply = <&vccin_5v>;
> > +             vin-supply = <&vcc_syson>;
> >       };
>
> [Severity: Medium]
> Does reparenting vcc3v4_rf from vccin_5v to vcc_syson actually change the
> hardware power state when running on battery, as described in the commit
> message?
>
> In arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts, vccin_5v, vcc_sysin,
> and vcc_syson are all regulator-fixed nodes marked regulator-always-on and
> regulator-boot-on with no GPIO, clock, or power-domain control, and vcc_syson
> chains directly back to vccin_5v:
>
>         vccin_5v: regulator-vccin-5v {
>                 compatible = "regulator-fixed";
>                 regulator-name = "vccin_5v";
>                 regulator-always-on;
>                 regulator-boot-on;
>                 regulator-min-microvolt = <5000000>;
>                 regulator-max-microvolt = <5000000>;
>         };
>
>         vcc_sysin: regulator-vcc-sysin {
>                 compatible = "regulator-fixed";
>                 regulator-name = "vcc_sysin";
>                 regulator-always-on;
>                 regulator-boot-on;
>                 regulator-min-microvolt = <5000000>;
>                 regulator-max-microvolt = <5000000>;
>                 vin-supply = <&vccin_5v>;
>         };
>
>         vcc_syson: regulator-vcc-syson {
>                 compatible = "regulator-fixed";
>                 regulator-name = "vcc_syson";
>                 regulator-always-on;
>                 regulator-boot-on;
>                 regulator-min-microvolt = <5000000>;
>                 regulator-max-microvolt = <5000000>;
>                 vin-supply = <&vcc_sysin>;
>         };
>
> In reg_fixed_voltage_probe() in drivers/regulator/fixed.c, a fixed regulator
> without a GPIO, clock, or PM domain uses empty fixed_voltage_ops and performs
> no hardware switching.
>
> Should the commit message clarify that this only updates the DT supply
> hierarchy to match the schematic, or is there a controllable switch or boost
> regulator needed to power the modem on battery?
>
> >
> >       vcc5v0_usb30_otg0: regulator-vcc5v0-usb30-otg0 {
>
> [Severity: High]
> In commit cf017fdefcec ("arm64: dts: rockchip: fix USB OTG power on Photonicat
> in battery mode") earlier in this series, vcc5v0_usb30_otg0 was updated to
> depend on regulator_5v_boost, which in turn depends on the newly added vbat
> fixed regulator:
>
>         regulator_5v_boost: regulator-5v-boost {
>                 ...
>                 vin-supply = <&vbat>;
>         };
>         ...
>         vbat: regulator-vbat {
>                 compatible = "regulator-fixed";
>                 regulator-name = "vbat";
>                 regulator-min-microvolt = <3400000>;
>                 regulator-max-microvolt = <4200000>;
>                 regulator-always-on;
>                 regulator-boot-on;
>         };
>
> However, reg_fixed_voltage_probe() -> of_get_fixed_voltage_config() in
> drivers/regulator/fixed.c requires regulator-min-microvolt and
> regulator-max-microvolt to be equal for a regulator-fixed node:
>
>         if (init_data->constraints.min_uV == init_data->constraints.max_uV) {
>                 config->microvolts = init_data->constraints.min_uV;
>         } else {
>                 dev_err(dev,
>                          "Fixed regulator specified with variable voltages\n");
>                 return ERR_PTR(-EINVAL);
>         }
>
> Won't vbat fail to probe with -EINVAL, causing regulator_5v_boost,
> regulator_5v_out, vcc5v0_usb30_otg0, and the consumer USB2 OTG PHY nodes
> (usb2phy0_otg and usb2phy1_otg) to permanently defer with -EPROBE_DEFER?
>
> [Severity: Medium]
> Also in arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts, the comment above
> regulator_5v_out states that 5V_OUT supplies VCC5V0_USB30_OTG0:
>
>         /*
>          * ...
>          * 5V_OUT supplies:
>          * - VCC5V0_USB30_OTG0 (USB VBUS via U12 load switch)
>          * - VCC5V_HDMI (HDMI 5V via D5000)
>          * - J53 expansion header (Pin 1)
>          */
>         regulator_5v_out: regulator-5v-out {
>                 compatible = "regulator-fixed";
>                 regulator-name = "5v_out";
>                 regulator-min-microvolt = <5000000>;
>                 regulator-max-microvolt = <5000000>;
>                 vin-supply = <&regulator_5v_boost>;
>         };
>
> However, vcc5v0_usb30_otg0 sets vin-supply to regulator_5v_boost directly,
> leaving regulator_5v_out completely unreferenced in the device tree:
>
>         vcc5v0_usb30_otg0: regulator-vcc5v0-usb30-otg0 {
>                 ...
>                 vin-supply = <&regulator_5v_boost>;
>         };
>
> Should vcc5v0_usb30_otg0 use vin-supply = <&regulator_5v_out> instead?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261102171838.2958552-1-coiaprant@gmail.com?part=6

Hi,

Thanks for the review. All points are valid and I will address them in v3.

For the High and Medium issues in patch 5:
1. vbat min/max is now fixed to 3700000 to satisfy the fixed-regulator
constraint (min == max) and avoid the EPROBE_DEFER cascade.
2. vcc5v0_usb30_otg0 now references <&regulator_5v_out> as its
vin-supply, making the 5v_out node functional. The commit message node
names are also aligned with the DTS labels.

For patch 6:
You are right that reparenting vcc3v4_rf has no runtime hardware
effect since both rails are always-on fixed regulators. This patch is
purely a DT topology fix to match the schematic (VCC3V4_RF is derived
from VCC_SYSON via a ferrite bead). I have updated the commit message
to clarify that it corrects the hardware description rather than
fixing a runtime power-cutoff issue.

Coia