Typc fusb302 powerloss issue on Radxa Rock 5b
Subsystems (4)
cover Typc fusb302 powerloss issue on Radxa Rock 5b
Hi All
On the Radxa Rock 5B, the system occasionally experiences intermittent hard resets during the boot process. Initially, I suspected a power supply issue, but further investigation points to the Type-C fusb302 module as the cause.
Specifically, probing or reloading the fusb302 module triggers a hard reset, which can result in immediate power loss and a reboot.
[root@rockpi-5b ~]# rmmod fusb302
[root@rockpi-5b ~]# lsmod | grep fusb302
[root@rockpi-5b ~]# modprobe fusb302
[root@rockpi-5b ~]# [ 3389.031608][ T7143] typec_fusb302 4-0022: Initiating hard-reset, which might result in machine power-loss.
[ 3390.030444][ T7143] typec_fusb302 4-0022: Initiating hard-reset, which might result in machine power-loss.
I attempted to trace the issue using ftrace but was unable to pinpoint the root cause. The problem appears to originate either from the I2C controller or the PMIC reset.
I have identified a potential workaround involving the I2C SCL debounce settings for the RK3588 and submitted a patch here:
[1] https://lore.kernel.org/all/[email protected]/
Please note that the submitted changes address a minor aspect but do not fully resolve the underlying issue.
Thanks
-Anand
Anand Moon (3):
arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Correct Type-C pin bias
settings
arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Fix USB host phy-supply
on Rock 5b-5bp-5t SbC
usb: typec: fusb302: Switch to threaded interrupt handler
arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi | 7 +++---- drivers/usb/typec/tcpm/fusb302.c | 7 ++++--- 2 files changed, 7 insertions(+), 7 deletions(-)
base-commit: 805f9a061372164d43ddef771d7cd63e3ba6d845
Signature · 1 line
2.50.1
PATCH 1/3 arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Correct Type-C pin bias settings
As pre FUSB302 datasheet interrupt line (INT_N) is an open-drain, active-low signal. It requires a pull-up resistor to maintain a stable high state when deasserted. Similarly, the TYPEC5V_PWREN_H enable signal requires a pull-down resistor to ensure it defaults to a low state, preventing unintended power delivery during the boot sequence.
Update the pinctrl entries to use pcfg_pull_up for usbc0_int and pcfg_pull_down for vbus5v0_typec_en to align with the hardware's electrical requirements.
---
v1: As per the shematics CC_INT_L interrupt pin is GPIO3_B4_u
As per the shematics TYPEC5V_PWREN_H pin is GPIO2_B6_d
--- arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi +2 -2
@@ -537,11 +537,11 @@ pcie3_vcc3v3_en: pcie3-vcc3v3-en { usb { usbc0_int: usbc0-int { - rockchip,pins = <3 RK_PB4 RK_FUNC_GPIO &pcfg_pull_none>; + rockchip,pins = <3 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up>; }; vbus5v0_typec_en: vbus5v0-typec-en { - rockchip,pins = <2 RK_PB6 RK_FUNC_GPIO &pcfg_pull_none>; + rockchip,pins = <2 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>; }; }; };
Signature · 1 line
2.50.1
Re: [PATCH v1 1/3] arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Correct Type-C pin bias settings
Hi,
Quoted 10 lines · Anand Moon
As pre FUSB302 datasheet interrupt line (
INT_N) is an open-drain, active-low signal. It requires a pull-up resistor to maintain a stable high state when deasserted. Similarly, theTYPEC5V_PWREN_Henable signal requires a pull-down resistor to ensure it defaults to a low state, preventing unintended power delivery during the boot sequence.Update the pinctrl entries to use
pcfg_pull_upforusbc0_intandpcfg_pull_downforvbus5v0_typec_ento align with the hardware's electrical requirements.
Quoted 7 lines
Cc Sebastian Reichel <[email protected]>Fixes 67b2c15d8fb3 ("arm64: dts: rockchip: add USB-C support for ROCK 5B/5B+/5T")Signed-off-by Anand Moon <[email protected]>---
v1: As per the shematicsCC_INT_Linterrupt pin isGPIO3_B4_u
As per the shematicsTYPEC5V_PWREN_Hpin isGPIO2_B6_d
---
Checking the schematics:
5B v1.45 - CC_INT_L - R2613 10K pull-up resistor
5B v1.45 - TYPEC5V_PWREN_H - GPIO is effectively unused because R95035 is NC
5B+ v1.2 - CC_INT_L - R2613 10K pull-up resistor 5B+ v1.2 - TYPEC5V_PWREN_H - R163 100K pull-down resistor
5T v1.2 - CC_INT_L - R2613 10K pull-up resistor 5T v1.2 - TYPEC5V_PWREN_H - R163 100K pull-down resistor
TLDR: All GPIOs have pull resistors in discrete hardware and do not need them muxed in the SoC.
Greetings,
-- Sebastian
Quoted 24 lines
arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi +2
-2@@ -537,11 +537,11 @@ pcie3_vcc3v3_en: pcie3-vcc3v3-en { usb { usbc0_int: usbc0-int { - rockchip,pins = <3 RK_PB4 RK_FUNC_GPIO &pcfg_pull_none>; + rockchip,pins = <3 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up>; }; vbus5v0_typec_en: vbus5v0-typec-en { - rockchip,pins = <2 RK_PB6 RK_FUNC_GPIO &pcfg_pull_none>; + rockchip,pins = <2 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>; }; }; };Signature · 1 line
2.50.1
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH v1 1/3] arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Correct Type-C pin bias settings
Hi Sebastian,
Thanks for your review comments.
Quoted 24 lines · Sebastian Reichel
Hi,
Quoted 10 lines · Anand Moon
As pre FUSB302 datasheet interrupt line (
INT_N) is an open-drain, active-low signal. It requires a pull-up resistor to maintain a stable high state when deasserted. Similarly, theTYPEC5V_PWREN_Henable signal requires a pull-down resistor to ensure it defaults to a low state, preventing unintended power delivery during the boot sequence.Update the pinctrl entries to use
pcfg_pull_upforusbc0_intandpcfg_pull_downforvbus5v0_typec_ento align with the hardware's electrical requirements.Quoted 7 lines
Cc Sebastian Reichel <[email protected]>Fixes 67b2c15d8fb3 ("arm64: dts: rockchip: add USB-C support for ROCK 5B/5B+/5T")Signed-off-by Anand Moon <[email protected]>---
v1: As per the shematicsCC_INT_Linterrupt pin isGPIO3_B4_u
As per the shematicsTYPEC5V_PWREN_Hpin isGPIO2_B6_d
---Checking the schematics:
Quoted 9 lines
5B v1.45 -
CC_INT_L- R2613 10K pull-up resistor
5B v1.45 -TYPEC5V_PWREN_H- GPIO is effectively unused because R95035 is NC5B+ v1.2 -
CC_INT_L- R2613 10K pull-up resistor 5B+ v1.2 -TYPEC5V_PWREN_H- R163 100K pull-down resistor5T v1.2 -
CC_INT_L- R2613 10K pull-up resistor 5T v1.2 -TYPEC5V_PWREN_H- R163 100K pull-down resistor
CC_INT_L is gpio interrupt pin to enable the I2C read operation.
As per FUSB302 datasheet, here is the updated version of the commit message
Configure CC_INT_L as an active-low, open-drain output. Per the hardware design, this pin utilizes an external pull-up and is driven LOW by the Type-C controller to
signal the processor to perform I2C register reads.
The TYPEC5V_PWREN_H pull-down resistor is turned on as a signal to the Type-C regulator, which has an active-enable-high property. In this context, the regulator is used to set the polarity of the GPIO used to enable or disable the regulator.
Quoted 3 lines
TLDR: All GPIOs have pull resistors in discrete hardware and do not need them muxed in the SoC.
It depends on the GPIO, pinctrl PMU configuration.
I was thinking of converting the vbus5v0_typec regulator to 'regulator-gpio.' to better reflect the hardware schematics. The TYPEC5V_PWREN_H signal (GPIO2_B6_d) acts as a state controller rather than a simple enable pin, and this change ensures the device tree aligns with that design
[1] https://dl.radxa.com/rock5/5b/docs/hw/radxa_rock_5b_v1450_schematic.pdf
(page 28)
$ git diff arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi
(quoted diff) +0 -0
b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi index 5f58e339a052..387ff009ec76 100644
arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi +3 -2
@@ -57,9 +57,10 @@ rfkill-bt { }; vbus5v0_typec: vbus5v0-typec { - compatible = "regulator-fixed"; + compatible = "regulator-gpio"; enable-active-high; - gpio = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>; + gpios = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>; + states = <5000000 0x1>, <0 0x0>; pinctrl-names = "default"; pinctrl-0 = <&vbus5v0_typec_en>; regulator-name = "vbus5v0_typec";
Could you please try this at your end?
Thanks
-Anand
Re: [PATCH v1 1/3] arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Correct Type-C pin bias settings
Hi,
Quoted 48 lines · Anand Moon
Hi Sebastian,
Thanks for your review comments.
Quoted 24 lines · Sebastian Reichel
Hi,
Quoted 10 lines · Anand Moon
As pre FUSB302 datasheet interrupt line (
INT_N) is an open-drain, active-low signal. It requires a pull-up resistor to maintain a stable high state when deasserted. Similarly, theTYPEC5V_PWREN_Henable signal requires a pull-down resistor to ensure it defaults to a low state, preventing unintended power delivery during the boot sequence.Update the pinctrl entries to use
pcfg_pull_upforusbc0_intandpcfg_pull_downforvbus5v0_typec_ento align with the hardware's electrical requirements.Quoted 7 lines
Cc Sebastian Reichel <[email protected]>Fixes 67b2c15d8fb3 ("arm64: dts: rockchip: add USB-C support for ROCK 5B/5B+/5T")Signed-off-by Anand Moon <[email protected]>---
v1: As per the shematicsCC_INT_Linterrupt pin isGPIO3_B4_u
As per the shematicsTYPEC5V_PWREN_Hpin isGPIO2_B6_d
---Checking the schematics:
Quoted 9 lines
5B v1.45 -
CC_INT_L- R2613 10K pull-up resistor
5B v1.45 -TYPEC5V_PWREN_H- GPIO is effectively unused because R95035 is NC5B+ v1.2 -
CC_INT_L- R2613 10K pull-up resistor 5B+ v1.2 -TYPEC5V_PWREN_H- R163 100K pull-down resistor5T v1.2 -
CC_INT_L- R2613 10K pull-up resistor 5T v1.2 -TYPEC5V_PWREN_H- R163 100K pull-down resistor
CC_INT_Lis gpio interrupt pin to enable the I2C read operation.As per FUSB302 datasheet, here is the updated version of the commit message
Configure
CC_INT_Las an active-low, open-drain output. Per the hardware design, this pin utilizes an external pull-up and is driven LOW by the Type-C controller to
signal the processor to perform I2C register reads.
Quoted 4 lines
The
TYPEC5V_PWREN_Hpull-down resistor is turned on as a signal to the Type-C regulator, which has an active-enable-high property. In this context, the regulator is used to set the polarity of the GPIO used to enable or disable the regulator.
I don't understand what you want to say here.
Quoted 4 lines
Quoted 3 lines
TLDR: All GPIOs have pull resistors in discrete hardware and do not need them muxed in the SoC.
It depends on the GPIO, pinctrl PMU configuration.
as I try to say it does not depend on pinctrl configuration for any of the GPIOs you are touching. The pull resistors for them exist as components on the board.
Quoted 28 lines
I was thinking of converting the
vbus5v0_typecregulator to 'regulator-gpio.' to better reflect the hardware schematics. TheTYPEC5V_PWREN_Hsignal (GPIO2_B6_d) acts as a state controller rather than a simple enable pin, and this change ensures the device tree aligns with that design[1] https://dl.radxa.com/rock5/5b/docs/hw/radxa_rock_5b_v1450_schematic.pdf
(page 28)$ git diff arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsiarch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi +3
-2@@ -57,9 +57,10 @@ rfkill-bt { }; vbus5v0_typec: vbus5v0-typec { - compatible = "regulator-fixed"; + compatible = "regulator-gpio"; enable-active-high; - gpio = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>; + gpios = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>; + states = <5000000 0x1>, <0 0x0>; pinctrl-names = "default"; pinctrl-0 = <&vbus5v0_typec_en>; regulator-name = "vbus5v0_typec";
Quoted 1 line
Could you please try this at your end?
This is missing gpios-states, but I also see no point in testing that. Having one state as 0 effectively means this is a complicated version of regulator-fixed. The switch from "gpio" to "gpios" is valid though, since "gpio" is deprecated.
Greetings,
-- Sebastian
Re: [PATCH v1 1/3] arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Correct Type-C pin bias settings
Hi Sebastian,
I am having difficulty articulating my thoughts clearly right now; please bear with me as I try to explain.
Quoted 52 lines · Sebastian Reichel
Hi,
Quoted 48 lines · Anand Moon
Hi Sebastian,
Thanks for your review comments.
Quoted 24 lines · Sebastian Reichel
Hi,
Quoted 10 lines · Anand Moon
As pre FUSB302 datasheet interrupt line (
INT_N) is an open-drain, active-low signal. It requires a pull-up resistor to maintain a stable high state when deasserted. Similarly, theTYPEC5V_PWREN_Henable signal requires a pull-down resistor to ensure it defaults to a low state, preventing unintended power delivery during the boot sequence.Update the pinctrl entries to use
pcfg_pull_upforusbc0_intandpcfg_pull_downforvbus5v0_typec_ento align with the hardware's electrical requirements.Quoted 7 lines
Cc Sebastian Reichel <[email protected]>Fixes 67b2c15d8fb3 ("arm64: dts: rockchip: add USB-C support for ROCK 5B/5B+/5T")Signed-off-by Anand Moon <[email protected]>---
v1: As per the shematicsCC_INT_Linterrupt pin isGPIO3_B4_u
As per the shematicsTYPEC5V_PWREN_Hpin isGPIO2_B6_d
---Checking the schematics:
Quoted 9 lines
5B v1.45 -
CC_INT_L- R2613 10K pull-up resistor
5B v1.45 -TYPEC5V_PWREN_H- GPIO is effectively unused because R95035 is NC5B+ v1.2 -
CC_INT_L- R2613 10K pull-up resistor 5B+ v1.2 -TYPEC5V_PWREN_H- R163 100K pull-down resistor5T v1.2 -
CC_INT_L- R2613 10K pull-up resistor 5T v1.2 -TYPEC5V_PWREN_H- R163 100K pull-down resistor
CC_INT_Lis gpio interrupt pin to enable the I2C read operation.As per FUSB302 datasheet, here is the updated version of the commit message
Configure
CC_INT_Las an active-low, open-drain output. Per the hardware design, this pin utilizes an external pull-up and is driven LOW by the Type-C controller to
signal the processor to perform I2C register reads.
Quoted 6 lines
Quoted 4 lines
The
TYPEC5V_PWREN_Hpull-down resistor is turned on as a signal to the Type-C regulator, which has an active-enable-high property. In this context, the regulator is used to set the polarity of the GPIO used to enable or disable the regulator.I don't understand what you want to say here.
I'm sorry for not being clear here. Well, while investigating the bug crash, I was looking for some more details on this issue.
Here is the datasheet for fuse302. It shares the interrupt pin details
[1} https://www.onsemi.com/download/data-sheet/pdf/fusb302b-d.pdf
SCL Input I2C serial clock signal to be
connected to the phone-based I2C master.
SDA Open-Drain I/O I2C serial data signal to be connected
to the phone-based I2C masterINT_N Open-Drain Output Active LOW open drain interrupt output used
to prompt the processor to
read the I2C register bits.
Quoted 10 lines
Quoted 5 lines
Quoted 4 lines
TLDR: All GPIOs have pull resistors in discrete hardware and do not need them muxed in the SoC.
It depends on the GPIO, pinctrl PMU configuration.
as I try to say it does not depend on pinctrl configuration for any of the GPIOs you are touching. The pull resistors for them exist as components on the board.
I understand this correctly, but these registers utilize a high-to-low state change specifically
to prevent subthreshold leakage current.
Please check the RK3588 Datasheet V0.1-20210727.pdf (3.4 Electrical
Characteristics for General IO)
[2] https://gitlab.com/rock-chips/rk3588/rk3588-doc/-/raw/master/RK3588%20Datasheet%20V0.1-20210727.pdf?ref_type=heads&inline=true
Quoted 28 lines
Quoted 28 lines
I was thinking of converting the
vbus5v0_typecregulator to 'regulator-gpio.' to better reflect the hardware schematics. TheTYPEC5V_PWREN_Hsignal (GPIO2_B6_d) acts as a state controller rather than a simple enable pin, and this change ensures the device tree aligns with that design[1] https://dl.radxa.com/rock5/5b/docs/hw/radxa_rock_5b_v1450_schematic.pdf
(page 28)$ git diff arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsiarch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi +3
-2@@ -57,9 +57,10 @@ rfkill-bt { }; vbus5v0_typec: vbus5v0-typec { - compatible = "regulator-fixed"; + compatible = "regulator-gpio"; enable-active-high; - gpio = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>; + gpios = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>; + states = <5000000 0x1>, <0 0x0>; pinctrl-names = "default"; pinctrl-0 = <&vbus5v0_typec_en>; regulator-name = "vbus5v0_typec";
Quoted 7 lines
Quoted 1 line
Could you please try this at your end?
This is missing gpios-states, but I also see no point in testing that. Having one state as 0 effectively means this is a complicated version of regulator-fixed. The switch from "gpio" to "gpios" is valid though, since "gpio" is deprecated.
Typically, a regulator-fixed GPIO input pin is used as EN to enable the regulator.
However, in this case, TYPEC5V_PWREN_H is being used to signal on/off to the FUSB302.
As the GPIO-controlled interrupt pin is used to enable or disable i2C controller.
This is the reason I want to switch from regulator-fixed to regulator-gpio. ok I will add gpios-states= <1>;
That’s why I requested these changes to be reviewed and tested.
Please let me know if these adjustments are incorrect; if so, we can discard them.
Currently, I'm not making the desired progress.
Quoted 3 lines
Greetings,
-- Sebastian
Thanks
-Anand
Quoted 4 lines
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
PATCH 2/3 arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Fix USB host phy-supply on Rock 5b-5bp-5t SbC
In reply to Anand MoonThe USB host USB2 and USB3 on the Rock 5B, 5BP, and 5T is powered by the USB_HOST_PWREN_H gpio pin which is used to enable vcc5v0_host host regulator which ensures proper power sequencing and management for onboard Double-USB-HOST.
Update the u2phy2_host node to reference the correct phy-supply.
--- arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi +1 -2
@@ -981,8 +981,7 @@ &u2phy2 { }; &u2phy2_host { - /* connected to USB hub, which is powered by vcc5v0_sys */ - phy-supply = <&vcc5v0_sys>; + phy-supply = <&vcc5v0_host>; status = "okay"; };
Signature · 1 line
2.50.1
Re: [PATCH v1 2/3] arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Fix USB host phy-supply on Rock 5b-5bp-5t SbC
Hi,
Quoted 10 lines · Anand Moon
The USB host USB2 and USB3 on the Rock 5B, 5BP, and 5T is powered by the
USB_HOST_PWREN_Hgpio pin which is used to enablevcc5v0_hosthost regulator which ensures proper power sequencing and management for onboard Double-USB-HOST.Update the
u2phy2_hostnode to reference the correct phy-supply.Cc Sebastian Reichel <[email protected]>Signed-off-by Anand Moon <[email protected]>---
u2phy2_host is connected to an onboard USB hub chip, which is powered by &vcc5v0_sys as the comment says.
You can see this on page 9 of the v1.45 ROCK 5B schematics (page name USB2HUB). The usb hub chip uses VCC5V0_SYS + VCC_3V3_S0 and is connected to USB20_HOST0_DP, USB20_HOST0_DM. The related USB controller for that in the upstream DT is &usb_host0_ehci, which in turn is using &u2phy2_host. Thus at least for the ROCK 5B the existing DT is correct. Properly describing the downstream port VBUS regulator would require something like the following series:
https://lore.kernel.org/all/20250911-v6-16-topic-usb-onboard-dev-v4-0-1af288125d74@pengutronix.de/
Greetings,
-- Sebastian
Quoted 20 lines
arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi +1
-2@@ -981,8 +981,7 @@ &u2phy2 { }; &u2phy2_host { - /* connected to USB hub, which is powered by vcc5v0_sys */ - phy-supply = <&vcc5v0_sys>; + phy-supply = <&vcc5v0_host>; status = "okay"; };Signature · 1 line
2.50.1
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH v1 2/3] arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Fix USB host phy-supply on Rock 5b-5bp-5t SbC
Hi Sebastian,
Thanks for your review comments.
Quoted 26 lines · Sebastian Reichel
Hi,
Quoted 10 lines · Anand Moon
The USB host USB2 and USB3 on the Rock 5B, 5BP, and 5T is powered by the
USB_HOST_PWREN_Hgpio pin which is used to enablevcc5v0_hosthost regulator which ensures proper power sequencing and management for onboard Double-USB-HOST.Update the
u2phy2_hostnode to reference the correct phy-supply.Cc Sebastian Reichel <[email protected]>Signed-off-by Anand Moon <[email protected]>---
u2phy2_hostis connected to an onboard USB hub chip, which is powered by&vcc5v0_sysas the comment says.You can see this on page 9 of the v1.45 ROCK 5B schematics (page name USB2HUB). The usb hub chip uses
VCC5V0_SYS+VCC_3V3_S0and is connected toUSB20_HOST0_DP,USB20_HOST0_DM. The related USB controller for that in the upstream DT is&usb_host0_ehci, which in turn is using&u2phy2_host. Thus at least for the ROCK 5B the existing DT is correct. Properly describing the downstream port VBUS regulator would require something like the following series:
Quoted 2 lines
https://lore.kernel.org/all/20250911-v6-16-topic-usb-onboard-dev-v4-0-1af288125d74@pengutronix.de/
Actually, the Radxa Rock 5 B doesn't have an onboard USB hub chip.
The USB_HOST_PWREN_H GPIO pin acts as the enable signal for both 5V_USB1 and 5V_USB2 regulators.
Data signals from the USB 2.0 Host 1 interface (USB20_HOST1_DM/DP) are routed through the CON20B chip to the Type-C controller's TYPEC1_OTG_DM/DP pins.
Additionally, the schematic indicates that the Type-C controller's power delivery is managed
by the USB2 host (5V_USB2) rail.
From the schematic, I could see that the Type-C controller is controlled by USB1 host (5V_USB1) and USB2 host (5V_USB2) for power delivery.
[1] https://dl.radxa.com/rock5/5b/docs/hw/radxa_rock_5b_v1450_schematic.pdf
(page 27)
Quoted 4 lines
Greetings,
-- Sebastian
Thanks
-Anand
Re: [PATCH v1 2/3] arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Fix USB host phy-supply on Rock 5b-5bp-5t SbC
Hi,
Quoted 25 lines · Anand Moon
Thanks for your review comments.
Quoted 23 lines · Sebastian Reichel
Quoted 10 lines · Anand Moon
The USB host USB2 and USB3 on the Rock 5B, 5BP, and 5T is powered by the
USB_HOST_PWREN_Hgpio pin which is used to enablevcc5v0_hosthost regulator which ensures proper power sequencing and management for onboard Double-USB-HOST.Update the
u2phy2_hostnode to reference the correct phy-supply.Cc Sebastian Reichel <[email protected]>Signed-off-by Anand Moon <[email protected]>---
u2phy2_hostis connected to an onboard USB hub chip, which is powered by&vcc5v0_sysas the comment says.You can see this on page 9 of the v1.45 ROCK 5B schematics (page name USB2HUB). The usb hub chip uses
VCC5V0_SYS+VCC_3V3_S0and is connected toUSB20_HOST0_DP,USB20_HOST0_DM. The related USB controller for that in the upstream DT is&usb_host0_ehci, which in turn is using&u2phy2_host. Thus at least for the ROCK 5B the existing DT is correct. Properly describing the downstream port VBUS regulator would require something like the following series:
Quoted 3 lines
Quoted 2 lines
https://lore.kernel.org/all/20250911-v6-16-topic-usb-onboard-dev-v4-0-1af288125d74@pengutronix.de/
Actually, the Radxa Rock 5 B doesn't have an onboard USB hub chip.
Seriously? I provided the exact page for the hub chip in the schematics and it's easily visisble in lsusb too.
I don't want to waste my time and expect a certain level of quality and this is certainly way below it...
-- Sebastian
Re: [PATCH v1 2/3] arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Fix USB host phy-supply on Rock 5b-5bp-5t SbC
Hi Sebastian,
I am having difficulty articulating my thoughts clearly right now; please bear with me as I try to explain.
Quoted 29 lines · Sebastian Reichel
Hi,
Quoted 25 lines · Anand Moon
Thanks for your review comments.
Quoted 23 lines · Sebastian Reichel
Quoted 10 lines · Anand Moon
The USB host USB2 and USB3 on the Rock 5B, 5BP, and 5T is powered by the
USB_HOST_PWREN_Hgpio pin which is used to enablevcc5v0_hosthost regulator which ensures proper power sequencing and management for onboard Double-USB-HOST.Update the
u2phy2_hostnode to reference the correct phy-supply.Cc Sebastian Reichel <[email protected]>Signed-off-by Anand Moon <[email protected]>---
u2phy2_hostis connected to an onboard USB hub chip, which is powered by&vcc5v0_sysas the comment says.You can see this on page 9 of the v1.45 ROCK 5B schematics (page name USB2HUB). The usb hub chip uses
VCC5V0_SYS+VCC_3V3_S0and is connected toUSB20_HOST0_DP,USB20_HOST0_DM. The related USB controller for that in the upstream DT is&usb_host0_ehci, which in turn is using&u2phy2_host. Thus at least for the ROCK 5B the existing DT is correct. Properly describing the downstream port VBUS regulator would require something like the following series:
Quoted 7 lines
Quoted 3 lines
Quoted 2 lines
https://lore.kernel.org/all/20250911-v6-16-topic-usb-onboard-dev-v4-0-1af288125d74@pengutronix.de/
Actually, the Radxa Rock 5 B doesn't have an onboard USB hub chip.
Seriously? I provided the exact page for the hub chip in the schematics and it's easily visisble in lsusb too.
My apologies for the confusion. To clarify, this board lacks a USB hub chip that matches the
detected USB IDs and is capable of powering both USB 2.0 and USB 3.0 interfaces.
This is the reason we have a separate power regulator for USB 2.0 and USB 3.0 ports
and Type-C controllers.
There is a USB2HUB -> U90078 FE1.1s_QFN chip (page 27) in the schematics, It's probably connected to the PCIE M.2 NGFF for Bluetooth
[2] https://www.snapeda.com/parts/FE1.1s-BQFN24B/Terminus-Tech/datasheet/
$ lsusb -tv
/: Bus 001.Port 001: Dev 001, Class=root_hub, Driver=ehci-platform/1p, 480M
ID 1d6b:0002 Linux Foundation 2.0 root hub
|__ Port 001: Dev 002, If 0, Class=Hub, Driver=hub/4p, 480M
ID 1a40:0101 Terminus Technology Inc. Hub
|__ Port 003: Dev 003, If 0, Class=Wireless, Driver=btusb, 12M
ID 8087:0a2b Intel Corp. Bluetooth wireless interface
|__ Port 003: Dev 003, If 1, Class=Wireless, Driver=btusb, 12M
ID 8087:0a2b Intel Corp. Bluetooth wireless interface
/: Bus 002.Port 001: Dev 001, Class=root_hub, Driver=ohci-platform/1p, 12M
ID 1d6b:0001 Linux Foundation 1.1 root hub
/: Bus 003.Port 001: Dev 001, Class=root_hub, Driver=ehci-platform/1p, 480M
ID 1d6b:0002 Linux Foundation 2.0 root hub
/: Bus 004.Port 001: Dev 001, Class=root_hub, Driver=ohci-platform/1p, 12M
ID 1d6b:0001 Linux Foundation 1.1 root hub
/: Bus 005.Port 001: Dev 001, Class=root_hub, Driver=xhci-hcd/1p, 480M
ID 1d6b:0002 Linux Foundation 2.0 root hub
/: Bus 006.Port 001: Dev 001, Class=root_hub, Driver=xhci-hcd/1p, 5000M
ID 1d6b:0003 Linux Foundation 3.0 root hub
/: Bus 007.Port 001: Dev 001, Class=root_hub, Driver=xhci-hcd/1p, 480M
ID 1d6b:0002 Linux Foundation 2.0 root hub
/: Bus 008.Port 001: Dev 001, Class=root_hub, Driver=xhci-hcd/1p, 5000M
ID 1d6b:0003 Linux Foundation 3.0 root hubQuoted 2 lines
I don't want to waste my time and expect a certain level of quality and this is certainly way below it...
Please accept my apologies for the miscommunication.
Could you check the enable status of the USB_HOST_PWREN_H pin in the schematics?
USB_HOST_PWREN_H -> VCC5V0_USB20_HOST0 / VCC5V0_USB20_HOST2 USB_HOST_PWREN_H -> VCC5V0_USB30_HOST0 / VCC5V0_USB30_HOST1
My current patch is trying to address this issue.
Quoted 2 lines
-- Sebastian
Thanks
-Anand
PATCH 3/3 usb: typec: fusb302: Switch to threaded interrupt handler
In reply to Anand MoonThe fusb302 driver triggers a "BUG: Invalid wait context" lockdep warning under certain configurations (such as when CONFIG_PROVE_RAW_LOCK_NESTING is enabled). This occurs because the interrupt handler, fusb302_irq_intn, attempts to acquire a regular spinlock (&chip->irq_lock) while running in hardirq context can lead to invalid wait context reports if the lock is considered "sleepable" or has incompatible nesting levels with the underlying interrupt controller's locks.
lockdep warnings:
[ 38.935276] [ C0] =============================
[ 38.935690] [ C0] [ BUG: Invalid wait context ]
[ 38.936106] [ C0] 6.19.0-rc2-2-ARM64-GCC #2 Tainted: GT
[ 38.936716] [ C0] -----------------------------
[ 38.937129] [ C0] kworker/0:0/8 is trying to lock:
[ 38.937566] [ C0] ffff000112c04190 (&chip->irq_lock){....}-{3:3}, at: fusb302_irq_intn+0x38/0x98 [fusb302]
[ 38.938450] [ C0] other info that might help us debug this:
[ 38.938953] [ C0] context-{2:2}
[ 38.939247] [ C0] 2 locks held by kworker/0:0/8:
[ 38.939670] [ C0] #0: ffff000100025148 ((wq_completion)events_freezable){+.+.}-{0:0}, at: process_one_work+0x224/0x4b8
[ 38.940645] [ C0] #1: ffff8000800fbd90 ((work_completion)(&(&host->detect)->work)){+.+.}-{0:0}, at: process_one_work+0x24c/0x4b8
[ 38.941691] [ C0] stack backtrace:
[ 38.942010] [ C0] CPU: 0 UID: 0 PID: 8 Comm: kworker/0:0 Tainted: GT 6.19.0-rc2-2-ARM64-GCC #2 PREEMPT(full) bd73c5afc1bd41f04ef9699c14f0381f835f4deb
[ 38.942017] [ C0] Tainted: [T]=RANDSTRUCT
[ 38.942019] [ C0] Hardware name: Radxa ROCK 5B (DT)
[ 38.942022] [ C0] Workqueue: events_freezable mmc_rescan
[ 38.942031] [ C0] Call trace:
[ 38.942033] [ C0] show_stack+0x24/0x40 (C)
[ 38.942041] [ C0] dump_stack_lvl+0x90/0xd8
[ 38.942047] [ C0] dump_stack+0x1c/0x3c
[ 38.942051] [ C0] __lock_acquire+0x5e8/0x9c8
[ 38.942059] [ C0] lock_acquire+0x134/0x280
[ 38.942065] [ C0] _raw_spin_lock_irqsave+0x80/0xb0
[ 38.942072] [ C0] fusb302_irq_intn+0x38/0x98 [fusb302 634bac905a09c450b54f88b96019accd2820228f]
[ 38.942082] [ C0] __handle_irq_event_percpu+0x138/0x3f0
[ 38.942088] [ C0] handle_irq_event+0x58/0xd8
[ 38.942093] [ C0] handle_level_irq+0x108/0x190
[ 38.942099] [ C0] handle_irq_desc+0x4c/0x78
[ 38.942106] [ C0] generic_handle_domain_irq+0x24/0x40
[ 38.942113] [ C0] rockchip_irq_demux+0x128/0x240
[ 38.942120] [ C0] handle_irq_desc+0x4c/0x78
[ 38.942127] [ C0] generic_handle_domain_irq+0x24/0x40
[ 38.942133] [ C0] __gic_handle_irq_from_irqson.isra.0+0x260/0x370
[ 38.942141] [ C0] gic_handle_irq+0x68/0xa0
[ 38.942146] [ C0] call_on_irq_stack+0x48/0x68
[ 38.942152] [ C0] do_interrupt_handler+0x74/0x98
[ 38.942158] [ C0] el1_interrupt+0x88/0xb0
[ 38.942165] [ C0] el1h_64_irq_handler+0x1c/0x30
[ 38.942170] [ C0] el1h_64_irq+0x84/0x88
[ 38.942175] [ C0] arch_counter_get_cntpct+0x4/0x20 (P)
[ 38.942181] [ C0] __const_udelay+0x30/0x48
[ 38.942187] [ C0] mci_send_cmd.constprop.0+0x84/0xc8
[ 38.942194] [ C0] dw_mci_setup_bus+0x60/0x210
[ 38.942200] [ C0] dw_mci_set_ios+0x1c8/0x260
[ 38.942206] [ C0] mmc_set_initial_state+0x110/0x140
[ 38.942211] [ C0] mmc_rescan_try_freq+0x154/0x198
[ 38.942216] [ C0] mmc_rescan+0x1cc/0x278
[ 38.942221] [ C0] process_one_work+0x284/0x4b8
[ 38.942225] [ C0] worker_thread+0x264/0x3a0
[ 38.942230] [ C0] kthread+0x11c/0x138
[ 38.942236] [ C0] ret_from_fork+0x10/0x20
[ 38.969307] [ T11] rockchip-dw-pcie a41000000.pcie: PCI host bridge to bus 0004:40
[ 38.969995] [ T11] pci_bus 0004:40: root bus resource [bus 40-4f]Following changes resolves the lockdep warnings and aligns the driver with best practices for I2C-based interrupt handling.
--- drivers/usb/typec/tcpm/fusb302.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/usb/typec/tcpm/fusb302.c +4 -3
@@ -1755,9 +1755,10 @@ static int fusb302_probe(struct i2c_client *client) goto destroy_workqueue; } - ret =request_irq (chip->gpio_int_n_irq, fusb302_irq_intn, -IRQF_ONESHOT |IRQF_TRIGGER_LOW , -"fsc_interrupt_int_n" , chip); + ret =request_threaded_irq (chip->gpio_int_n_irq, +NULL , fusb302_irq_intn, +IRQF_ONESHOT |IRQF_TRIGGER_LOW , +"fsc_interrupt_int_n" , chip);if (ret <0 ) {dev_err (dev,"cannot request IRQ for GPIO Int_N, ret=%d" , ret); goto tcpm_unregister_port;
Signature · 1 line
2.50.1
Re: [PATCH v1 3/3] usb: typec: fusb302: Switch to threaded interrupt handler
Hi,
Quoted 64 lines · Anand Moon
The fusb302 driver triggers a "BUG: Invalid wait context" lockdep warning under certain configurations (such as when
CONFIG_PROVE_RAW_LOCK_NESTINGis enabled). This occurs because the interrupt handler,fusb302_irq_intn, attempts to acquire a regular spinlock (&chip->irq_lock) while running in hardirq context can lead to invalid wait context reports if the lock is considered "sleepable" or has incompatible nesting levels with the underlying interrupt controller's locks.lockdep warnings:
[ 38.935276] [ C0] ============================= [ 38.935690] [ C0] [ BUG: Invalid wait context ] [ 38.936106] [ C0] 6.19.0-rc2-2-ARM64-GCC #2 Tainted: GT [ 38.936716] [ C0] ----------------------------- [ 38.937129] [ C0] kworker/0:0/8 is trying to lock: [ 38.937566] [ C0] ffff000112c04190 (&chip->irq_lock){....}-{3:3}, at: fusb302_irq_intn+0x38/0x98 [fusb302] [ 38.938450] [ C0] other info that might help us debug this: [ 38.938953] [ C0] context-{2:2} [ 38.939247] [ C0] 2 locks held by kworker/0:0/8: [ 38.939670] [ C0] #0: ffff000100025148 ((wq_completion)events_freezable){+.+.}-{0:0}, at: process_one_work+0x224/0x4b8 [ 38.940645] [ C0] #1: ffff8000800fbd90 ((work_completion)(&(&host->detect)->work)){+.+.}-{0:0}, at: process_one_work+0x24c/0x4b8 [ 38.941691] [ C0] stack backtrace: [ 38.942010] [ C0] CPU: 0 UID: 0 PID: 8 Comm: kworker/0:0 Tainted: GT 6.19.0-rc2-2-ARM64-GCC #2 PREEMPT(full) bd73c5afc1bd41f04ef9699c14f0381f835f4deb [ 38.942017] [ C0] Tainted: [T]=RANDSTRUCT [ 38.942019] [ C0] Hardware name: Radxa ROCK 5B (DT) [ 38.942022] [ C0] Workqueue: events_freezable mmc_rescan [ 38.942031] [ C0] Call trace: [ 38.942033] [ C0] show_stack+0x24/0x40 (C) [ 38.942041] [ C0] dump_stack_lvl+0x90/0xd8 [ 38.942047] [ C0] dump_stack+0x1c/0x3c [ 38.942051] [ C0] __lock_acquire+0x5e8/0x9c8 [ 38.942059] [ C0] lock_acquire+0x134/0x280 [ 38.942065] [ C0] _raw_spin_lock_irqsave+0x80/0xb0 [ 38.942072] [ C0] fusb302_irq_intn+0x38/0x98 [fusb302 634bac905a09c450b54f88b96019accd2820228f] [ 38.942082] [ C0] __handle_irq_event_percpu+0x138/0x3f0 [ 38.942088] [ C0] handle_irq_event+0x58/0xd8 [ 38.942093] [ C0] handle_level_irq+0x108/0x190 [ 38.942099] [ C0] handle_irq_desc+0x4c/0x78 [ 38.942106] [ C0] generic_handle_domain_irq+0x24/0x40 [ 38.942113] [ C0] rockchip_irq_demux+0x128/0x240 [ 38.942120] [ C0] handle_irq_desc+0x4c/0x78 [ 38.942127] [ C0] generic_handle_domain_irq+0x24/0x40 [ 38.942133] [ C0] __gic_handle_irq_from_irqson.isra.0+0x260/0x370 [ 38.942141] [ C0] gic_handle_irq+0x68/0xa0 [ 38.942146] [ C0] call_on_irq_stack+0x48/0x68 [ 38.942152] [ C0] do_interrupt_handler+0x74/0x98 [ 38.942158] [ C0] el1_interrupt+0x88/0xb0 [ 38.942165] [ C0] el1h_64_irq_handler+0x1c/0x30 [ 38.942170] [ C0] el1h_64_irq+0x84/0x88 [ 38.942175] [ C0] arch_counter_get_cntpct+0x4/0x20 (P) [ 38.942181] [ C0] __const_udelay+0x30/0x48 [ 38.942187] [ C0] mci_send_cmd.constprop.0+0x84/0xc8 [ 38.942194] [ C0] dw_mci_setup_bus+0x60/0x210 [ 38.942200] [ C0] dw_mci_set_ios+0x1c8/0x260 [ 38.942206] [ C0] mmc_set_initial_state+0x110/0x140 [ 38.942211] [ C0] mmc_rescan_try_freq+0x154/0x198 [ 38.942216] [ C0] mmc_rescan+0x1cc/0x278 [ 38.942221] [ C0] process_one_work+0x284/0x4b8 [ 38.942225] [ C0] worker_thread+0x264/0x3a0 [ 38.942230] [ C0] kthread+0x11c/0x138 [ 38.942236] [ C0] ret_from_fork+0x10/0x20 [ 38.969307] [ T11] rockchip-dw-pcie a41000000.pcie: PCI host bridge to bus 0004:40 [ 38.969995] [ T11] pci_bus 0004:40: root bus resource [bus 40-4f]
Quoted 8 lines
Following changes resolves the lockdep warnings and aligns the driver with best practices for I2C-based interrupt handling.
Cc Hans de Goede <[email protected]>Cc Yongbo Zhang <[email protected]>Cc Sebastian Reichel <[email protected]>Fixes 309b6341d557 ("usb: typec: fusb302: Revert incorrect threaded irq fix")Signed-off-by Anand Moon <[email protected]>
If you look closer at the code then you will see that fusb302_irq_intn() is effectively doing its own threaded interrupt handling this is done to be able to delay the threaded part till after the i2c-controller is resumed when a fusb302 irq wakes up the system.
See commit 207338ec5a27 ("usb: typec: fusb302: Improve suspend/resume handling") for details.
And if you look at the fusb302 git history then you'll seen an earlier change the switch the interrupt handler to a threaded IRQ which was reverted (mostly due to it also making other undesirable changes).
This change is different though. This is actually quite similar to commit cee3dba7b741 ("mei: vsc: Fix "BUG: Invalid wait context" lockdep error"). Where I fixed more or less the same issue in the same way. So I guess this change also is ok.
Still ideally we would solve this in another way then switching to a threaded IRQ handler.
As the commit message of the mei-vsc fix mentions the root cause of these errors is typically an interrupt chip driver which uses IRQF_NO_THREAD disabling the auto threading of all interrupt handlers in RT mode.
So the first question here would be to see if that flag is used in the interrupt chip and if yes, is that flag really necessary ?
Regards,
Hans
Quoted 22 lines
--- drivers/usb/typec/tcpm/fusb302.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)drivers/usb/typec/tcpm/fusb302.c +4
-3@@ -1755,9 +1755,10 @@ static int fusb302_probe(struct i2c_client *client) goto destroy_workqueue; } - ret =request_irq (chip->gpio_int_n_irq, fusb302_irq_intn, -IRQF_ONESHOT |IRQF_TRIGGER_LOW , -"fsc_interrupt_int_n" , chip); + ret =request_threaded_irq (chip->gpio_int_n_irq, +NULL , fusb302_irq_intn, +IRQF_ONESHOT |IRQF_TRIGGER_LOW , +"fsc_interrupt_int_n" , chip);if (ret <0 ) {dev_err (dev,"cannot request IRQ for GPIO Int_N, ret=%d" , ret); goto tcpm_unregister_port;
Re: [PATCH v1 3/3] usb: typec: fusb302: Switch to threaded interrupt handler
Quoted 11 lines
Still ideally we would solve this in another way then switching to a threaded IRQ handler.
As the commit message of the mei-vsc fix mentions the root cause of these errors is typically an interrupt chip driver which uses
IRQF_NO_THREADdisabling the auto threading of all interrupt handlers in RT mode.So the first question here would be to see if that flag is used in the interrupt chip and if yes, is that flag really necessary ?
This is very similar to the issue addressed in commit 24b176d8827d ("drm/msm/dsi: Remove spurious IRQF_ONESHOT flag").
The IRQF_ONESHOT flag is preventing forced threading here.
In irq_setup_forced_threading(), the conversion to threaded interrupts is explicitly skipped if any of the IRQF_NO_THREAD, IRQF_PERCPU, or IRQF_ONESHOT flags are present. In this case, IRQF_ONESHOT appears to be the reason.
Regards,
giraffesnn
Re: [PATCH v1 3/3] usb: typec: fusb302: Switch to threaded interrupt handler
Hi,
Quoted 11 lines · 张永波
Quoted 11 lines
Still ideally we would solve this in another way then switching to a threaded IRQ handler.
As the commit message of the mei-vsc fix mentions the root cause of these errors is typically an interrupt chip driver which uses
IRQF_NO_THREADdisabling the auto threading of all interrupt handlers in RT mode.So the first question here would be to see if that flag is used in the interrupt chip and if yes, is that flag really necessary ?
Quoted 8 lines
This is very similar to the issue addressed in commit
24b176d8827d("drm/msm/dsi: Remove spuriousIRQF_ONESHOTflag").
TheIRQF_ONESHOTflag is preventing forced threading here.In
irq_setup_forced_threading(), the conversion to threaded interrupts is explicitly skipped if any of theIRQF_NO_THREAD,IRQF_PERCPU, orIRQF_ONESHOTflags are present. In this case,IRQF_ONESHOTappears to be the reason.
Ah, well the code effectively does its own IRQF_ONESHOT handling, since it needs to do its own threaded-irq like handling for suspend/resume reasons. It disables the IRQ when it fires and then only re-enables it once the work has done processing the IRQ.
So it should be perfectly safe to drop the IRQF_ONESHOT flag.
If that also works to resolve the lockdep issue that would be the preferred way of fixing this IMHO.
Regards,
Hans
Re: [PATCH v1 3/3] usb: typec: fusb302: Switch to threaded interrupt handler
Hi Hans,
Quoted 24 lines · Hans de Goede
Hi,
Quoted 11 lines · 张永波
Quoted 11 lines
Still ideally we would solve this in another way then switching to a threaded IRQ handler.
As the commit message of the mei-vsc fix mentions the root cause of these errors is typically an interrupt chip driver which uses
IRQF_NO_THREADdisabling the auto threading of all interrupt handlers in RT mode.So the first question here would be to see if that flag is used in the interrupt chip and if yes, is that flag really necessary ?
Quoted 8 lines
This is very similar to the issue addressed in commit
24b176d8827d("drm/msm/dsi: Remove spuriousIRQF_ONESHOTflag").
TheIRQF_ONESHOTflag is preventing forced threading here.In
irq_setup_forced_threading(), the conversion to threaded interrupts is explicitly skipped if any of theIRQF_NO_THREAD,IRQF_PERCPU, orIRQF_ONESHOTflags are present. In this case,IRQF_ONESHOTappears to be the reason.
Quoted 7 lines
Ah, well the code effectively does its own
IRQF_ONESHOThandling, since it needs to do its own threaded-irq like handling for suspend/resume reasons. It disables the IRQ when it fires and then only re-enables it once the work has done processing the IRQ.So it should be perfectly safe to drop the
IRQF_ONESHOTflag.
Yes, the warning disappears
Quoted 3 lines
If that also works to resolve the lockdep issue that would be the preferred way of fixing this IMHO.
After applying these changes, the device initially triggered a hard reset; And the board reboots. I need to find another way to fix this warning.
However, after several reboots, it returned to a normal boot sequence.
[ 40.404525][ T5974] rockchip-dw-pcie a41000000.pcie: LTSSM_STATUS: 0x130011 [ 40.404936][ T4957] r8169 0004:41:00.0: enabling Mem-Wr-Inval [ 40.406591][ T56] dwmmc_rockchip fe2d0000.mmc: IDMAC supports 32-bit address mode. [ 40.407370][ T56] dwmmc_rockchip fe2d0000.mmc: Using internal DMA controller. [ 40.408013][ T56] dwmmc_rockchip fe2d0000.mmc: Version ID is 270a [ 40.408610][ T56] dwmmc_rockchip fe2d0000.mmc: DW MMC controller at irq 104,32 bit host data width,256 deep fifo [ 40.413373][ T56] mmc_host mmc2: card is non-removable. [ 40.431072][ T56] mmc_host mmc2: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0) [ 40.468235][ T67] mmc_host mmc2: Bus speed (slot 0) = 300000Hz (slot req 300000Hz, actual 300000HZ div = 0) [ 40.492229][ T4957] r8169 0004:41:00.0 eth0: RTL8125B, 00:e0:4c:68:00:35, XID 641, IRQ 153 [ 40.492993][ T4957] r8169 0004:41:00.0 eth0: jumbo features [frames: 16362 bytes, tx checksumming: ko] [ 40.493873][ T4957] r8169 0004:41:00.0: vgaarb: pci_notify [ 40.507133][ T5974] rockchip-dw-pcie a41000000.pcie: Received Link up event. Starting enumeration! [ 40.508219][ T5974] pci_bus 0004:40: scanning bus [ 40.517387][ T5974] pcieport 0004:40:00.0: scanning [bus 41-41] behind bridge, pass 0 [ 40.517867][ T67] mmc_host mmc2: Bus speed (slot 0) = 200000Hz (slot req 200000Hz, actual 200000HZ div = 0) [ 40.518336][ T5974] pci_bus 0004:41: scanning bus [ 40.519531][ T5974] pci_bus 0004:41: bus scan returning with max=41 [ 40.520294][ T5974] pcieport 0004:40:00.0: scanning [bus 41-41] behind bridge, pass 1 [ 40.521273][ T5974] pci_bus 0004:40: bus scan returning with max=41
DDR ff1a08bde6 typ 25/03/13-15:39:39,fwver: v1.19
ch0 ttot10
ch1 ttot10
ch2 ttot10
ch3 ttot10
ch0 ttot16
LPDDR4X, 2112MHz
channel[0] BW=16 Col=10 Bk=8 CS0 Row=16 CS1 Row=16 CS=2 Die BW=16 Size=2048MB ch1 ttot16
channel[1] BW=16 Col=10 Bk=8 CS0 Row=16 CS1 Row=16 CS=2 Die BW=16 Size=2048MB ch2 ttot16
channel[2] BW=16 Col=10 Bk=8 CS0 Row=16 CS1 Row=16 CS=2 Die BW=16 Size=2048MB ch3 ttot16
channel[3] BW=16 Col=10 Bk=8 CS0 Row=16 CS1 Row=16 CS=2 Die BW=16 Size=2048MB Manufacturer ID:0x6
Quoted 4 lines
Regards,
Hans
Thanks
-Anand
Re: [PATCH v1 3/3] usb: typec: fusb302: Switch to threaded interrupt handler
Hi,
Quoted 32 lines · Anand Moon
Hi Hans,
Quoted 29 lines · Hans de Goede
Hi,
Quoted 11 lines · 张永波
Quoted 11 lines
Still ideally we would solve this in another way then switching to a threaded IRQ handler.
As the commit message of the mei-vsc fix mentions the root cause of these errors is typically an interrupt chip driver which uses
IRQF_NO_THREADdisabling the auto threading of all interrupt handlers in RT mode.So the first question here would be to see if that flag is used in the interrupt chip and if yes, is that flag really necessary ?
Quoted 8 lines
This is very similar to the issue addressed in commit
24b176d8827d("drm/msm/dsi: Remove spuriousIRQF_ONESHOTflag").
TheIRQF_ONESHOTflag is preventing forced threading here.In
irq_setup_forced_threading(), the conversion to threaded interrupts is explicitly skipped if any of theIRQF_NO_THREAD,IRQF_PERCPU, orIRQF_ONESHOTflags are present. In this case,IRQF_ONESHOTappears to be the reason.Ah, well the code effectively does its own
IRQF_ONESHOThandling, since it needs to do its own threaded-irq like handling for suspend/resume reasons. It disables the IRQ when it fires and then only re-enables it once the work has done processing the IRQ.
Quoted 8 lines
Quoted 2 lines
So it should be perfectly safe to drop the
IRQF_ONESHOTflag.Yes, the warning disappears
Quoted 3 lines
If that also works to resolve the lockdep issue that would be the preferred way of fixing this IMHO.
After applying these changes, the device initially triggered a hard reset; And the board reboots. I need to find another way to fix this warning.
Hmm, I guess that without the oneshot flag fusb302_irq_intn() might trigger a second time before it disables the IRQ causing an IRQ enable unbalance issue.
So lets just go with your original fix of moving this to a threaded IRQ handler.
Regards,
Hans
Re: [PATCH v1 3/3] usb: typec: fusb302: Switch to threaded interrupt handler
In reply to Hans de GoedeHi Hans De,
Thanks for your review comments.
Quoted 83 lines · Hans de Goede
Hi,
Quoted 64 lines · Anand Moon
The fusb302 driver triggers a "BUG: Invalid wait context" lockdep warning under certain configurations (such as when
CONFIG_PROVE_RAW_LOCK_NESTINGis enabled). This occurs because the interrupt handler,fusb302_irq_intn, attempts to acquire a regular spinlock (&chip->irq_lock) while running in hardirq context can lead to invalid wait context reports if the lock is considered "sleepable" or has incompatible nesting levels with the underlying interrupt controller's locks.lockdep warnings:
[ 38.935276] [ C0] ============================= [ 38.935690] [ C0] [ BUG: Invalid wait context ] [ 38.936106] [ C0] 6.19.0-rc2-2-ARM64-GCC #2 Tainted: GT [ 38.936716] [ C0] ----------------------------- [ 38.937129] [ C0] kworker/0:0/8 is trying to lock: [ 38.937566] [ C0] ffff000112c04190 (&chip->irq_lock){....}-{3:3}, at: fusb302_irq_intn+0x38/0x98 [fusb302] [ 38.938450] [ C0] other info that might help us debug this: [ 38.938953] [ C0] context-{2:2} [ 38.939247] [ C0] 2 locks held by kworker/0:0/8: [ 38.939670] [ C0] #0: ffff000100025148 ((wq_completion)events_freezable){+.+.}-{0:0}, at: process_one_work+0x224/0x4b8 [ 38.940645] [ C0] #1: ffff8000800fbd90 ((work_completion)(&(&host->detect)->work)){+.+.}-{0:0}, at: process_one_work+0x24c/0x4b8 [ 38.941691] [ C0] stack backtrace: [ 38.942010] [ C0] CPU: 0 UID: 0 PID: 8 Comm: kworker/0:0 Tainted: GT 6.19.0-rc2-2-ARM64-GCC #2 PREEMPT(full) bd73c5afc1bd41f04ef9699c14f0381f835f4deb [ 38.942017] [ C0] Tainted: [T]=RANDSTRUCT [ 38.942019] [ C0] Hardware name: Radxa ROCK 5B (DT) [ 38.942022] [ C0] Workqueue: events_freezable mmc_rescan [ 38.942031] [ C0] Call trace: [ 38.942033] [ C0] show_stack+0x24/0x40 (C) [ 38.942041] [ C0] dump_stack_lvl+0x90/0xd8 [ 38.942047] [ C0] dump_stack+0x1c/0x3c [ 38.942051] [ C0] __lock_acquire+0x5e8/0x9c8 [ 38.942059] [ C0] lock_acquire+0x134/0x280 [ 38.942065] [ C0] _raw_spin_lock_irqsave+0x80/0xb0 [ 38.942072] [ C0] fusb302_irq_intn+0x38/0x98 [fusb302 634bac905a09c450b54f88b96019accd2820228f] [ 38.942082] [ C0] __handle_irq_event_percpu+0x138/0x3f0 [ 38.942088] [ C0] handle_irq_event+0x58/0xd8 [ 38.942093] [ C0] handle_level_irq+0x108/0x190 [ 38.942099] [ C0] handle_irq_desc+0x4c/0x78 [ 38.942106] [ C0] generic_handle_domain_irq+0x24/0x40 [ 38.942113] [ C0] rockchip_irq_demux+0x128/0x240 [ 38.942120] [ C0] handle_irq_desc+0x4c/0x78 [ 38.942127] [ C0] generic_handle_domain_irq+0x24/0x40 [ 38.942133] [ C0] __gic_handle_irq_from_irqson.isra.0+0x260/0x370 [ 38.942141] [ C0] gic_handle_irq+0x68/0xa0 [ 38.942146] [ C0] call_on_irq_stack+0x48/0x68 [ 38.942152] [ C0] do_interrupt_handler+0x74/0x98 [ 38.942158] [ C0] el1_interrupt+0x88/0xb0 [ 38.942165] [ C0] el1h_64_irq_handler+0x1c/0x30 [ 38.942170] [ C0] el1h_64_irq+0x84/0x88 [ 38.942175] [ C0] arch_counter_get_cntpct+0x4/0x20 (P) [ 38.942181] [ C0] __const_udelay+0x30/0x48 [ 38.942187] [ C0] mci_send_cmd.constprop.0+0x84/0xc8 [ 38.942194] [ C0] dw_mci_setup_bus+0x60/0x210 [ 38.942200] [ C0] dw_mci_set_ios+0x1c8/0x260 [ 38.942206] [ C0] mmc_set_initial_state+0x110/0x140 [ 38.942211] [ C0] mmc_rescan_try_freq+0x154/0x198 [ 38.942216] [ C0] mmc_rescan+0x1cc/0x278 [ 38.942221] [ C0] process_one_work+0x284/0x4b8 [ 38.942225] [ C0] worker_thread+0x264/0x3a0 [ 38.942230] [ C0] kthread+0x11c/0x138 [ 38.942236] [ C0] ret_from_fork+0x10/0x20 [ 38.969307] [ T11] rockchip-dw-pcie a41000000.pcie: PCI host bridge to bus 0004:40 [ 38.969995] [ T11] pci_bus 0004:40: root bus resource [bus 40-4f]Quoted 8 lines
Following changes resolves the lockdep warnings and aligns the driver with best practices for I2C-based interrupt handling.
Cc Hans de Goede <[email protected]>Cc Yongbo Zhang <[email protected]>Cc Sebastian Reichel <[email protected]>Fixes 309b6341d557 ("usb: typec: fusb302: Revert incorrect threaded irq fix")Signed-off-by Anand Moon <[email protected]>If you look closer at the code then you will see that
fusb302_irq_intn()is effectively doing its own threaded interrupt handling this is done to be able to delay the threaded part till after the i2c-controller is resumed when a fusb302 irq wakes up the system.
Quoted 8 lines
See commit
207338ec5a27("usb: typec: fusb302: Improve suspend/resume handling") for details.And if you look at the fusb302 git history then you'll seen an earlier change the switch the interrupt handler to a threaded IRQ which was reverted (mostly due to it also making other undesirable changes).
Yes, I have gone through the change logs
Quoted 5 lines
This change is different though. This is actually quite similar to commit
cee3dba7b741("mei: vsc: Fix "BUG: Invalid wait context" lockdep error"). Where I fixed more or less the same issue in the same way. So I guess this change also is ok.
Yes, ideally, all the CPU cores should handle the IRQ.
alarm@rockpi-5b:~$ cat /proc/interrupts | grep fsc_interrupt_int_n
59: 15 0 0 0 0 0
0 0 rockchip_gpio_irq 12 Level
fsc_interrupt_int_nQuoted 3 lines
Still ideally we would solve this in another way then switching to a threaded IRQ handler.
I don't know but this could be related to
Quoted 9 lines
As the commit message of the mei-vsc fix mentions the root cause of these errors is typically an interrupt chip driver which uses
IRQF_NO_THREADdisabling the auto threading of all interrupt handlers in RT mode.So the first question here would be to see if that flag is used in the interrupt chip and if yes, is that flag really necessary ?
No, I did not find this IRQF_NO_THREAD flag being used by Rockchip SoC
i2c
[1] https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-rk3x.c#L1310-L1325
gpio
[2] https://github.com/torvalds/linux/blob/master/drivers/gpio/gpio-rockchip.c#L520-L582
pinctrl
[3] https://github.com/torvalds/linux/blob/master/drivers/pinctrl/pinctrl-rockchip.c
pinctrl power management IC
[4] https://github.com/torvalds/linux/blob/master/drivers/pinctrl/pinctrl-rk805.c
Quoted 4 lines
Regards,
Hans
Thnaks
-Anand
Hello Anand,
Quoted 5 lines · Anand Moon
On the Radxa Rock 5B, the system occasionally experiences intermittent hard resets during the boot process. Initially, I suspected a power supply issue, but further investigation points to the Type-C fusb302 module as the cause.
Quoted 8 lines
Specifically, probing or reloading the fusb302 module triggers a hard reset, which can result in immediate power loss and a reboot.
[root@rockpi-5b ~]# rmmod fusb302
[root@rockpi-5b ~]# lsmod | grep fusb302
[root@rockpi-5b ~]# modprobe fusb302
[root@rockpi-5b ~]# [ 3389.031608][ T7143]typec_fusb3024-0022: Initiating hard-reset, which might result in machine power-loss.[ 3390.030444][ T7143] typec_fusb302 4-0022: Initiating hard-reset, which might result in machine power-loss.
If you see this message the TypeC port manager (TCPM) state machine reached the hard reset error state. A USB-PD hard reset involves removing VBUS for a short time, which effectively removes the board power on ROCK 5B. Unfortunately the situation is quite complex :)
Quoted 3 lines
I attempted to trace the issue using ftrace but was unable to pinpoint the root cause. The problem appears to originate either from the I2C controller or the PMIC reset.
I2C and PMIC are not at fault. This is all about USB-PD communication itself.
Quoted 4 lines
I have identified a potential workaround involving the I2C SCL debounce settings for the RK3588 and submitted a patch here:
This is most likely a red herring and just slightly changing timings in the USB PD communication.
Quoted 2 lines
Please note that the submitted changes address a minor aspect but do not fully resolve the underlying issue.
I don't expect any fix from this series regarding your problem. Also I suggest having a look at my talk at the Linux Plumbers Conference from last month where I discussed this issue :)
slides: https://lpc.events/event/19/contributions/2156/attachments/1784/3861/improving-stability-for-TCPM-using-boards-that-are-not-self-powered.pdf
video: https://www.youtube.com/watch?v=DmLsePJoH8I
Something that might be sensible to do on your end is figure out *how* the state machine ended up in the error state and check if we can avoid it. The related code for that lives in drivers/usb/typec/tcpm/tcpm.c and quite complex. I use the following two patches to ease debugging:
* https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/6edc68e3c0ec4c209b5e96b848e17201059ce9ee
* https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/8ca8b1d6ee36e80f794bcf351a8b78d5a96daf06
Combined with CONFIG_DYNAMIC_DEBUG=y and booting with the following kernel arguments: loglevel=8 tcpm.dyndbg="+p" fusb302.dyndbg="+p"
Greetings,
-- Sebastian
Quoted 11 lines
Thanks
-AnandAnand Moon (3):
arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Correct Type-C pin bias
settings
arm64: dts: rockchip: rk3588-rock-5b-5bp-5t: Fix USB host phy-supply
on Rock 5b-5bp-5t SbC
usb: typec: fusb302: Switch to threaded interrupt handler
Quoted 9 lines
arch/arm64/boot/dts/rockchip/rk3588-rock-5b-5bp-5t.dtsi | 7 +++---- drivers/usb/typec/tcpm/fusb302.c | 7 ++++--- 2 files changed, 7 insertions(+), 7 deletions(-)base-commit:
805f9a061372164d43ddef771d7cd63e3ba6d845Signature · 1 line
2.50.1
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi Sebastian,
Thanks for sharing the details.
Quoted 18 lines · Sebastian Reichel
Hello Anand,
Quoted 5 lines · Anand Moon
On the Radxa Rock 5B, the system occasionally experiences intermittent hard resets during the boot process. Initially, I suspected a power supply issue, but further investigation points to the Type-C fusb302 module as the cause.
Quoted 8 lines
Specifically, probing or reloading the fusb302 module triggers a hard reset, which can result in immediate power loss and a reboot.
[root@rockpi-5b ~]# rmmod fusb302
[root@rockpi-5b ~]# lsmod | grep fusb302
[root@rockpi-5b ~]# modprobe fusb302
[root@rockpi-5b ~]# [ 3389.031608][ T7143]typec_fusb3024-0022: Initiating hard-reset, which might result in machine power-loss.[ 3390.030444][ T7143] typec_fusb302 4-0022: Initiating hard-reset, which might result in machine power-loss.
Quoted 5 lines
If you see this message the TypeC port manager (TCPM) state machine reached the hard reset error state. A USB-PD hard reset involves removing VBUS for a short time, which effectively removes the board power on ROCK 5B. Unfortunately the situation is quite complex :)
I completely understand this issue,
Quoted 7 lines
Quoted 3 lines
I attempted to trace the issue using ftrace but was unable to pinpoint the root cause. The problem appears to originate either from the I2C controller or the PMIC reset.
I2C and PMIC are not at fault. This is all about USB-PD communication itself.
Quoted 7 lines
Quoted 4 lines
I have identified a potential workaround involving the I2C SCL debounce settings for the RK3588 and submitted a patch here:
This is most likely a red herring and just slightly changing timings in the USB PD communication.
Yes, but it's related I2C protocol.
Quoted 11 lines
Quoted 3 lines
Please note that the submitted changes address a minor aspect but do not fully resolve the underlying issue.
I don't expect any fix from this series regarding your problem. Also I suggest having a look at my talk at the Linux Plumbers Conference from last month where I discussed this issue :)
slides: https://lpc.events/event/19/contributions/2156/attachments/1784/3861/improving-stability-for-TCPM-using-boards-that-are-not-self-powered.pdf
video: https://www.youtube.com/watch?v=DmLsePJoH8I
It's a great talk. I understood the issue much better
I have understood the previous discussion regarding the reset failure. In this specific case, the Type-C power is controlled by the USB 3.0 and USB 2.0 host controller power domains, I feel so.
Quoted 6 lines
Something that might be sensible to do on your end is figure out *how* the state machine ended up in the error state and check if we can avoid it. The related code for that lives in
drivers/usb/typec/tcpm/tcpm.cand quite complex. I use the following two patches to ease debugging:
Quoted 6 lines
* https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/6edc68e3c0ec4c209b5e96b848e17201059ce9ee
* https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/8ca8b1d6ee36e80f794bcf351a8b78d5a96daf06Combined with
CONFIG_DYNAMIC_DEBUG=yand booting with the following kernel arguments: loglevel=8 tcpm.dyndbg="+p" fusb302.dyndbg="+p"
Yep, I have enabled this, but I could not find many details
Quoted 4 lines
Greetings,
-- Sebastian
Thanks
-Anand