Re: [PATCH v1 1/1] arm64: dts: rockchip: Add USB-C support to ROCK 5B

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sebastian,

On 12/11/24 08:08, Sebastian Reichel wrote:
Hello Naoki,

On Wed, Dec 11, 2024 at 07:10:55AM +0900, FUKAUMI Naoki wrote:
Hi Sebastian,

Thank you very much for your work!

$ cat /sys/class/power_supply/tcpm-source-psy-4-0022/{current_max,current_now,online,type,usb_type,voltage_max,voltage_min,voltage_now}
1500000
1500000
1
USB
C [PD] PD_PPS
20000000
20000000
20000000

$ cat /sys/class/power_supply/tcpm-source-psy-4-0022/{current_max,current_now,online,type,usb_type,voltage_max,voltage_min,voltage_now}
5000000
5000000
1
USB
C PD [PD_PPS]
20000000
20000000
20000000

$ ls /sys/class/udc/
fc000000.usb

I can configure it as CDC-NCM and host detects it.
But I could not use it as a HOST port. How to use it?

You can switch between host and peripheral for Type-C ports like
this depending on the remote sides capabilities:

  * echo host > /sys/class/typec/<port>/data_role
  * echo device > /sys/class/typec/<port>/data_role

thanks!

I tested both data_role and both orientation. all works.

I tested this with a USB-C hub connected to the port, which works
in host mode.

some minor nitpick is below:

On 12/11/24 01:36, Sebastian Reichel wrote:
Add hardware description for the USB-C port in the Radxa Rock 5 Model B.
This describes the OHCI, EHCI and XHCI USB parts, but not yet the
DisplayPort AltMode (bindings are not yet upstream).

The fusb302 node is marked with status "fail", since the board is usually
powered through the USB-C port. Handling of errors can result in hard
resets, which removed the bus power for some time resulting in a board
reset.

The main problem is that devices are supposed to interact with the
power-supply within 5 seconds after the plug event according to the
USB PD specification. This is more or less impossible to achieve when
the kernel is the first software communicating with the power-supply.

Recent U-Boot (v2025.01) will start doing USB-PD communication, which
solves this issue. Upstream U-Boot doing USB-PD communication will also
set the fusb302 node status to "okay". That way booting a kernel with
the updated DT on an old U-Boot avoids a reset loop.

Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
---
   .../boot/dts/rockchip/rk3588-rock-5b.dts      | 121 ++++++++++++++++++
   1 file changed, 121 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
index d597112f1d5b..cb5990df6ccb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
@@ -5,6 +5,7 @@
   #include <dt-bindings/gpio/gpio.h>
   #include <dt-bindings/leds/common.h>
   #include <dt-bindings/soc/rockchip,vop2.h>
+#include <dt-bindings/usb/pd.h>
   #include "rk3588.dtsi"
   / {
@@ -84,6 +85,15 @@ rfkill-bt {
   		shutdown-gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
   	};
+	vcc12v_dcin: regulator-vcc12v-dcin {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc12v_dcin";

typec_vin by schematic.

Ack. Will update in v2.

+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;

both microvolt line can be removed.

+	};
+
   	vcc3v3_pcie2x1l0: regulator-vcc3v3-pcie2x1l0 {
   		compatible = "regulator-fixed";
   		enable-active-high;
@@ -142,6 +152,7 @@ vcc5v0_sys: regulator-vcc5v0-sys {
   		regulator-boot-on;
   		regulator-min-microvolt = <5000000>;
   		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc12v_dcin>;

typec_vin.

   	};
   	vcc_1v1_nldo_s3: regulator-vcc-1v1-nldo-s3 {
@@ -264,6 +275,67 @@ regulator-state-mem {
   	};
   };
+&i2c4 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c4m1_xfer>;
+	status = "okay";
+
+	usbc0: usb-typec@22 {

Is "usbc0" label necessary?

no, but does it hurt?

sorry, please keep it.

+		compatible = "fcs,fusb302";
+		reg = <0x22>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <RK_PB4 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usbc0_int>;

cc_int_l by schematic.

Ack. I intentionally switched away from this naming, since cc prefix
is imho a way worse prefix than usbc0. The _l suffix just means
active low, which is already encoded in DT.

But I don't have a strong opinion and can fix this in v2.

+		vbus-supply = <&vcc12v_dcin>;

typec_vin

+		/*
+		 * When the board is starting to send power-delivery messages
+		 * too late (5 seconds according to the specification), the
+		 * power-supply reacts with a hard-reset. That removes the
+		 * power from VBUS for some time, which resets te whole board.

... resets "the" whole board.

Ack.


+		 */
+		status = "fail";
+
+		usb_con: connector {

Is "usb_con" label necessary?

No. It should either be changed to "usbc0_con" or removed. In
general I tend to add some labels when they might be needed by
something in the future. They are more or less free anyways.

+1 for "usbc0_con".

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

-- Sebastian





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux