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

[FUKAUMI Naoki]

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 at collabora.com>
>>> ---
>>>    .../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 at 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